Add pre and post finalize functions to canvas hosts

For low latency canvases the order of events is important. The resource
must get single buffered before it is drawn. Because drawing is
something that the rendering context owns, it makes sense to split up
the parts of finalizing the frame on the host.

Bug: 1002523
Change-Id: Ie57965c4394d25f4197b4683bc238670edf1693d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848934Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704640}

third_party/blink/renderer/core/html/canvas/canvas_rendering_context.cc

@@ -127,7 +127,10 @@ void CanvasRenderingContext::DidProcessTask(
   // The end of a script task that drew content to the canvas is the point
   // at which the current frame may be considered complete.
   if (Host())
-    Host()->FinalizeFrame();
+    Host()->PreFinalizeFrame();
+  FinalizeFrame();
+  if (Host())
+    Host()->PostFinalizeFrame();
 }
 
 CanvasRenderingContext::ContextType CanvasRenderingContext::ContextTypeFromId(

third_party/blink/renderer/core/html/canvas/canvas_rendering_context.h

@@ -94,7 +94,7 @@ class CORE_EXPORT CanvasRenderingContext : public ScriptWrappable,
   virtual bool IsOriginTopLeft() const {
     // Canvas contexts have the origin of coordinates on the top left corner.
     // Accelerated resources (e.g. GPU textures) have their origin of
-    // coordinates in the uppper left corner.
+    // coordinates in the upper left corner.
     return !IsAccelerated();
   }
   virtual bool ShouldAntialias() const { return false; }

third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.h

@@ -46,7 +46,8 @@ class CORE_EXPORT CanvasRenderingContextHost : public CanvasResourceHost,
   virtual void DidDraw(const FloatRect& rect) = 0;
   virtual void DidDraw() = 0;
 
-  virtual void FinalizeFrame() = 0;
+  virtual void PreFinalizeFrame() = 0;
+  virtual void PostFinalizeFrame() = 0;
   virtual bool PushFrame(scoped_refptr<CanvasResource> frame,
                          const SkIRect& damage_rect) = 0;
   virtual bool OriginClean() const = 0;
@@ -96,7 +97,7 @@ class CORE_EXPORT CanvasRenderingContextHost : public CanvasResourceHost,
   CanvasColorParams ColorParams() const;
 
   // For deferred canvases this will have the side effect of drawing recorded
-  // commands in order to finalize the frame
+  // commands in order to finalize the frame.
   ScriptPromise convertToBlob(ScriptState*,
                               const ImageEncodeOptions*,
                               ExceptionState&);

third_party/blink/renderer/core/html/canvas/html_canvas_element.cc

@@ -393,43 +393,35 @@ void HTMLCanvasElement::DidDraw() {
   DidDraw(FloatRect(0, 0, Size().Width(), Size().Height()));
 }
 
-void HTMLCanvasElement::FinalizeFrame() {
-  TRACE_EVENT0("blink", "HTMLCanvasElement::FinalizeFrame");
+void HTMLCanvasElement::PreFinalizeFrame() {
   RecordCanvasSizeToUMA(size_);
 
-  // FinalizeFrame indicates the end of a script task that may have rendered
+  // PreFinalizeFrame indicates the end of a script task that may have rendered
   // into the canvas, now is a good time to unlock cache entries.
   auto* resource_provider = ResourceProvider();
   if (resource_provider)
     resource_provider->ReleaseLockedImages();
 
-  // Low-latency 2d canvases will produce their frames after the resource gets
-  // single buffered
-  if (context_ && !(Is2d() && LowLatencyEnabled()))
-    context_->FinalizeFrame();
-
+  // Low-latency 2d canvases produce their frames after the resource gets
+  // single buffered.
   if (LowLatencyEnabled() && !dirty_rect_.IsEmpty()) {
     if (GetOrCreateCanvasResourceProvider(kPreferAcceleration)) {
       const bool webgl_overlay_enabled =
           RuntimeEnabledFeatures::WebGLImageChromiumEnabled() ||
           context_->UsingSwapChain();
-      // TryEnableSingleBuffering() the first time we FinalizeFrame().
+      // TryEnableSingleBuffering() the first time we finalize a frame.
       if (!ResourceProvider()->IsSingleBuffered()) {
         ResourceProvider()->TryEnableSingleBuffering();
         if (Is3d() && webgl_overlay_enabled)
           context_->ProvideBackBufferToResourceProvider();
       }
+    }
+  }
+}
 
-      // TODO(aaronhk) This should just be a generic call to finalize frame
-      // but the pixel 2 bot hanging prevents it (crbug.com/1002946)
-      if (canvas2d_bridge_) {
-        canvas2d_bridge_->FlushRecording();
-      } else {
-        DCHECK(Is3d());
-        if (!webgl_overlay_enabled)
-          context_->PaintRenderingResultsToCanvas(kBackBuffer);
-      }
-
+void HTMLCanvasElement::PostFinalizeFrame() {
+  if (LowLatencyEnabled() && !dirty_rect_.IsEmpty()) {
+    if (GetOrCreateCanvasResourceProvider(kPreferAcceleration)) {
       const base::TimeTicks start_time = base::TimeTicks::Now();
       const scoped_refptr<CanvasResource> canvas_resource =
           ResourceProvider()->ProduceCanvasResource();

third_party/blink/renderer/core/html/canvas/html_canvas_element.h

@@ -185,7 +185,8 @@ class CORE_EXPORT HTMLCanvasElement final
 
   void DoDeferredPaintInvalidation();
 
-  void FinalizeFrame() override;
+  void PreFinalizeFrame() override;
+  void PostFinalizeFrame() override;
 
   CanvasResourceDispatcher* GetOrCreateResourceDispatcher() override;
 

third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h

@@ -111,7 +111,8 @@ class CORE_EXPORT OffscreenCanvas final
   }
 
   // CanvasRenderingContextHost implementation.
-  void FinalizeFrame() override {}
+  void PreFinalizeFrame() override {}
+  void PostFinalizeFrame() override {}
   void DetachContext() override { context_ = nullptr; }
   CanvasRenderingContext* RenderingContext() const override { return context_; }
 

third_party/blink/renderer/core/paint/html_canvas_painter_test.cc

@@ -85,7 +85,9 @@ TEST_P(HTMLCanvasPainterTestForCAP, Canvas2DLayerAppearsInLayerTree) {
   ASSERT_TRUE(element->IsAccelerated());
 
   // Force the page to paint.
-  element->FinalizeFrame();
+  element->PreFinalizeFrame();
+  context->FinalizeFrame();
+  element->PostFinalizeFrame();
   UpdateAllLifecyclePhasesForTest();
 
   // Fetch the layer associated with the <canvas>, and check that it was

third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc

@@ -672,6 +672,7 @@ scoped_refptr<StaticBitmapImage> blink::CanvasRenderingContext2D::GetImage(
 }
 
 void CanvasRenderingContext2D::FinalizeFrame() {
+  TRACE_EVENT0("blink", "CanvasRenderingContext2D::FinalizeFrame");
   if (canvas() && canvas()->GetCanvas2DLayerBridge())
     canvas()->GetCanvas2DLayerBridge()->FinalizeFrame();
   usage_counters_.num_frames_since_reset++;

third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d_test.cc

@@ -116,7 +116,9 @@ class CanvasRenderingContext2DTest : public ::testing::Test {
   }
   void DrawSomething() {
     CanvasElement().DidDraw();
-    CanvasElement().FinalizeFrame();
+    CanvasElement().PreFinalizeFrame();
+    Context2d()->FinalizeFrame();
+    CanvasElement().PostFinalizeFrame();
     // Grabbing an image forces a flush
     CanvasElement().Snapshot(kBackBuffer, kPreferAcceleration);
   }

third_party/blink/renderer/modules/canvas/htmlcanvas/html_canvas_element_module_test.cc

@@ -145,7 +145,9 @@ TEST_P(HTMLCanvasElementModuleTest, LowLatencyCanvasCompositorFrameOpacity) {
             EXPECT_NE(shared_quad_state_list.front()->are_contents_opaque,
                       context_alpha);
           })));
-  canvas_element().FinalizeFrame();
+  canvas_element().PreFinalizeFrame();
+  context_->FinalizeFrame();
+  canvas_element().PostFinalizeFrame();
   platform->RunUntilIdle();
 }
 

third_party/blink/renderer/modules/canvas/imagebitmap/image_bitmap_rendering_context_base.h

@@ -43,7 +43,7 @@ class MODULES_EXPORT ImageBitmapRenderingContextBase
   void SetIsHidden(bool) override {}
   bool isContextLost() const override { return false; }
   void SetImage(ImageBitmap*);
-  // The acceleration hing here is ignored as GetImage(AccelerationHint) only
+  // The acceleration hint here is ignored as GetImage(AccelerationHint) only
   // calls to image_layer_bridge->GetImage(), without giving it a hint
   scoped_refptr<StaticBitmapImage> GetImage(AccelerationHint) final;
   // This function resets the internal image resource to a image of the same

third_party/blink/renderer/modules/webgl/webgl_rendering_context_base.cc

@@ -1410,6 +1410,12 @@ void WebGLRenderingContextBase::FinalizeFrame() {
   if (GetDrawingBuffer() && GetDrawingBuffer()->UsingSwapChain())
     GetDrawingBuffer()->PresentSwapChain();
   marked_canvas_dirty_ = false;
+
+  // For low-latency canvases
+  const bool webgl_overlay_enabled =
+      RuntimeEnabledFeatures::WebGLImageChromiumEnabled() || UsingSwapChain();
+  if (canvas() && canvas()->LowLatencyEnabled() && !webgl_overlay_enabled)
+    PaintRenderingResultsToCanvas(kBackBuffer);
 }
 
 void WebGLRenderingContextBase::OnErrorMessage(const char* message,

third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc

@@ -732,11 +732,9 @@ void Canvas2DLayerBridge::FinalizeFrame() {
   if (!GetOrCreateResourceProvider(kPreferAcceleration))
     return;
 
+  FlushRecording();
   ++frames_since_last_commit_;
   if (frames_since_last_commit_ >= 2) {
-    // TODO(aaronhk) Ideally we'd want to call FlushRecording() here, but this
-    // causes webview to hang for pixel 2. See crbug.com/1002946
-    ResourceProvider()->FlushSkia();
     if (IsAccelerated() && !rate_limiter_) {
       // Make sure the GPU is never more than two animation frames behind.
       constexpr unsigned kMaxCanvasAnimationBacklog = 2;