Fixing Canvas2DLayerBridge to handle lost graphics contexts

Fixing Canvas2DLayerBridge to handle lost graphics contexts

This patch detects lost contexts and attempts to restore canvas layers
to a usable state, whenever possible. If the layer can not be recovered
All operation that draw to and read from the canvas are skipped.

BUG=240881, 226928, 237799
R=senorblanco@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=154134

[Justin Novosad, 2013-05-24 17:36:15]

PTAL

[Justin Novosad, 2013-05-24 17:37:53]

On 2013/05/24 17:36:15, junov wrote:
> PTAL

Cannot submit until skia r9276 rolls-in
https://code.google.com/p/skia/source/detail?r=9276

[danakj, 2013-05-24 17:42:36]

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:186: if
(!sharedContext || sharedContext->webContext()->isContextLost()) {
Do you actually see a way to have isContextLost() be true here other than the
context getting lost in between you grabbing it and checking the lost value?

I believe the context should be getting a callback on loss and be invalidating
the ContextProvider as soon as the loss happens. When you call SharedGC3D::get()
and get the ContextProvider (in a WGContextProvider), it should see the loss,
and make a new ContextProvider at that time.

[Justin Novosad, 2013-05-24 17:49:39]

On 2013/05/24 17:42:36, danakj wrote:
> Do you actually see a way to have isContextLost() be true here other than the
> context getting lost in between you grabbing it and checking the lost value?

Yes, when the GPU process strikes-out after three restarts, the
SharedGraphicsContext3D remains stays in a lost state.

[Stephen White, 2013-05-24 17:51:01]

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:195: // Old
context was lost and sharedContext has recoverred.
Nit: recovered.

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:207:
SkAutoTUnref<SkSurface> surface(SkSurface::NewRenderTarget(gr, info));
This should be refactored with the same code in ImageBuffer (one place or the
other).

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:210: // FIXME:
draw sad canvas picture into new buffer crbug.com/
Need bug ID. :)

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
File Source/core/platform/graphics/skia/ImageBufferSkia.cpp (right):

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:196: static bool
contextIsValid(const ImageBufferData* data)
I think this should be ImageBuffer::isValid(), and take no args.

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:214: if
(contextIsValid(&m_data)) {
Since this is the exceptional state, this should be the early-return.

I.e.,

if (!isValid())
    return BitmapImage::create(NativeImageSkia::create());

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:274: if
(contextIsValid(&m_data)) {
Same here:

if (!isValid())
    return;

https://codereview.chromium.org/16032003/diff/1/Source/core/platform/graphics...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:284: if
(contextIsValid(&m_data)) {
Same here (and more places below; I'll stop nagging now).

[danakj, 2013-05-24 18:02:46]

On Fri, May 24, 2013 at 1:49 PM, <junov@chromium.org> wrote:

> On 2013/05/24 17:42:36, danakj wrote:
>
>> Do you actually see a way to have isContextLost() be true here other than
>> the
>> context getting lost in between you grabbing it and checking the lost
>> value?
>>
>
> Yes, when the GPU process strikes-out after three restarts, the
> SharedGraphicsContext3D remains stays in a lost state.
>

Ah! Cool, thanks!


>
>
>
https://codereview.chromium.**org/16032003/<https://codereview.chromium.org/1...
>

[Justin Novosad, 2013-05-27 23:41:10]

Oops, I forgot to mention: New Patch.

[Stephen White, 2013-05-28 14:23:10]

https://codereview.chromium.org/16032003/diff/7002/Source/core/platform/graph...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/16032003/diff/7002/Source/core/platform/graph...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:74: // Factory
overload used for unit testing
Nit: this comment should probably be in the header, to be more easily noticed.

https://codereview.chromium.org/16032003/diff/7002/Source/core/platform/graph...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:242:
m_layer->setTextureId(renderTarget->asTexture()->getTextureHandle());
Not new to this patch, but I see this code in four different places now in this
file (constructor, backBufferTexture(), prepareTexture(), here). IWBN to
refactor them.

https://codereview.chromium.org/16032003/diff/7002/Source/core/platform/graph...
File Source/core/platform/graphics/skia/ImageBufferSkia.cpp (right):

https://codereview.chromium.org/16032003/diff/7002/Source/core/platform/graph...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:340: return
result.release();
Nit: I think you could just return the result of Uint8ClampedArray::create()
without storing it in a temporary.

https://codereview.chromium.org/16032003/diff/7002/Source/core/platform/graph...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:348:
RefPtr<Uint8ClampedArray> result = Uint8ClampedArray::create(rect.width() *
rect.height() * 4);
Same here.

[Stephen White, 2013-05-28 14:23:41]

LGTM as-is, or you can fix the nits (your call).

[commit-bot: I haz the power, 2013-05-28 14:55:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/7002

[commit-bot: I haz the power, 2013-05-28 15:02:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/18001

[commit-bot: I haz the power, 2013-05-28 15:17:35]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_layout.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-28 15:24:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/28001

[commit-bot: I haz the power, 2013-05-28 15:43:58]

Retried try job too often on linux_layout_rel for step(s) webkit_tests,
webkit_unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[Justin Novosad, 2013-05-28 17:37:21]

On 2013/05/28 15:43:58, I haz the power (commit-bot) wrote:
Requesting new review.
Patch Set 5 has significant changes, which were necessary to resolve unit test
failures: Canvas2DLayerBridge no longer has two create methods; initialization
code was factored out of constructor so that it can run with overridden vtable;
made isValid virtual for testing purposes. Changes to unit test code.

[Stephen White, 2013-05-28 18:05:43]

https://codereview.chromium.org/16032003/diff/38001/Source/WebKit/chromium/te...
File Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp (right):

https://codereview.chromium.org/16032003/diff/38001/Source/WebKit/chromium/te...
Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:68: virtual bool
isValid() OVERRIDE
Now that we have a virtual isValid(), maybe we could use it for testing for the
correct behaviour in the lost-context case.

https://codereview.chromium.org/16032003/diff/38001/Source/core/platform/grap...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/16032003/diff/38001/Source/core/platform/grap...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:70:
layerBridge->init(opacityMode, threading);
You're gonna hate me, but I liked this better when it was a separate constructor
for testing. Breaking out init() into its own method makes it fragile in case
the caller forgets to init(). I'd prefer that the constructor always return a
fully-constructed object. I wonder if we could do this by doing a lazy-init, and
only do it on first draw?

https://codereview.chromium.org/16032003/diff/38001/Source/core/platform/grap...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:103:
TRACE_EVENT_INSTANT0("test_gpu", "Canvas2DLayerBridgeCreation");
If this does stay in its own function, the tracing event text should probably be
renamed to "Canvas2DLayerBridge::init()".

[Justin Novosad, 2013-05-29 20:55:18]

>
https://codereview.chromium.org/16032003/diff/38001/Source/core/platform/grap...
> Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:70:
> layerBridge->init(opacityMode, threading);
> You're gonna hate me, but I liked this better when it was a separate
constructor
> for testing. Breaking out init() into its own method makes it fragile in case
> the caller forgets to init(). I'd prefer that the constructor always return a
> fully-constructed object. I wonder if we could do this by doing a lazy-init,
and
> only do it on first draw?
> 

I played with the idea. Init in the constructor is not possible because init
needs the final vtable. lazy-init is a little too messy for its own good.  How
about just verifying initialization with an assertion?

[Justin Novosad, 2013-05-29 21:11:21]

Patch.

Added unit test.
sprinkled ASSERT(m_layer) in a few places to verify that
Canvas2DLayerBridge::init was called.

[Justin Novosad, 2013-05-31 14:43:58]

Review ping.

[Stephen White, 2013-05-31 15:06:18]

https://codereview.chromium.org/16032003/diff/47001/Source/WebKit/chromium/te...
File Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp (right):

https://codereview.chromium.org/16032003/diff/47001/Source/WebKit/chromium/te...
Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:121: void
contextLostTest()
Thanks for the new test!

https://codereview.chromium.org/16032003/diff/47001/Source/WebKit/chromium/te...
Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:124: // Which simulated
what happens when the shared context successfully recoveres
Nit: "for a new, Which simulated what happens" i can haz grammar?

https://codereview.chromium.org/16032003/diff/47001/Source/core/platform/grap...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp (right):

https://codereview.chromium.org/16032003/diff/47001/Source/core/platform/grap...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:105:
m_canvas->setNotificationClient(this);
As discussed, let's make one last attempt to get rid of init() by putting this
call last in the constructor (with a comment).

https://codereview.chromium.org/16032003/diff/47001/Source/core/platform/grap...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.h (right):

https://codereview.chromium.org/16032003/diff/47001/Source/core/platform/grap...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.h:86: SkCanvas*
getCanvas() {return m_canvas;}
Nit: Missing spaces?

https://codereview.chromium.org/16032003/diff/47001/Source/core/platform/grap...
File Source/core/platform/graphics/skia/ImageBufferSkia.cpp (right):

https://codereview.chromium.org/16032003/diff/47001/Source/core/platform/grap...
Source/core/platform/graphics/skia/ImageBufferSkia.cpp:78: data->m_layerBridge =
Canvas2DLayerBridge::create(context3D.release(), size, bridgeOpacityMode,
threadMode);
Thanks for refactoring this!

[Justin Novosad, 2013-05-31 16:17:46]

All done.  New patch.

[Stephen White, 2013-05-31 17:13:33]

LGTM

https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
File Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp (right):

https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:123: // This simulates
what happens when the shared context successfully recoveres
Nit: recovers

[Justin Novosad, 2013-05-31 18:07:56]

Thx.

Looks like I am missing an OWNER for the unit tests.
+jamesr

[jamesr, 2013-05-31 18:13:44]

On 2013/05/31 18:07:56, junov wrote:
> Thx.
> 
> Looks like I am missing an OWNER for the unit tests.

You should move them!

> +jamesr

[jamesr, 2013-05-31 18:15:45]

Are there any tests (unit or otherwise) that exercise the codepath when a
recovery at least partially succeeds?  What actually happens in this case?  I'm
worried about leaving canvases in weird intermediate states after lost context,
that seems like it could be worse than just having the canvas be blank.

https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
File Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp (right):

https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:144: // to recover.
Recovery aborts because the fake context does not provide a grContext.
hmm, so are there any tests that cover a successful(ish) recovery?

[Justin Novosad, 2013-05-31 18:45:52]

On 2013/05/31 18:15:45, jamesr wrote:
> Are there any tests (unit or otherwise) that exercise the codepath when a
> recovery at least partially succeeds?  What actually happens in this case? 
I'm
> worried about leaving canvases in weird intermediate states after lost
context,
> that seems like it could be worse than just having the canvas be blank.

Agreed. The implementation of isValid is designed to prevent that from
happening. If any part of the recovery fails, isValid will return false, and the
canvas will not be used.
> 
>
https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
> File Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp (right):
> 
>
https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
> Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:144: // to recover.
> Recovery aborts because the fake context does not provide a grContext.
> hmm, so are there any tests that cover a successful(ish) recovery?

I think this is as far as we can go with just a FakeGraphicsContext3D.  To take
testing to the next level, we'd need to bootstrap faking at the GLES2 command
buffer interface level so that SkSurface and GrContext can do their thing.  I am
thinking that approach is too complex for it's own good. Is there a way to
trigger (simulate?) a graphics reset in a layout test?

[jamesr, 2013-05-31 19:28:48]

On 2013/05/31 18:45:52, junov wrote:
> On 2013/05/31 18:15:45, jamesr wrote:
> > Are there any tests (unit or otherwise) that exercise the codepath when a
> > recovery at least partially succeeds?  What actually happens in this case? 
> I'm
> > worried about leaving canvases in weird intermediate states after lost
> context,
> > that seems like it could be worse than just having the canvas be blank.
> 
> Agreed. The implementation of isValid is designed to prevent that from
> happening. If any part of the recovery fails, isValid will return false, and
the
> canvas will not be used.
> > 
> >
>
https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
> > File Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp (right):
> > 
> >
>
https://codereview.chromium.org/16032003/diff/57001/Source/WebKit/chromium/te...
> > Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:144: // to recover.
> > Recovery aborts because the fake context does not provide a grContext.
> > hmm, so are there any tests that cover a successful(ish) recovery?
> 
> I think this is as far as we can go with just a FakeGraphicsContext3D.  To
take
> testing to the next level, we'd need to bootstrap faking at the GLES2 command
> buffer interface level so that SkSurface and GrContext can do their thing.  I
am
> thinking that approach is too complex for it's own good. Is there a way to
> trigger (simulate?) a graphics reset in a layout test?

I don't see any way to do that currently, but it should be possible to add.

[Justin Novosad, 2013-06-18 17:24:17]

Patch!

Added layout test with internal API method to trigger a context loss from JS

[Stephen White, 2013-06-18 18:06:57]

LGTM, but leaving for James.

https://codereview.chromium.org/16032003/diff/66001/LayoutTests/fast/canvas/s...
File LayoutTests/fast/canvas/script-tests/canvas-lost-gpu-context.js (right):

https://codereview.chromium.org/16032003/diff/66001/LayoutTests/fast/canvas/s...
LayoutTests/fast/canvas/script-tests/canvas-lost-gpu-context.js:10:
window.internals.loseSharedGraphicsContext3D();
I wonder if we should do this four times, to test the three-strikes software
fallback? Not sure, though.

[jamesr, 2013-06-19 18:06:24]

On 2013/06/18 18:06:57, Stephen White wrote:
> LGTM, but leaving for James.
> 

What in particular do you want me to look at?

[Stephen White, 2013-06-19 18:16:45]

On 2013/06/19 18:06:24, jamesr wrote:
> On 2013/06/18 18:06:57, Stephen White wrote:
> > LGTM, but leaving for James.
> > 
> 
> What in particular do you want me to look at?

The new layout test, which you requested in #27, I think.

[jamesr, 2013-06-19 18:22:47]

Ah, got it.  I don't see any tests covering what happens when the canvas is not
recoverable.

[Justin Novosad, 2013-06-19 18:32:14]

On 2013/06/19 18:22:47, jamesr wrote:
> Ah, got it.  I don't see any tests covering what happens when the canvas is
not
> recoverable.

You are referring to the three strikes I presume. Can I trigger that with
loseContextCHROMIUM?  I'll give it a try but if it does not work, I will need
some pointers.  Are there WebGL tests that cover that case?

[jamesr, 2013-06-19 18:34:01]

On 2013/06/19 18:32:14, junov wrote:
> On 2013/06/19 18:22:47, jamesr wrote:
> > Ah, got it.  I don't see any tests covering what happens when the canvas is
> not
> > recoverable.
> 
> You are referring to the three strikes I presume. Can I trigger that with
> loseContextCHROMIUM?  I'll give it a try but if it does not work, I will need
> some pointers.  Are there WebGL tests that cover that case?

The description says "If the layer can not be recovered all operation that draw
to and read from the canvas are skipped."  I'm sure there must be some cases
where we can't fully recover the canvas context's contents after a loss, since
we don't keep everything around in software, so what happens then?

[Justin Novosad, 2013-06-19 20:27:08]

On 2013/06/19 18:34:01, jamesr wrote:
> On 2013/06/19 18:32:14, junov wrote:
> > On 2013/06/19 18:22:47, jamesr wrote:
> > > Ah, got it.  I don't see any tests covering what happens when the canvas
is
> > not
> > > recoverable.
> > 
> > You are referring to the three strikes I presume. Can I trigger that with
> > loseContextCHROMIUM?  I'll give it a try but if it does not work, I will
need
> > some pointers.  Are there WebGL tests that cover that case?
> 
> The description says "If the layer can not be recovered all operation that
draw
> to and read from the canvas are skipped."  I'm sure there must be some cases
> where we can't fully recover the canvas context's contents after a loss, since
> we don't keep everything around in software, so what happens then?

Turns out that doing this was a very good idea because it made me discover that
the call to loseContextCHROMIUM was not doing anything. So the previous test was
not even testing anything useful.  After some investigation, I figured-out
why...  It turns out that loseContextCHROMIUM is only implemented in the
in-process command buffer, and not in the inter-process version of the command
buffer!

[Justin Novosad, 2013-06-26 21:51:42]

New Patch. 

Got the test working properly.  Will launch try jobs after this lands:
https://codereview.chromium.org/17176027/

[Justin Novosad, 2013-06-27 13:58:52]

New Patch: Test fix: restored name of Canvas2DLayerBridgeCreation trace event,
which was tracked by gpu_test Canvas2DAllowed

[Justin Novosad, 2013-06-28 14:44:57]

On 2013/06/27 13:58:52, junov wrote:
Review Ping.

[Justin Novosad, 2013-07-04 15:19:02]

On 2013/06/28 14:44:57, junov wrote:
> On 2013/06/27 13:58:52, junov wrote:
> Review Ping.

re-ping

[Stephen White, 2013-07-04 16:11:55]

https://codereview.chromium.org/16032003/diff/96001/Source/core/platform/grap...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.h (right):

https://codereview.chromium.org/16032003/diff/96001/Source/core/platform/grap...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.h:91: // Special
constructor only used for unit testing
Nit: Is this comment stale? It looks like there's no longer a new constructor,
just this one.

[Justin Novosad, 2013-07-04 17:34:03]

https://codereview.chromium.org/16032003/diff/96001/Source/core/platform/grap...
File Source/core/platform/graphics/chromium/Canvas2DLayerBridge.h (right):

https://codereview.chromium.org/16032003/diff/96001/Source/core/platform/grap...
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.h:91: // Special
constructor only used for unit testing
On 2013/07/04 16:11:56, Stephen White wrote:
> Nit: Is this comment stale? It looks like there's no longer a new constructor,
> just this one.

Done.

[commit-bot: I haz the power, 2013-07-11 20:01:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/5629499534...

[commit-bot: I haz the power, 2013-07-11 20:02:05]

Failed to apply patch for
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
  Hunk #2 succeeded at 44 (offset -1 lines).
  Hunk #3 succeeded at 90 (offset -1 lines).
  Hunk #4 succeeded at 98 (offset -1 lines).
  Hunk #5 succeeded at 121 (offset -1 lines).
  Hunk #6 FAILED at 186.
  Hunk #7 succeeded at 283 (offset -7 lines).
  Hunk #8 succeeded at 295 (offset -7 lines).
  1 out of 8 hunks FAILED -- saving rejects to file
Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp.rej

Patch:       Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
Index: Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
diff --git a/Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
b/Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
index
b723a85fe9e6f62b7edfe467dab0b212185a7f83..4dbe1bea3ff22ed218c7507e8b83194de418f8ba
100644
--- a/Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
+++ b/Source/core/platform/graphics/chromium/Canvas2DLayerBridge.cpp
@@ -34,6 +34,7 @@
 #include "core/platform/graphics/GraphicsContext3D.h"
 #include "core/platform/graphics/GraphicsLayer.h"
 #include "core/platform/graphics/chromium/Canvas2DLayerManager.h"
+#include "core/platform/graphics/gpu/SharedGraphicsContext3D.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebCompositorSupport.h"
 #include "public/platform/WebGraphicsContext3D.h"
@@ -44,11 +45,38 @@ using WebKit::WebTextureUpdater;
 
 namespace WebCore {
 
+static SkSurface* createSurface(GraphicsContext3D* context3D, const IntSize&
size)
+{
+    ASSERT(!context3D->webContext()->isContextLost());
+    GrContext* gr = context3D->grContext();
+    if (!gr)
+        return 0;
+    gr->resetContext();
+    SkImage::Info info;
+    info.fWidth = size.width();
+    info.fHeight = size.height();
+    info.fColorType = SkImage::kPMColor_ColorType;
+    info.fAlphaType = SkImage::kPremul_AlphaType;
+    return SkSurface::NewRenderTarget(gr, info);
+}
+
+PassOwnPtr<Canvas2DLayerBridge>
Canvas2DLayerBridge::create(PassRefPtr<GraphicsContext3D> context, const
IntSize& size, OpacityMode opacityMode, ThreadMode threading)
+{
+    TRACE_EVENT_INSTANT0("test_gpu", "Canvas2DLayerBridgeCreation");
+    SkAutoTUnref<SkSurface> surface(createSurface(context.get(), size));
+    if (!surface.get())
+        return PassOwnPtr<Canvas2DLayerBridge>();
+    SkDeferredCanvas* canvas = new SkDeferredCanvas(surface);
+    OwnPtr<Canvas2DLayerBridge> layerBridge = adoptPtr(new
Canvas2DLayerBridge(context, canvas, opacityMode, threading));
+    return layerBridge.release();
+}
+
 Canvas2DLayerBridge::Canvas2DLayerBridge(PassRefPtr<GraphicsContext3D> context,
SkDeferredCanvas* canvas, OpacityMode opacityMode, ThreadMode threadMode)
     : m_canvas(canvas)
     , m_context(context)
     , m_bytesAllocated(0)
     , m_didRecordDrawCommand(false)
+    , m_surfaceIsValid(true)
     , m_framesPending(0)
     , m_rateLimitingEnabled(false)
     , m_next(0)
@@ -63,6 +91,7 @@
Canvas2DLayerBridge::Canvas2DLayerBridge(PassRefPtr<GraphicsContext3D> context,
     m_layer->setOpaque(opacityMode == Opaque);
     GraphicsLayer::registerContentsLayer(m_layer->layer());
     m_layer->setRateLimitContext(m_rateLimitingEnabled);
+    m_canvas->setNotificationClient(this);
 }
 
 Canvas2DLayerBridge::~Canvas2DLayerBridge()
@@ -70,9 +99,9 @@ Canvas2DLayerBridge::~Canvas2DLayerBridge()
     GraphicsLayer::unregisterContentsLayer(m_layer->layer());
     Canvas2DLayerManager::get().layerToBeDestroyed(this);
     m_canvas->setNotificationClient(0);
+    m_mailboxes.clear();
     m_layer->clearTexture();
     m_layer.clear();
-    m_mailboxes.clear();
 }
 
 void Canvas2DLayerBridge::limitPendingFrames()
@@ -93,6 +122,14 @@ void Canvas2DLayerBridge::limitPendingFrames()
 
 void Canvas2DLayerBridge::prepareForDraw()
 {
+    ASSERT(m_layer);
+    if (!isValid()) {
+        if (m_canvas) {
+            // drop pending commands because there is no surface to draw to
+            m_canvas->silentFlush();
+        }
+        return;
+    }
     m_context->makeContextCurrent();
 }
 
@@ -149,18 +186,51 @@ void Canvas2DLayerBridge::flush()
 
 unsigned Canvas2DLayerBridge::prepareTexture(WebTextureUpdater& updater)
 {
+    if (!isValid())
+        return 0;
     ASSERT_NOT_REACHED();
     return 0;
 }
 
 WebGraphicsContext3D* Canvas2DLayerBridge::context()
 {
-    return m_context->webContext();
+    return isValid() ? m_context->webContext() : 0;
+}
+
+bool Canvas2DLayerBridge::isValid()
+{
+    ASSERT(m_layer);
+    if (m_context->webContext()->isContextLost() || !m_surfaceIsValid) {
+        // Attempt to recover.
+        m_layer->clearTexture();
+        RefPtr<GraphicsContext3D> sharedContext =
SharedGraphicsContext3D::get();
+        if (!sharedContext || sharedContext->webContext()->isContextLost()) {
+            m_surfaceIsValid = false;
+            return false;
+        }
+        m_context = sharedContext;
+        IntSize size(m_canvas->getTopDevice()->width(),
m_canvas->getTopDevice()->height());
+        SkAutoTUnref<SkSurface> surface(createSurface(m_context.get(), size));
+        if (surface.get()) {
+            m_canvas->setSurface(surface.get());
+            m_surfaceIsValid = true;
+            // FIXME: draw sad canvas picture into new buffer crbug.com/243842
+        } else {
+            // Surface allocation failed. Set m_surfaceIsValid to false to
+            // trigger subsequent retry.
+            m_surfaceIsValid = false;
+            return false;
+        }
+    }
+    return true;
+
 }
 
 bool Canvas2DLayerBridge::prepareMailbox(WebKit::WebExternalTextureMailbox*
outMailbox, WebKit::WebExternalBitmap* bitmap)
 {
     ASSERT(!bitmap);
+    if (!isValid())
+        return false;
     // Release to skia textures that were previouosly released by the
     // compositor. We do this before acquiring the next snapshot in
     // order to cap maximum gpu memory consumption.
@@ -253,6 +323,7 @@ void Canvas2DLayerBridge::mailboxReleased(const
WebKit::WebExternalTextureMailbo
 
 WebKit::WebLayer* Canvas2DLayerBridge::layer()
 {
+    ASSERT(m_layer);
     return m_layer->layer();
 }
 
@@ -264,6 +335,8 @@ void Canvas2DLayerBridge::contextAcquired()
 
 unsigned Canvas2DLayerBridge::backBufferTexture()
 {
+    if (!isValid())
+        return 0;
     contextAcquired();
     m_canvas->flush();
     m_context->flush();

[commit-bot: I haz the power, 2013-07-12 17:00:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/125001

[commit-bot: I haz the power, 2013-07-12 17:21:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/125002

[commit-bot: I haz the power, 2013-07-12 18:03:06]

Retried try job too often on linux_layout_rel for step(s) webkit_python_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[Justin Novosad, 2013-07-12 18:27:00]

Committed patchset #17 manually as r154134 (presubmit successful).

[eseidel, 2013-07-12 21:53:21]

I think this caused 12 canvas tests to crash.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=vir...

[commit-bot: I haz the power, 2013-07-26 13:56:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/151001

[commit-bot: I haz the power, 2013-07-26 15:49:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junov@chromium.org/16032003/151001

[commit-bot: I haz the power, 2013-07-26 15:49:14]

Failed to apply patch for LayoutTests/fast/canvas/canvas-lost-gpu-context.html:
File exist but was about to be overwriten
Patch:   NR 
LayoutTests/fast/backgrounds/script-tests/TEMPLATE.html->LayoutTests/fast/canvas/canvas-lost-gpu-context.html
Index: LayoutTests/fast/canvas/canvas-lost-gpu-context.html
diff --git a/LayoutTests/fast/backgrounds/script-tests/TEMPLATE.html
b/LayoutTests/fast/canvas/canvas-lost-gpu-context.html
similarity index 70%
copy from LayoutTests/fast/backgrounds/script-tests/TEMPLATE.html
copy to LayoutTests/fast/canvas/canvas-lost-gpu-context.html
index
fd31b933c78f47a87c0cbfcca4eeb09ac9c5dfd1..81d007a7c6aff07470ff34713e328e74b43759b8
100644
--- a/LayoutTests/fast/backgrounds/script-tests/TEMPLATE.html
+++ b/LayoutTests/fast/canvas/canvas-lost-gpu-context.html
@@ -1,10 +1,9 @@
-<!DOCTYPE html>
 <html>
 <head>
 <script src="../js/resources/js-test-pre.js"></script>
 </head>
 <body>
-<script src="YOUR_JS_FILE_HERE"></script>
+<script src="script-tests/canvas-lost-gpu-context.js"></script>
 <script src="../js/resources/js-test-post.js"></script>
 </body>
 </html>