DrawingBuffer cleanup: Part 1 of many

third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

Index: third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
index d5f2fe7387d81e90e828bf51832487a7ce01e17e..2da3b9155fbf954664abadbb2648fadb02487204 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
@@ -408,22 +408,34 @@ bool DrawingBuffer::prepareTextureMailboxInternal(
 void DrawingBuffer::gpuMailboxReleased(const gpu::Mailbox& mailbox,
                                        const gpu::SyncToken& syncToken,
                                        bool lostResource) {
-  if (m_destructionInProgress ||
+  // Creation of image backed mailboxes is very expensive, so be less
+  // aggressive about pruning them.
+  size_t cacheLimit = 1;
+  if (shouldUseChromiumImage())
+    cacheLimit = 4;
+
+  if (m_destructionInProgress || m_recycledMailboxQueue.size() >= cacheLimit ||
       m_gl->GetGraphicsResetStatusKHR() != GL_NO_ERROR || lostResource ||
       m_isHidden) {
     deleteMailbox(mailbox, syncToken);
     return;
   }
 
+  RefPtr<MailboxInfo> mailboxInfo;
   for (size_t i = 0; i < m_textureMailboxes.size(); i++) {
-    RefPtr<MailboxInfo> mailboxInfo = m_textureMailboxes[i];
-    if (mailboxInfo->mailbox == mailbox) {
-      m_recycledMailboxQueue.prepend(
-          adoptRef(new RecycledMailbox(mailbox, syncToken)));
-      return;
+    if (m_textureMailboxes[i]->mailbox == mailbox) {
+      mailboxInfo = m_textureMailboxes[i];
+      break;
     }
   }
-  ASSERT_NOT_REACHED();
+  DCHECK(mailboxInfo);
+  if (mailboxInfo->size != m_size) {
+    deleteMailbox(mailbox, syncToken);
+    return;
+  }
+
+  m_recycledMailboxQueue.prepend(
+      adoptRef(new RecycledMailbox(mailbox, syncToken)));
 }
 
 void DrawingBuffer::softwareMailboxReleased(
@@ -570,19 +582,7 @@ PassRefPtr<DrawingBuffer::MailboxInfo> DrawingBuffer::takeRecycledMailbox() {
   if (m_recycledMailboxQueue.isEmpty())
     return nullptr;
 
-  // Creation of image backed mailboxes is very expensive, so be less
-  // aggressive about pruning them.
-  size_t cacheLimit = 1;
-  if (shouldUseChromiumImage())
-    cacheLimit = 4;
-
-  RefPtr<RecycledMailbox> recycled;
-  while (m_recycledMailboxQueue.size() > cacheLimit) {
-    recycled = m_recycledMailboxQueue.takeLast();
-    deleteMailbox(recycled->mailbox, recycled->syncToken);
-  }
-  recycled = m_recycledMailboxQueue.takeLast();
-
+  RefPtr<RecycledMailbox> recycled = m_recycledMailboxQueue.takeLast();
   if (recycled->syncToken.HasData())
     m_gl->WaitSyncTokenCHROMIUM(recycled->syncToken.GetData());
 
@@ -593,13 +593,8 @@ PassRefPtr<DrawingBuffer::MailboxInfo> DrawingBuffer::takeRecycledMailbox() {
       break;
     }
   }
-  ASSERT(mailboxInfo);
-
-  if (mailboxInfo->size != m_size) {
-    resizeTextureMemory(&mailboxInfo->textureInfo, m_size);
-    mailboxInfo->size = m_size;
-  }
-
+  DCHECK(mailboxInfo);
+  DCHECK(mailboxInfo->size == m_size);

[Ken Russell (switch to Gerrit), 2016/10/06 03:47:52]

Am I understanding correctly that this invariant is now maintained by:

 - Discarding recycled mailboxes in gpuMailboxReleased if it has a different
size
 - Throwing out all of the recycled mailboxes in DrawingBuffer::reset

? Just trying to understand the situation.

[ccameron, 2016/10/06 08:34:24]

Yes, exactly.

   return mailboxInfo.release();
 }
 
@@ -805,7 +800,7 @@ void DrawingBuffer::beginDestruction() {
     m_gl->DeleteTextures(1, &m_colorBuffer.textureId);
   }
 
-  setSize(IntSize());
+  m_size = IntSize();
 
   m_colorBuffer = TextureInfo();
   m_frontColorBuffer = FrontBufferInfo();
@@ -829,64 +824,55 @@ GLuint DrawingBuffer::createColorTexture(const TextureParameters& parameters) {
   return offscreenColorTexture;
 }
 
-bool DrawingBuffer::resizeMultisampleFramebuffer(const IntSize& size) {
-  DCHECK(wantExplicitResolve());
-  m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_multisampleFBO);
-  m_gl->BindRenderbuffer(GL_RENDERBUFFER, m_multisampleRenderbuffer);
-  m_gl->RenderbufferStorageMultisampleCHROMIUM(
-      GL_RENDERBUFFER, m_sampleCount, getMultisampledRenderbufferFormat(),
-      size.width(), size.height());
-
-  if (m_gl->GetError() == GL_OUT_OF_MEMORY)
-    return false;
-
-  m_gl->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
-                                GL_RENDERBUFFER, m_multisampleRenderbuffer);
-
-  return true;
-}
-
-void DrawingBuffer::resizeDepthStencil(const IntSize& size) {
-  m_gl->BindFramebuffer(GL_FRAMEBUFFER,
-                        m_multisampleFBO ? m_multisampleFBO : m_fbo);
-  if (!m_depthStencilBuffer)
-    m_gl->GenRenderbuffers(1, &m_depthStencilBuffer);
-  m_gl->BindRenderbuffer(GL_RENDERBUFFER, m_depthStencilBuffer);
-  if (m_antiAliasingMode == MSAAImplicitResolve)
-    m_gl->RenderbufferStorageMultisampleEXT(GL_RENDERBUFFER, m_sampleCount,
-                                            GL_DEPTH24_STENCIL8_OES,
-                                            size.width(), size.height());
-  else if (m_antiAliasingMode == MSAAExplicitResolve)
-    m_gl->RenderbufferStorageMultisampleCHROMIUM(GL_RENDERBUFFER, m_sampleCount,
-                                                 GL_DEPTH24_STENCIL8_OES,
-                                                 size.width(), size.height());
-  else
-    m_gl->RenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8_OES,
-                              size.width(), size.height());
-  // For ES 2.0 contexts DEPTH_STENCIL is not available natively, so we emulate
-  // it at the command buffer level for WebGL contexts.
-  m_gl->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT,
-                                GL_RENDERBUFFER, m_depthStencilBuffer);
-  m_gl->BindRenderbuffer(GL_RENDERBUFFER, 0);
-}
-
 bool DrawingBuffer::resizeDefaultFramebuffer(const IntSize& size) {
   // Resize or create m_colorBuffer.
   if (m_colorBuffer.textureId) {
-    resizeTextureMemory(&m_colorBuffer, size);
-  } else {
-    m_colorBuffer = createTextureAndAllocateMemory(size);
+    deleteChromiumImageForTexture(&m_colorBuffer);
+    m_gl->DeleteTextures(1, &m_colorBuffer.textureId);
   }
+  m_colorBuffer = createTextureAndAllocateMemory(size);
 
   attachColorBufferToReadFramebuffer();
 
   if (wantExplicitResolve()) {
-    if (!resizeMultisampleFramebuffer(size))
+    m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_multisampleFBO);
+    m_gl->BindRenderbuffer(GL_RENDERBUFFER, m_multisampleRenderbuffer);
+    m_gl->RenderbufferStorageMultisampleCHROMIUM(
+        GL_RENDERBUFFER, m_sampleCount, getMultisampledRenderbufferFormat(),
+        size.width(), size.height());
+
+    if (m_gl->GetError() == GL_OUT_OF_MEMORY)
       return false;
+
+    m_gl->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                                  GL_RENDERBUFFER, m_multisampleRenderbuffer);
   }
 
