Fix crash in Canvas2DLayerBridge after a GPU resource is lost

BUG=411864
TEST=blink unit test Canvas2DLayerBridgeTest.testPrepareMailboxAndLoseResource

Review URL: https://codereview.chromium.org/552423002

git-svn-id: svn://svn.chromium.org/blink/trunk@181679 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

@@ -424,6 +424,17 @@ bool Canvas2DLayerBridge::prepareMailbox(WebExternalTextureMailbox* outMailbox,
 
     ASSERT(mailboxInfo->m_mailbox.syncPoint == 0);
     ASSERT(mailboxInfo->m_image.get());
+
+    // set m_parentLayerBridge to make sure 'this' stays alive as long as it has
+    // live mailboxes
+    ASSERT(!mailboxInfo->m_parentLayerBridge);
+    mailboxInfo->m_parentLayerBridge = this;
+    *outMailbox = mailboxInfo->m_mailbox;
+
+    GrContext* grContext = m_contextProvider->grContext();
+    if (!grContext)
+        return true; // for testing: skip gl stuff when using a mock graphics context.
+
     ASSERT(mailboxInfo->m_image->getTexture());
 
     // Because of texture sharing with the compositor, we must invalidate
@@ -448,13 +459,7 @@ bool Canvas2DLayerBridge::prepareMailbox(WebExternalTextureMailbox* outMailbox,
     webContext->bindTexture(GL_TEXTURE_2D, 0);
     // Because we are changing the texture binding without going through skia,
     // we must dirty the context.
-    m_contextProvider->grContext()->resetContext(kTextureBinding_GrGLBackendState);
-
-    // set m_parentLayerBridge to make sure 'this' stays alive as long as it has
-    // live mailboxes
-    ASSERT(!mailboxInfo->m_parentLayerBridge);
-    mailboxInfo->m_parentLayerBridge = this;
-    *outMailbox = mailboxInfo->m_mailbox;
+    grContext->resetContext(kTextureBinding_GrGLBackendState);
 
     return true;
 }
@@ -505,16 +510,29 @@ void Canvas2DLayerBridge::mailboxReleased(const WebExternalTextureMailbox& mailb
                 // texture and remove the mailbox from list to avoid reusing it
                 // in future.
                 if (mailboxInfo->m_image) {
-                    mailboxInfo->m_image->getTexture()->resetFlag(
-                        static_cast<GrTextureFlags>(GrTexture::kReturnToCache_FlagBit));
-                    mailboxInfo->m_image->getTexture()->textureParamsModified();
+                    GrTexture* texture = mailboxInfo->m_image->getTexture();
+                    if (texture) {
+                        texture->resetFlag(static_cast<GrTextureFlags>(GrTexture::kReturnToCache_FlagBit));
+                        texture->textureParamsModified();
+                    }
                     mailboxInfo->m_image.clear();
                 }
-                size_t i = mailboxInfo - m_mailboxes.begin();
-                m_mailboxes.remove(i);
-                Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
-                // Here we need to return early since mailboxInfo removal would
-                // also clear m_parentLayerBridge reference.
+                if (m_destructionInProgress) {
+                    mailboxInfo->m_status = MailboxAvailable; // To satisfy assert in destructor
+
+                    // The following line may trigger self destruction. We do not care about
+                    // not cleaning up m_mailboxes during destruction sequence because
+                    // mailboxes will not be recycled after this point. Calling remove()
+                    // could trigger a memory use after free, so we just clear the self
+                    // reference to be safe, and we let the Canvas2DLayerBridge destructor
+                    // take care of freeing m_mailboxes.
+                    mailboxInfo->m_parentLayerBridge.clear();
+                } else {
+                    size_t i = mailboxInfo - m_mailboxes.begin();
+                    m_mailboxes.remove(i); // indirectly clears mailboxInfo->m_parentLayerBridge
+                    Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
+                }
+                // mailboxInfo is not valid from this point, so we return immediately.
                 return;
             } else {
                 mailboxInfo->m_status = MailboxReleased;

third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridgeTest.cpp

@@ -181,6 +181,40 @@ protected:
         bridge->prepareMailbox(0, &bitmap);
         EXPECT_EQ(0u, bridge->m_lastImageId);
     }
+
+    void prepareMailboxAndLoseResourceTest()
+    {
+        MockCanvasContext mainMock;
+        RefPtr<SkSurface> surface = adoptRef(SkSurface::NewRasterPMColor(300, 150));
+        bool lostResource = true;
+
+        // Prepare a mailbox, then report the resource as lost.
+        // This test passes by not crashing and not triggering assertions.
+        {
+            WebExternalTextureMailbox mailbox;
+            OwnPtr<SkDeferredCanvas> canvas = adoptPtr(SkDeferredCanvas::Create(surface.get()));
+            OwnPtr<MockWebGraphicsContext3DProvider> mainMockProvider = adoptPtr(new MockWebGraphicsContext3DProvider(&mainMock));
+            Canvas2DLayerBridgePtr bridge(adoptRef(new Canvas2DLayerBridge(mainMockProvider.release(), canvas.release(), surface, 0, NonOpaque)));
+            bridge->prepareMailbox(&mailbox, 0);
+            bridge->mailboxReleased(mailbox, lostResource);
+        }
+
+        // Retry with mailbox released while bridge destruction is in progress
+        {
+            OwnPtr<SkDeferredCanvas> canvas = adoptPtr(SkDeferredCanvas::Create(surface.get()));
+            OwnPtr<MockWebGraphicsContext3DProvider> mainMockProvider = adoptPtr(new MockWebGraphicsContext3DProvider(&mainMock));
+            WebExternalTextureMailbox mailbox;
+            Canvas2DLayerBridge* rawBridge;
+            {
+                Canvas2DLayerBridgePtr bridge(adoptRef(new Canvas2DLayerBridge(mainMockProvider.release(), canvas.release(), surface, 0, NonOpaque)));
+                bridge->prepareMailbox(&mailbox, 0);
+                rawBridge = bridge.get();
+            } // bridge goes out of scope, but object is kept alive by self references
+            // before fixing crbug.com/411864, the following line you cause a memory use after free
+            // that sometimes causes a crash in normal builds and crashes consistently with ASAN.
+            rawBridge->mailboxReleased(mailbox, lostResource); // This should self-destruct the bridge.
+        }
+    }
 };
 
 namespace {
@@ -195,9 +229,14 @@ TEST_F(Canvas2DLayerBridgeTest, testNoDrawOnContextLost)
     noDrawOnContextLostTest();
 }
 
-TEST_F(Canvas2DLayerBridgeTest, prepareMailboxWithBitmapTest)
+TEST_F(Canvas2DLayerBridgeTest, testPrepareMailboxWithBitmap)
 {
     prepareMailboxWithBitmapTest();
 }
 
+TEST_F(Canvas2DLayerBridgeTest, testPrepareMailboxAndLoseResource)
+{
+    prepareMailboxAndLoseResourceTest();
+}
+
 } // namespace