Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1745)

Unified Diff: Source/platform/graphics/Canvas2DLayerBridge.cpp

Issue 552423002: Fix crash in Canvas2DLayerBridge after a GPU resource is lost (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: response to review Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | Source/platform/graphics/Canvas2DLayerBridgeTest.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/platform/graphics/Canvas2DLayerBridge.cpp
diff --git a/Source/platform/graphics/Canvas2DLayerBridge.cpp b/Source/platform/graphics/Canvas2DLayerBridge.cpp
index 3df94ff34ad91e32ccd7235374d49669af529b17..bc23de81f21ec395ebd8295764b3516e413bbbe1 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,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()
Hongbo Min 2014/09/10 04:51:08 It seems that the issue of memory-use-after-free i
+ // 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;
« no previous file with comments | « no previous file | Source/platform/graphics/Canvas2DLayerBridgeTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698