Submit Compositor Frame from OffscreenCanvas on main

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 b5f8febd6e7bfcd7f93520e4634a53b70aa5fd7f..7a41d2339b0f2af3a52184f7655089b4ea5a0d67 100644
--- a/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
+++ b/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp
@@ -9,7 +9,9 @@
 #include "core/html/canvas/CanvasRenderingContext.h"
 #include "core/html/canvas/CanvasRenderingContextFactory.h"
 #include "platform/graphics/ImageBuffer.h"
+#include "platform/graphics/OffscreenCanvasFrameDispatcherImpl.h"
 #include "wtf/MathExtras.h"
+#include "wtf/PtrUtil.h"
 #include <memory>
 
 namespace blink {
@@ -134,6 +136,13 @@ bool OffscreenCanvas::isPaintable() const
     return m_context->isPaintable();
 }
 
+OffscreenCanvasFrameDispatcher* OffscreenCanvas::getOrCreateFrameDispatcher()
+{
+    if (!m_frameDispatcher)
+        m_frameDispatcher = wrapUnique(new OffscreenCanvasFrameDispatcherImpl(m_clientId, m_localId, m_nonce));

[danakj, 2016/09/02 19:12:42]

why isn't this created when OffscreenCanvas is constructed? then the ordering
requirements would be more clear

[xlai (Olivia), 2016/09/02 20:10:06]

The FrameDispatcher is needed only when user intends to do commit(); after
transferring control from HTMLCanvasElement to OffscreenCanvas. There are use
cases when OffscreenCanvas is constructed but users do not intend to call
commit().

[danakj, 2016/09/02 21:51:00]

Oops I mean to change this comment but it got lost. What about creating this
when the surface ids are set instead? That happens when transferring control.

[danakj, 2016/09/02 21:53:50]

Though also from
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...
? Which I don't know when that happens.

[xlai (Olivia), 2016/09/02 23:41:28]

The code link you've given happens when OffscreenCanvas is transferred from main
thread
to worker thread, which may or may not happen depending on user's JavaScript
code.

Also, after users call HTMLCanvasElement.transferControlToOffscreen() in
JavaScript, they may
or may not call OffscreenCanvas2D.commit(). If they don't call commit(), then we
are doing an 
eager instantiation of OffscreenCanvasFrameDispatcher here.

I'm just feeling that a lazy instantiation of frameDispatcher is better--that I
only create new object
when I am sure that I'm gonna use it.

P.S. the sequence of JavaScript code that users write
1. var offscreenCanvas = htmlCanvas.transferControlToOffscreen();
2. worker.postMessage(offscreenCanvas, [offscreenCanvas]);
3. var ctx2d = offscreenCanvas.getContext("2d");
4. // do a lot of drawing actions on ctx2d...
5. ctx2d.commit();    <<-- Here is the point of time when m_frameDispatcher is
needed.

[danakj, 2016/09/03 00:40:42]

I see. I guess I mostly find it a bit confusing that this class has like 3
states:
- Constructed
- Constructed with surface id
- Constructed with surface id and dispatcher

I was poking at getting rid of the third one by merging it with the 2rd. Maybe
the 2nd could merge with the 3rd instead, or with the 1st. Anyhow.. something
for another day, this LGTM :)

+    return m_frameDispatcher.get();
+}
+
 DEFINE_TRACE(OffscreenCanvas)
 {
     visitor->trace(m_context);