Fix crash in Canvas2DLayerBridge after a GPU resource is lost

Source/platform/graphics/Canvas2DLayerBridge.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 406 matching lines @@
     if (image->uniqueID() == m_lastImageId)
         return false;
     m_lastImageId = image->uniqueID();
 
     MailboxInfo* mailboxInfo = createMailboxInfo();
     mailboxInfo->m_status = MailboxInUse;
     mailboxInfo->m_image = image;
 
     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
     // the state cached in skia so that the deferred copy on write
     // in SkSurface_Gpu does not make any false assumptions.
     mailboxInfo->m_image->getTexture()->textureParamsModified();
 
     webContext->bindTexture(GL_TEXTURE_2D, mailboxInfo->m_image->getTexture()->getTextureHandle());
     webContext->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
     webContext->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
     webContext->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
     webContext->texParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
     webContext->produceTextureCHROMIUM(GL_TEXTURE_2D, mailboxInfo->m_mailbox.name);
     if (isHidden()) {
         // With hidden canvases, we release the SkImage immediately because
         // there is no need for animations to be double buffered.
         mailboxInfo->m_image.clear();
     } else {
         webContext->flush();
         mailboxInfo->m_mailbox.syncPoint = webContext->insertSyncPoint();
     }
     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;
 }
 
 Canvas2DLayerBridge::MailboxInfo* Canvas2DLayerBridge::createMailboxInfo() {
     ASSERT(!m_destructionInProgress);
     MailboxInfo* mailboxInfo;
     for (mailboxInfo = m_mailboxes.begin(); mailboxInfo < m_mailboxes.end(); mailboxInfo++) {
         if (mailboxInfo->m_status == MailboxAvailable) {
             return mailboxInfo;
@@ skipping 30 matching lines @@
                 // No need to clean up the mailbox resource, but make sure the
                 // mailbox can also be reusable once the context is restored.
                 mailboxInfo->m_status = MailboxAvailable;
                 m_releasedMailboxInfoIndex = InvalidMailboxIndex;
                 Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
             } else if (lostResource) {
                 // In case of the resource is lost, we need to delete the backing
                 // 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;
                 m_releasedMailboxInfoIndex = mailboxInfo - m_mailboxes.begin();
                 m_framesSinceMailboxRelease = 0;
                 if (isHidden()) {
                     freeReleasedMailbox();
                 } else {
                     ASSERT(!m_destructionInProgress);
                     Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
@@ skipping 41 matching lines @@
     // This copy constructor should only be used for Vector reallocation
     // Assuming 'other' is to be destroyed, we transfer m_image and
     // m_parentLayerBridge ownership rather than do a refcount dance.
     memcpy(&m_mailbox, &other.m_mailbox, sizeof(m_mailbox));
     m_image = const_cast<MailboxInfo*>(&other)->m_image.release();
     m_parentLayerBridge = const_cast<MailboxInfo*>(&other)->m_parentLayerBridge.release();
     m_status = other.m_status;
 }
 
 } // namespace blink