-  if (wantDepthOrStencil())
-    resizeDepthStencil(size);
+  if (wantDepthOrStencil()) {
+    m_gl->BindFramebuffer(GL_FRAMEBUFFER,
+                          m_multisampleFBO ? m_multisampleFBO : m_fbo);
+    if (!m_depthStencilBuffer)
+      m_gl->GenRenderbuffers(1, &m_depthStencilBuffer);
+    m_gl->BindRenderbuffer(GL_RENDERBUFFER, m_depthStencilBuffer);
+    if (m_antiAliasingMode == MSAAImplicitResolve) {
+      m_gl->RenderbufferStorageMultisampleEXT(GL_RENDERBUFFER, m_sampleCount,
+                                              GL_DEPTH24_STENCIL8_OES,
+                                              size.width(), size.height());
+    } else if (m_antiAliasingMode == MSAAExplicitResolve) {
+      m_gl->RenderbufferStorageMultisampleCHROMIUM(
+          GL_RENDERBUFFER, m_sampleCount, GL_DEPTH24_STENCIL8_OES, size.width(),
+          size.height());
+    } else {
+      m_gl->RenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8_OES,
+                                size.width(), size.height());
+    }
+    // For ES 2.0 contexts DEPTH_STENCIL is not available natively, so we
+    // emulate
+    // it at the command buffer level for WebGL contexts.
+    m_gl->FramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT,
+                                  GL_RENDERBUFFER, m_depthStencilBuffer);
+    m_gl->BindRenderbuffer(GL_RENDERBUFFER, 0);
+  }
 
   if (wantExplicitResolve()) {
     m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_multisampleFBO);
