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

Source/platform/graphics/gpu/DrawingBuffer.cpp

Index: Source/platform/graphics/gpu/DrawingBuffer.cpp
diff --git a/Source/platform/graphics/gpu/DrawingBuffer.cpp b/Source/platform/graphics/gpu/DrawingBuffer.cpp
index 0ec4bf9179012c1217b92aa3362f4f7d02e1e9df..2f6340ba22f2cdff9e74c9f816d4963862855109 100644
--- a/Source/platform/graphics/gpu/DrawingBuffer.cpp
+++ b/Source/platform/graphics/gpu/DrawingBuffer.cpp
@@ -276,9 +276,9 @@ bool DrawingBuffer::prepareMailbox(blink::WebExternalTextureMailbox* outMailbox,
     return true;
 }
 
-void DrawingBuffer::mailboxReleased(const blink::WebExternalTextureMailbox& mailbox)
+void DrawingBuffer::mailboxReleased(const blink::WebExternalTextureMailbox& mailbox, bool lostResource)

[danakj, 2014/07/03 15:09:02]

why is this new variable not used?

[dshwang, 2014/07/03 16:20:17]

After reading https://code.google.com/p/chromium/issues/detail?id=390960#c6, I
understand this situation.

If (lostResource == false && !m_context->isContextLost()), it's normal case
If (lostResource == true && m_context->isContextLost()), gpu process crashed.
If (lostResource == true && !m_context->isContextLost()), now render process is
being shutdown.
If (lostResource == false && m_context->isContextLost()), webgl context is
evicted or javascript kill webgl context.

So
If (lostResource == true && m_context->isContextLost()), we can return early.
But calling mailboxReleasedWhileDestructionInProgress(mailbox) would be better
although all GL calls is no-op, because
mailboxReleasedWhileDestructionInProgress() cleans its member variables.
If (lostResource == true && !m_context->isContextLost()), we need to remove
mailbox on shutdown process, so call
mailboxReleasedWhileDestructionInProgress(), as danakj@ mentioned.
If (lostResource == false && m_context->isContextLost()), we can return early,
but calling mailboxReleasedWhileDestructionInProgress(mailbox) would be better.

In conclusion, all abnormal cases need to remove resources directly.

[Hongbo Min, 2014/07/07 04:30:28]

Thanks your summary, dshwang@.

The reason why lostResource is not used is straightforward, the lostResource can
not determine if a resource is recycled or not, only m_context->isContextLost
can determine.

The lostResource flag is true in case of :
1. The GPU process crashes, or
2. The render thread/process is shutting down, but at this point, it is not
receiving the resource reclaim ACK message for the most recent frame resources
from browser-side compositor for some reasons, e.g. browser compositor becomes
invisible

In the above 2 cases, there is no need to recycle the resource, but we need to
clear the internal reference to ref-counted object held by mailbox. However, we
can not decide if the GPU texture resource is recycled via lostResource flag.

If the context is lost indicating by isContextLost, we don't need to recycle GPU
texture resource, but of course, we need to clear the internal reference.


On 2014/07/03 16:20:17, dshwang wrote:

 {
-    if (m_destructionInProgress) {
+    if (m_destructionInProgress && !m_context->isContextLost()) {

[dshwang, 2014/07/07 09:18:14]

As my explanation is applied to code, this if statement should be
 if (m_destructionInProgress || m_context->isContextLost() || lostResource)

All three case needs to clear internal members as well as texture if possible.

[Hongbo Min, 2014/07/07 09:39:47]

Thanks for your comments. It seems mailboxReleasedWhileDestructionInProgress
requires a valid context to perform mailbox deletion. It needs to call
makeContextCurrent firstly, right?

[dshwang, 2014/07/07 10:09:43]

afaik, when we use invalid context like TexImage2D, it becomes no-op. So IMO
calling mailboxReleasedWhileDestructionInProgress is fine when the context is
invalid.
danakj@, am I right?

         mailboxReleasedWhileDestructionInProgress(mailbox);
         return;
     }
@@ -289,7 +289,8 @@ void DrawingBuffer::mailboxReleased(const blink::WebExternalTextureMailbox& mail
             mailboxInfo->mailbox.syncPoint = mailbox.syncPoint;
             ASSERT(mailboxInfo->m_parentDrawingBuffer.get() == this);
             mailboxInfo->m_parentDrawingBuffer.clear();
-            m_recycledMailboxQueue.prepend(mailboxInfo->mailbox);
+            if (!m_context->isContextLost())

[dshwang, 2014/07/07 09:18:14]

remove

+                m_recycledMailboxQueue.prepend(mailboxInfo->mailbox);
             return;
         }
     }