Synchronize OffscreenCanvas content with the placeholder canvas

third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp

Index: third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp b/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
index 970e6a6f35b3090e2c69ff0de021474c53bf8a36..86f8611d7b0b27c0fc11a7f456699b2de269a839 100644
--- a/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
+++ b/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
@@ -7,7 +7,10 @@
 #include "cc/output/compositor_frame.h"
 #include "cc/quads/texture_draw_quad.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
+#include "platform/CrossThreadFunctional.h"
 #include "platform/Histogram.h"
+#include "platform/WebTaskRunner.h"
+#include "platform/graphics/OffscreenCanvasPlaceholder.h"
 #include "platform/graphics/gpu/SharedGpuContext.h"
 #include "public/platform/InterfaceProvider.h"
 #include "public/platform/Platform.h"
@@ -29,6 +32,7 @@ OffscreenCanvasFrameDispatcherImpl::OffscreenCanvasFrameDispatcherImpl(
     uint32_t sinkId,
     uint32_t localId,
     uint64_t nonce,
+    int canvasId,
     int width,
     int height)
     : m_surfaceId(cc::FrameSinkId(clientId, sinkId),
@@ -36,7 +40,8 @@ OffscreenCanvasFrameDispatcherImpl::OffscreenCanvasFrameDispatcherImpl(
       m_width(width),
       m_height(height),
       m_nextResourceId(1u),
-      m_binding(this) {
+      m_binding(this),
+      m_placeholderCanvasId(canvasId) {
   DCHECK(!m_sink.is_bound());
   mojom::blink::OffscreenCanvasCompositorFrameSinkProviderPtr provider;
   Platform::current()->interfaceProvider()->getInterface(
@@ -146,6 +151,25 @@ void OffscreenCanvasFrameDispatcherImpl::
   m_cachedImages.add(m_nextResourceId, std::move(image));
 }
 
+namespace {
+
+void updatePlaceholderImage(WeakPtr<OffscreenCanvasFrameDispatcher> dispatcher,
+                            std::unique_ptr<WebTaskRunner> taskRunner,
+                            int placeholderCanvasId,
+                            RefPtr<blink::Image> image,
+                            unsigned resourceId) {
+  DCHECK(isMainThread());
+  OffscreenCanvasPlaceholder* placeholderCanvas =
+      OffscreenCanvasPlaceholder::getPlaceholderById(placeholderCanvasId);
+  if (placeholderCanvas) {
+    placeholderCanvas->setPlaceholderFrame(std::move(image),
+                                           std::move(dispatcher),
+                                           std::move(taskRunner), resourceId);
+  }
+}
+
+}  // namespace
+
 void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
     RefPtr<StaticBitmapImage> image,
     double commitStartTime,
@@ -211,6 +235,20 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
       setTransferableResourceToSharedBitmap(resource, image);
     }
   }
+
+  // After this point, |image| can only be used on the main thread, until
+  // it is returned.
+  image->transfer();

[xlai (Olivia), 2016/11/11 16:28:33]

In the case of gpu-accelerated offscreencanvas, this image is kept as a resource
in OffscreenCanvasFrameDispatcher in worker. I'm not sure if you transferring it
to main thread here will cause any trouble.

[Justin Novosad, 2016/11/11 22:09:00]

The idea is that We don't need to use this snapshot on the worker except for
when comes the time to delete the resource. So this should be fine.
Except that I was missing the reciprocal transfer in
OffscreenCanvasPlaceholder::releasePlaceholderFrame.  That is fixed now.

+  std::unique_ptr<WebTaskRunner> dispatcherTaskRunner =
+      Platform::current()->currentThread()->getWebTaskRunner()->clone();
+
+  Platform::current()->mainThread()->getWebTaskRunner()->postTask(

[xlai (Olivia), 2016/11/11 16:28:33]

This dispatchFrame() is invoked in every commit(). Whilst I understand that you
need to refresh the new frame and resource id for every commit(), you may end up
posting certain duplicate information of (dispatcherTaskRunner,
this->createWeakPtr()) to the main thread repeatedly. This is an overhead that
may become more and more painful when there are lots of commit().

A better place to send the dispatcherTaskRunner and this->createWeakPtr() should
be in OffscreenCanvas::getContext(), which is guaranteed to be invoked once in a
lifetime of an OffscreenCanvas.

[Justin Novosad, 2016/11/11 22:09:00]

I tried that and it is actually more complicated with no real reduction in
overhead.  The problem is that if OffscreenCanvasPlaceholder retains a
long-lived WeakPtr, then it can't std::move the weakptr when posting the call to
releaseFrameToDispatcher.  To solve that, you'd have to pass a weakPtr to the
OffscreenCanvasPlaceholder so that we can access m_frameDispatcherTaskRunner. 
And we're right back where we were, creating a weakPtr at every frame, except
that now, we've also added a weakptr factory on OffscreenCanvasPlaceholder.

+      BLINK_FROM_HERE,
+      crossThreadBind(updatePlaceholderImage, this->createWeakPtr(),
+                      passed(std::move(dispatcherTaskRunner)),
+                      m_placeholderCanvasId, std::move(image), resource.id));
+  m_spareResourceLocks.add(m_nextResourceId);
+
   commitTypeHistogram.count(commitType);
 
   m_nextResourceId++;
@@ -238,6 +276,10 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
   frame.render_pass_list.push_back(std::move(pass));
 
   double elapsedTime = WTF::monotonicallyIncreasingTime() - commitStartTime;
+
+  // TODO(crbug.com/663916): The off-main-thread metrics are commented-out
+  // because they cause thread check errors (static variable accessed in many
+  // threads)
   switch (commitType) {
     case CommitGPUCanvasGPUCompositing:
       if (isMainThread()) {
@@ -246,13 +288,13 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
             ("Blink.Canvas.OffscreenCommit.GPUCanvasGPUCompositingMain", 0,
              10000000, 50));
         commitGPUCanvasGPUCompositingMainTimer.count(elapsedTime * 1000000.0);
-      } else {
+      } /* else {
         DEFINE_STATIC_LOCAL(
             CustomCountHistogram, commitGPUCanvasGPUCompositingWorkerTimer,
             ("Blink.Canvas.OffscreenCommit.GPUCanvasGPUCompositingWorker", 0,
              10000000, 50));
         commitGPUCanvasGPUCompositingWorkerTimer.count(elapsedTime * 1000000.0);
-      }
+      } */
       break;
     case CommitGPUCanvasSoftwareCompositing:
       if (isMainThread()) {
@@ -262,14 +304,14 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
              10000000, 50));
         commitGPUCanvasSoftwareCompositingMainTimer.count(elapsedTime *
                                                           1000000.0);
-      } else {
+      } /* else {
         DEFINE_STATIC_LOCAL(
             CustomCountHistogram, commitGPUCanvasSoftwareCompositingWorkerTimer,
             ("Blink.Canvas.OffscreenCommit.GPUCanvasSoftwareCompositingWorker",
              0, 10000000, 50));
         commitGPUCanvasSoftwareCompositingWorkerTimer.count(elapsedTime *
                                                             1000000.0);
-      }
+      } */
       break;
     case CommitSoftwareCanvasGPUCompositing:
       if (isMainThread()) {
@@ -279,14 +321,14 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
              10000000, 50));
         commitSoftwareCanvasGPUCompositingMainTimer.count(elapsedTime *
                                                           1000000.0);