@@ -912,13 +898,6 @@ void DrawingBuffer::clearFramebuffers(GLbitfield clearMask) {
   m_gl->Clear(clearMask);
 }
 
-void DrawingBuffer::setSize(const IntSize& size) {
-  if (m_size == size)
-    return;
-
-  m_size = size;
-}
-
 IntSize DrawingBuffer::adjustSize(const IntSize& desiredSize,
                                   const IntSize& curSize,
                                   int maxTextureSize) {
@@ -950,7 +929,7 @@ bool DrawingBuffer::reset(const IntSize& newSize) {
       break;
     } while (!adjustedSize.isEmpty());
 
-    setSize(adjustedSize);
+    m_size = adjustedSize;
 
     if (adjustedSize.isEmpty())
       return false;
@@ -974,6 +953,7 @@ bool DrawingBuffer::reset(const IntSize& newSize) {
   }
 
   clearFramebuffers(clearMask);
+  freeRecycledMailboxes();
   return true;
 }
 
@@ -1248,44 +1228,9 @@ DrawingBuffer::TextureInfo DrawingBuffer::createDefaultTextureAndAllocateMemory(
   allocateConditionallyImmutableTexture(
       parameters.target, parameters.creationInternalColorFormat, size.width(),
       size.height(), 0, parameters.colorFormat, GL_UNSIGNED_BYTE);
-  info.immutable = m_storageTextureSupported;
   return info;
 }
 
-void DrawingBuffer::resizeTextureMemory(TextureInfo* info,
-                                        const IntSize& size) {
-  ASSERT(info->textureId);
-  if (!shouldUseChromiumImage()) {
-    if (info->immutable) {
-      DCHECK(m_storageTextureSupported);
-      m_gl->DeleteTextures(1, &info->textureId);
-      info->textureId = createColorTexture(info->parameters);
-    }
-    m_gl->BindTexture(info->parameters.target, info->textureId);
-    allocateConditionallyImmutableTexture(
-        info->parameters.target, info->parameters.creationInternalColorFormat,
-        size.width(), size.height(), 0, info->parameters.colorFormat,
-        GL_UNSIGNED_BYTE);
-    info->immutable = m_storageTextureSupported;
-    return;
-  }
-
-  DCHECK(!info->immutable);
-  deleteChromiumImageForTexture(info);
-  info->imageId = m_gl->CreateGpuMemoryBufferImageCHROMIUM(
-      size.width(), size.height(), info->parameters.creationInternalColorFormat,
-      GC3D_SCANOUT_CHROMIUM);
-  if (info->imageId) {
-    m_gl->BindTexture(info->parameters.target, info->textureId);
-    m_gl->BindTexImage2DCHROMIUM(info->parameters.target, info->imageId);
-    clearChromiumImageAlpha(*info);
-  } else {
-    // At this point, the texture still exists, but has no allocated
-    // storage. This is intentional, and mimics the behavior of a texImage2D
-    // failure.
-  }
-}
-
 void DrawingBuffer::attachColorBufferToReadFramebuffer() {
   m_gl->BindFramebuffer(GL_FRAMEBUFFER, m_fbo);