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

Source/platform/graphics/gpu/DrawingBuffer.cpp

 /*
  * Copyright (c) 2010, 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 258 matching lines @@
     markLayerComposited();
 
     // set m_parentDrawingBuffer to make sure 'this' stays alive as long as it has live mailboxes
     ASSERT(!frontColorBufferMailbox->m_parentDrawingBuffer);
     frontColorBufferMailbox->m_parentDrawingBuffer = this;
     *outMailbox = frontColorBufferMailbox->mailbox;
     m_frontColorBuffer = frontColorBufferMailbox->textureInfo;
     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;
     }
 
     for (size_t i = 0; i < m_textureMailboxes.size(); i++) {
         RefPtr<MailboxInfo> mailboxInfo = m_textureMailboxes[i];
         if (nameEquals(mailboxInfo->mailbox, mailbox)) {
             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;
         }
     }
     ASSERT_NOT_REACHED();
 }
 
 void DrawingBuffer::mailboxReleasedWhileDestructionInProgress(const blink::WebExternalTextureMailbox& mailbox)
 {
     ASSERT(m_textureMailboxes.size());
     m_context->makeContextCurrent();
@@ skipping 760 matching lines @@
 void DrawingBuffer::deleteChromiumImageForTexture(TextureInfo* info)
 {
     if (info->imageId) {
         m_context->releaseTexImage2DCHROMIUM(GL_TEXTURE_2D, info->imageId);
         m_context->destroyImageCHROMIUM(info->imageId);
         info->imageId = 0;
     }
 }
 
 } // namespace WebCore