-      } else {
+      } /* else {
         DEFINE_STATIC_LOCAL(
             CustomCountHistogram, commitSoftwareCanvasGPUCompositingWorkerTimer,
             ("Blink.Canvas.OffscreenCommit.SoftwareCanvasGPUCompositingWorker",
              0, 10000000, 50));
         commitSoftwareCanvasGPUCompositingWorkerTimer.count(elapsedTime *
                                                             1000000.0);
-      }
+      } */
       break;
     case CommitSoftwareCanvasSoftwareCompositing:
       if (isMainThread()) {
@@ -297,7 +339,7 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
                              0, 10000000, 50));
         commitSoftwareCanvasSoftwareCompositingMainTimer.count(elapsedTime *
                                                                1000000.0);
-      } else {
+      } /* else {
         DEFINE_STATIC_LOCAL(CustomCountHistogram,
                             commitSoftwareCanvasSoftwareCompositingWorkerTimer,
                             ("Blink.Canvas.OffscreenCommit."
@@ -305,7 +347,7 @@ void OffscreenCanvasFrameDispatcherImpl::dispatchFrame(
                              0, 10000000, 50));
         commitSoftwareCanvasSoftwareCompositingWorkerTimer.count(elapsedTime *
                                                                  1000000.0);
-      }
+      } */
       break;
     case OffscreenCanvasCommitTypeCount:
       NOTREACHED();
@@ -327,10 +369,24 @@ void OffscreenCanvasFrameDispatcherImpl::ReclaimResources(
     RefPtr<StaticBitmapImage> image = m_cachedImages.get(resource.id);
     if (image)
       image->updateSyncToken(resource.sync_token);
-    m_cachedImages.remove(resource.id);
-    m_sharedBitmaps.remove(resource.id);
-    m_cachedTextureIds.remove(resource.id);
+    reclaimResource(resource.id);
+  }
+}
+
+void OffscreenCanvasFrameDispatcherImpl::reclaimResource(unsigned resourceId) {
+  // An image resource needs to be returned by both the
+  // CompositorFrameSink and the HTMLCanvasElement. These
+  // events can happen in any order.  The first of the two
+  // to return a given resource will result in the spare
+  // resource lock being lifted, and the second will delete
+  // the resource for real.
+  if (m_spareResourceLocks.contains(resourceId)) {
+    m_spareResourceLocks.remove(resourceId);
+    return;
   }
+  m_cachedImages.remove(resourceId);
+  m_sharedBitmaps.remove(resourceId);
+  m_cachedTextureIds.remove(resourceId);
 }
 
 bool OffscreenCanvasFrameDispatcherImpl::verifyImageSize(