Let canvas decide how to handle the case of resource lost reported by client: 2D part

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 305 matching lines @@
     if (!mailboxInfo)
         return;
 
     ASSERT(mailboxInfo->m_status == MailboxReleased);
     if (mailboxInfo->m_mailbox.syncPoint) {
         context()->waitSyncPoint(mailboxInfo->m_mailbox.syncPoint);
         mailboxInfo->m_mailbox.syncPoint = 0;
     }
     // Invalidate texture state in case the compositor altered it since the copy-on-write.
     if (mailboxInfo->m_image) {
         if (isHidden() || releasedMailboxHasExpired())

[dshwang, 2014/07/09 08:14:50]

Is "|| lostResource" needed?

[Justin Novosad, 2014/07/11 17:22:38]

Not necessary. If we are in a state where resources are being dropped, we won't
be reclaiming from this context later.  So we're safe.

             mailboxInfo->m_image->getTexture()->resetFlag(static_cast<GrTextureFlags>(GrTexture::kReturnToCache_FlagBit));
         mailboxInfo->m_image->getTexture()->textureParamsModified();
         mailboxInfo->m_image.clear();
     }
     mailboxInfo->m_status = MailboxAvailable;
     m_releasedMailboxInfoIndex = InvalidMailboxIndex;
     Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
 }
 
 blink::WebGraphicsContext3D* Canvas2DLayerBridge::context()
@@ skipping 145 matching lines @@
     // means there is a problem.
     // For the single-threaded case, this value needs to be at least
     // kMaxSwapBuffersPending+1 (in render_widget.h).
     // Because of crbug.com/247874, it needs to be kMaxSwapBuffersPending+2.
     // TODO(piman): fix this.
     ASSERT(m_mailboxes.size() <= 4);
     ASSERT(mailboxInfo < m_mailboxes.end());
     return mailboxInfo;
 }
 
-void Canvas2DLayerBridge::mailboxReleased(const blink::WebExternalTextureMailbox& mailbox)
+void Canvas2DLayerBridge::mailboxReleased(const blink::WebExternalTextureMailbox& mailbox, bool lostResource)
 {
     freeReleasedMailbox(); // Never have more than one mailbox in the released state.
+    bool contextLost = m_contextProvider->context3d()->isContextLost() || !m_isSurfaceValid;
     Vector<MailboxInfo>::iterator mailboxInfo;
     for (mailboxInfo = m_mailboxes.begin(); mailboxInfo < m_mailboxes.end(); ++mailboxInfo) {
         if (nameEquals(mailboxInfo->m_mailbox, mailbox)) {
             mailboxInfo->m_mailbox.syncPoint = mailbox.syncPoint;
             ASSERT(mailboxInfo->m_status == MailboxInUse);
-            mailboxInfo->m_status = MailboxReleased;
             // Trigger Canvas2DLayerBridge self-destruction if this is the
             // last live mailbox and the layer bridge is not externally
             // referenced.
-            m_releasedMailboxInfoIndex = mailboxInfo - m_mailboxes.begin();
-            m_framesSinceMailboxRelease = 0;
-            if (isHidden()) {
-                freeReleasedMailbox();
-            } else {
-                ASSERT(!m_destructionInProgress);
-                Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
-            }
             ASSERT(mailboxInfo->m_parentLayerBridge.get() == this);
             mailboxInfo->m_parentLayerBridge.clear();
+
+            if (contextLost) {
+                // 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);

[dshwang, 2014/07/09 08:14:50]

Do we need to remove the given mailboxInfo from m_mailboxes?

[Justin Novosad, 2014/07/11 17:22:38]

That is a good question. A generated mailbox is nothing more than a
cryptographically strong name. This name is not strictly associated with the
context that created it or any context that uses it. My understanding is that
the mailbox will be unbound from the resource it carries if that resource is
lost, but you can still continue to use the mailbox safely. Even if it were not
the case that mailboxes survive a context loss, we would still be OK, because
the context lost/restore mechanism will tear down the current layer bridge and
spawn a new one.  The Canvas2DLayerBridge associated with the lost context will
not be preparing any new mailboxes from that point on.

[danakj, 2014/07/11 17:31:07]

To be sure we're on the same page the |lostResource| does not imply the texture
is invalid or the context3d is lost. It just says that there is no known valid
sync point at which you can start using the texture again, so the only safe
thing you can do is drop your ref to it and stop using it (delete your local
texture id bound to the mailbox). We should not be reusing the mailbox string at
all after that point. Also, when a context is really lost, all textures and
mailbox strings are no longer valid. New textures will have to be made and
produced into mailboxes to make use of again.

+            } else {
+                mailboxInfo->m_status = MailboxReleased;
+                m_releasedMailboxInfoIndex = mailboxInfo - m_mailboxes.begin();
+                m_framesSinceMailboxRelease = 0;
+                if (isHidden() || lostResource) {

[Justin Novosad, 2014/07/11 18:05:05]

According to Dana's most recent comment, we probably need an additional step
here to discard the current mailboxInfo in the lostResource case. Also going
through the MailboxReleased state and then calling freeReleasedMailbox would not
be necessary since we don't have to care about the syncPoint for a lost
resource.

[Hongbo Min, 2014/07/15 13:51:13]

To make code much clean, I reset the syncPoint to 0 for a lost resource, and
reuse freeReleasedMailbox to update the mailbox status.

@junov, @danakj, does it look good to you?

+                    freeReleasedMailbox();
+                } else {
+                    ASSERT(!m_destructionInProgress);
+                    Canvas2DLayerManager::get().layerTransientResourceAllocationChanged(this);
+                }
+            }
             return;
         }
     }
 }
 
 blink::WebLayer* Canvas2DLayerBridge::layer() const
 {
     ASSERT(!m_destructionInProgress);
     ASSERT(m_layer);
     return m_layer->layer();
@@ skipping 24 matching lines @@
 Canvas2DLayerBridge::MailboxInfo::MailboxInfo(const MailboxInfo& other) {
     // This copy constructor should only be used for Vector reallocation
     // Assuming 'other' is to be destroyed, we transfer m_image 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_status = other.m_status;
 }
 
 }