Implementing promise-based commit for driving OffscreenCanvas animations

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 4c3d4e0421fbbd2b85ca16d1ce435b634533a1ca..2c4f1c4c659a9daa883f0e2a80b1159c20657bbb 100644
--- a/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
+++ b/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
@@ -22,14 +22,28 @@
 
 namespace blink {
 
-OffscreenCanvas::OffscreenCanvas(const IntSize& size)
-    : m_size(size), m_originClean(true) {}
+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)));
 }
 
+OffscreenCanvas::~OffscreenCanvas() {}
+
+void OffscreenCanvas::dispose() {
+  if (m_context) {
+    m_context->detachOffscreenCanvas();
+    m_context = nullptr;
+  }
+  if (m_commitPromiseResolver) {
+    // keepAliveWhilePending() guarantees the promise resolver is never
+    // GC-ed before the OffscreenCanvas
+    m_commitPromiseResolver->reject();
+    m_commitPromiseResolver.clear();
+  }
+}
+
 void OffscreenCanvas::setWidth(unsigned width) {
   IntSize newSize = m_size;
   newSize.setWidth(clampTo<int>(width));
@@ -213,12 +227,46 @@ OffscreenCanvasFrameDispatcher* OffscreenCanvas::getOrCreateFrameDispatcher() {
     // (either main or worker) to the browser process and remains unchanged
     // throughout the lifetime of this OffscreenCanvas.
     m_frameDispatcher = WTF::wrapUnique(new OffscreenCanvasFrameDispatcherImpl(
-        m_clientId, m_sinkId, m_placeholderCanvasId, m_size.width(),
+        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) {
+  if (m_commitPromiseResolver) {
+    m_overdrawFrame = std::move(image);

[xlai (Olivia), 2016/12/21 19:54:25]

I'm wondering if you need to store the scriptState for the overdraw frame as
well. Will the scriptState ever change from frame to frame?

[Justin Novosad, 2016/12/21 21:15:40]

The overdraw frame does not generate a promise (the commit call re-returns the
previous promise), so we have no use for the scriptState.

+    m_overdrawFrameIsWebGLSoftwareRendering = isWebGLSoftwareRendering;
+  } else {
+    m_overdrawFrame = nullptr;
+    m_commitPromiseResolver = ScriptPromiseResolver::create(scriptState);
+    m_commitPromiseResolver->keepAliveWhilePending();
+    doCommit(std::move(image), isWebGLSoftwareRendering);
+  }
+  return m_commitPromiseResolver->promise();
+}
+
+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
+    // first and save the promise resolution for later.
+    doCommit(std::move(m_overdrawFrame),
+             m_overdrawFrameIsWebGLSoftwareRendering);

[xlai (Olivia), 2016/12/21 19:54:25]

Hmmmm...After you doCommit on the overdrawFrame, do you need to set
m_overdrawFrame to be null? If not, when the next beginFrame() starts, will you
repeat doCommit on the same frame?

[Justin Novosad, 2016/12/21 21:15:40]

std::move already takes care of that. 

+  } else if (m_commitPromiseResolver) {
+    m_commitPromiseResolver->resolve();

[xlai (Olivia), 2016/12/21 19:54:25]

So you only resolve the promiseResolver when there is no more overdraw frame?
(as this is else if)

[Justin Novosad, 2016/12/21 21:15:40]

Exactly. As long as there is a backlog, we do not need to be triggering new
frames.

+    m_commitPromiseResolver.clear();
+  }
+}
+
 ScriptPromise OffscreenCanvas::convertToBlob(ScriptState* scriptState,
                                              const ImageEncodeOptions& options,
                                              ExceptionState& exceptionState) {
@@ -271,6 +319,7 @@ ScriptPromise OffscreenCanvas::convertToBlob(ScriptState* scriptState,
 DEFINE_TRACE(OffscreenCanvas) {
   visitor->trace(m_context);
   visitor->trace(m_executionContext);
+  visitor->trace(m_commitPromiseResolver);
   EventTargetWithInlineData::trace(visitor);
 }