Submit Compositor Frame from OffscreenCanvas on main

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 "core/dom/ExceptionCode.h"
 #include "core/html/canvas/CanvasContextCreationAttributes.h"
 #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 {
 
 OffscreenCanvas::OffscreenCanvas(const IntSize& size)
     : m_size(size)
     , m_originClean(true)
 {
 }
 
@@ skipping 104 matching lines @@
     return m_originClean && !m_disableReadingFromCanvas;
 }
 
 bool OffscreenCanvas::isPaintable() const
 {
     if (!m_context)
         return ImageBuffer::canCreateImageBuffer(m_size);
     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);
 }
 
 } // namespace blink