Synchronize commits at end of JS task

third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/offscreencanvas/OffscreenCanvas.h"
 
+#include <memory>
 #include "core/dom/ExceptionCode.h"
 #include "core/fileapi/Blob.h"
 #include "core/frame/ImageBitmap.h"
 #include "core/html/ImageData.h"
 #include "core/html/canvas/CanvasAsyncBlobCreator.h"
 #include "core/html/canvas/CanvasContextCreationAttributes.h"
 #include "core/html/canvas/CanvasRenderingContext.h"
 #include "core/html/canvas/CanvasRenderingContextFactory.h"
 #include "platform/graphics/Image.h"
 #include "platform/graphics/ImageBuffer.h"
 #include "platform/graphics/OffscreenCanvasFrameDispatcherImpl.h"
 #include "platform/graphics/StaticBitmapImage.h"
 #include "platform/image-encoders/ImageEncoderUtils.h"
+#include "public/platform/Platform.h"
 #include "wtf/MathExtras.h"
-#include <memory>
 
 namespace blink {
 
 OffscreenCanvas::OffscreenCanvas(const IntSize& size) : m_size(size) {}
 
 OffscreenCanvas* OffscreenCanvas::create(unsigned width, unsigned height) {
   return new OffscreenCanvas(
       IntSize(clampTo<int>(width), clampTo<int>(height)));
 }
 
@@ skipping 198 matching lines @@
         this, m_clientId, m_sinkId, m_placeholderCanvasId, m_size.width(),
         m_size.height()));
   }
   return m_frameDispatcher.get();
 }
 
 ScriptPromise OffscreenCanvas::commit(RefPtr<StaticBitmapImage> image,
                                       bool isWebGLSoftwareRendering,
                                       ScriptState* scriptState) {
   getOrCreateFrameDispatcher()->setNeedsBeginFrame(true);
-  if (m_commitPromiseResolver) {
-    if (image) {
-      m_overdrawFrame = std::move(image);
-      m_overdrawFrameIsWebGLSoftwareRendering = isWebGLSoftwareRendering;
-    }
-  } else {
-    m_overdrawFrame = nullptr;
+
+  if (!m_commitPromiseResolver) {
     m_commitPromiseResolver = ScriptPromiseResolver::create(scriptState);
     m_commitPromiseResolver->keepAliveWhilePending();
-    doCommit(std::move(image), isWebGLSoftwareRendering);
+
+    if (image) {
+      // We defer the submission of commit frames at the end of JS task
+      m_currentFrame = std::move(image);
+      m_currentFrameIsWebGLSoftwareRendering = isWebGLSoftwareRendering;
+      Platform::current()->currentThread()->addTaskObserver(m_context);

[Justin Novosad, 2017/03/24 18:40:31]

I am concerned that the registration and de-registration of the task observer
can get tangled with CanvasRenderingContext's own mechanism.  Would be cleaner
if you created a new method CanvasRenderingContext::needsFinalizeFrame(), whic
you would call from here and from CanvasRenderingContext::didDraw

[xlai (Olivia), 2017/03/24 19:50:17]

Done.

+    }
+  } else if (image) {
+    // Two possible scenarios:
+    // 1. An override of m_currentFrame can happen when there are multiple
+    // frames committed before JS task finishes. (m_currentFrame!=nullptr)
+    // 2. The current frame has been dispatched but the promise is not
+    // resolved yet. (m_currentFrame==nullptr)
+    m_currentFrame = std::move(image);
+    m_currentFrameIsWebGLSoftwareRendering = isWebGLSoftwareRendering;
   }
+
   return m_commitPromiseResolver->promise();
 }
 
+void OffscreenCanvas::doCommitAtEndOfTask() {

[Justin Novosad, 2017/03/24 18:40:31]

I'd call this finalizeFrame

[xlai (Olivia), 2017/03/24 19:50:17]

Done.

+  doCommit(std::move(m_currentFrame), m_currentFrameIsWebGLSoftwareRendering);

[Justin Novosad, 2017/03/24 18:40:31]

Should put this inside "if (m_currentFrame)", just in case finalizeFrame was
scheduled for some other reason than commit.

[xlai (Olivia), 2017/03/24 19:50:17]

Done.

+  Platform::current()->currentThread()->removeTaskObserver(m_context);
+}
+
 void OffscreenCanvas::doCommit(RefPtr<StaticBitmapImage> image,
                                bool isWebGLSoftwareRendering) {
   double commitStartTime = WTF::monotonicallyIncreasingTime();
   getOrCreateFrameDispatcher()->dispatchFrame(std::move(image), commitStartTime,
                                               isWebGLSoftwareRendering);
 }
 
 void OffscreenCanvas::beginFrame() {
-  if (m_overdrawFrame) {
-    // if we have an overdraw backlog, push the frame from the backlog
+  if (m_currentFrame) {
+    // If we have an overdraw backlog, push the frame from the backlog
     // first and save the promise resolution for later.
-    doCommit(std::move(m_overdrawFrame),
-             m_overdrawFrameIsWebGLSoftwareRendering);
+    // Then we need to wait for one more frame time to resolve the existing
+    // promise.
+    doCommit(std::move(m_currentFrame), m_currentFrameIsWebGLSoftwareRendering);
   } else if (m_commitPromiseResolver) {
     m_commitPromiseResolver->resolve();
     m_commitPromiseResolver.clear();
     // We need to tell parent frame to stop sending signals on begin frame to
     // avoid overhead once we resolve the promise.
-    // In the case of overdraw frame (if block), we still need to wait for one
-    // more frame time to resolve the existing promise.
     getOrCreateFrameDispatcher()->setNeedsBeginFrame(false);
   }
 }
 
 ScriptPromise OffscreenCanvas::convertToBlob(ScriptState* scriptState,
                                              const ImageEncodeOptions& options,
                                              ExceptionState& exceptionState) {
   if (this->isNeutered()) {
     exceptionState.throwDOMException(InvalidStateError,
                                      "OffscreenCanvas object is detached.");
@@ skipping 43 matching lines @@
 }
 
 DEFINE_TRACE(OffscreenCanvas) {
   visitor->trace(m_context);
   visitor->trace(m_executionContext);
   visitor->trace(m_commitPromiseResolver);
   EventTargetWithInlineData::trace(visitor);
 }
 
 }  // namespace blink