Fix crash in Canvas2DLayerBridge after a GPU resource is lost

Source/platform/graphics/Canvas2DLayerBridge.cpp

Index: Source/platform/graphics/Canvas2DLayerBridge.cpp
diff --git a/Source/platform/graphics/Canvas2DLayerBridge.cpp b/Source/platform/graphics/Canvas2DLayerBridge.cpp
index 3df94ff34ad91e32ccd7235374d49669af529b17..3f43e5c0d95cd1f2deb1111213d0f08c4b8ac949 100644
--- a/Source/platform/graphics/Canvas2DLayerBridge.cpp
+++ b/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,30 @@ 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();
+                    if (mailboxInfo->m_image->getTexture()) {

[Stephen White, 2014/09/09 20:33:06]

Nit: refactor out mailboxInfo->m_image->getTexture() into a temporary, as it's
called 3 times now.

+                        mailboxInfo->m_image->getTexture()->resetFlag(
+                            static_cast<GrTextureFlags>(GrTexture::kReturnToCache_FlagBit));
+                        mailboxInfo->m_image->getTexture()->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) {
+#if ENABLE(ASSERT)

[Stephen White, 2014/09/09 20:33:06]

Code paths that are conditional on asserts give me pause. If this has no effect
in non-assert mode, let's just assign it unconditionally (ie., no #ifdef).

+                    mailboxInfo->m_status = MailboxAvailable; // To satisfy assert in destructor
+#endif
+                    // 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;