Synchronize commits at end of JS task

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

Index: third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
diff --git a/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp b/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
index a41f0c69a83ff962d9490753f956682b135ca320..248f30d1c4942ad818b23588d2b9a39011bd8277 100644
--- a/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
+++ b/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
@@ -4,6 +4,7 @@
 
 #include "core/offscreencanvas/OffscreenCanvas.h"
 
+#include <memory>
 #include "core/dom/ExceptionCode.h"
 #include "core/fileapi/Blob.h"
 #include "core/frame/ImageBitmap.h"
@@ -17,8 +18,8 @@
 #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 {
 
@@ -237,20 +238,35 @@ 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();
@@ -259,18 +275,17 @@ void OffscreenCanvas::doCommit(RefPtr<StaticBitmapImage> image,
 }
 
 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);
   }
 }