Replace |have_recorded_draw_commands_| in OffscreenCanvasRenderingContext2D

The cause of this bug was the switch to a RecordPaintCanvas() on the
CanvasResourceProvider. Previously, a Clear Operation was done directly
on the skia canvas in OffscreenCanvas::GetOrCreateResourceProvider().

Now that CanvasResourceProvider::Canvas() returns a RecordPaintCanvas(),
a Clear Op is recorded, but not drawn to the backing skia canvas until
flush. The context's tracking boolean, |have_recorded_draw_commands_|,
was not updated to signal that a flush was needed.

This resulted in WritePixels()'s call to FlushRecording() earlying out
with no flush, despite the existence of draw operations.

By replacing |have_recorded_draw_commands_| with a check of the
recorder's actual DisplayItemList, we can avoid juggling the
|have_recorded_draw_commands_| boolean and can always correctly
determine if a flush is necessary.

Bug: 1059121
Change-Id: Ic511229a0cdb0ef3fe48dbd4d760066a0be2d10c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099299Reviewed-by: Aaron Krajeski <aaronhk@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Commit-Queue: Jonah Chin <jochin@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#751090}

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

@@ -424,6 +424,7 @@ CanvasResourceProvider* OffscreenCanvas::GetOrCreateResourceProvider() {
       base::UmaHistogramEnumeration("Blink.Canvas.ResourceProviderType",
                                     ResourceProvider()->GetType());
       ResourceProvider()->Clear();
+      DidDraw();
 
       if (needs_matrix_clip_restore_) {
         needs_matrix_clip_restore_ = false;

third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.cc

@@ -87,7 +87,7 @@ OffscreenCanvasRenderingContext2D::OffscreenCanvasRenderingContext2D(
 
   // Clear the background transparent or opaque.
   if (IsCanvas2DBufferValid()) {
-    GetOrCreateCanvasResourceProvider()->Clear();
+    GetCanvasResourceProvider()->Clear();
     DidDraw();
   }
 
@@ -120,13 +120,12 @@ void OffscreenCanvasRenderingContext2D::commit() {
 }
 
 void OffscreenCanvasRenderingContext2D::FlushRecording() {
-  if (!have_recorded_draw_commands_)
+  if (!GetCanvasResourceProvider() ||
+      !GetCanvasResourceProvider()->HasRecordedDrawOps())
     return;
 
   GetCanvasResourceProvider()->FlushCanvas();
   GetCanvasResourceProvider()->ReleaseLockedImages();
-
-  have_recorded_draw_commands_ = false;
 }
 
 void OffscreenCanvasRenderingContext2D::FinalizeFrame() {
@@ -272,7 +271,6 @@ cc::PaintCanvas* OffscreenCanvasRenderingContext2D::GetPaintCanvas() const {
 }
 
 void OffscreenCanvasRenderingContext2D::DidDraw() {
-  have_recorded_draw_commands_ = true;
   dirty_rect_for_commit_.setWH(Width(), Height());
   Host()->DidDraw();
   if (GetCanvasResourceProvider() && GetCanvasResourceProvider()->needs_flush())
@@ -280,7 +278,6 @@ void OffscreenCanvasRenderingContext2D::DidDraw() {
 }
 
 void OffscreenCanvasRenderingContext2D::DidDraw(const SkIRect& dirty_rect) {
-  have_recorded_draw_commands_ = true;
   dirty_rect_for_commit_.join(dirty_rect);
   Host()->DidDraw(SkRect::Make(dirty_rect_for_commit_));
   if (GetCanvasResourceProvider() && GetCanvasResourceProvider()->needs_flush())
@@ -340,7 +337,6 @@ bool OffscreenCanvasRenderingContext2D::WritePixels(
 
   DCHECK(IsPaintable());
   FinalizeFrame();
-  have_recorded_draw_commands_ = false;
 
   return offscreenCanvasForBinding()->ResourceProvider()->WritePixels(
       orig_info, pixels, row_bytes, x, y);

third_party/blink/renderer/modules/canvas/offscreencanvas2d/offscreen_canvas_rendering_context_2d.h

@@ -127,8 +127,6 @@ class MODULES_EXPORT OffscreenCanvasRenderingContext2D final
 
   bool PushFrame() override;
 
-  bool HasRecordedDrawCommands() { return have_recorded_draw_commands_; }
-
  protected:
   CanvasColorParams ColorParams() const override;
   bool WritePixels(const SkImageInfo& orig_info,
@@ -139,7 +137,6 @@ class MODULES_EXPORT OffscreenCanvasRenderingContext2D final
   void WillOverwriteCanvas() override;
 
  private:
-  bool have_recorded_draw_commands_;
   void FinalizeFrame() final;
   void FlushRecording();
 

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

@@ -1129,7 +1129,7 @@ GrContext* CanvasResourceProvider::GetGrContext() const {
 }
 
 void CanvasResourceProvider::FlushCanvas() {
-  if (!recorder_ || !recorder_->ListHasDrawOps())
+  if (!HasRecordedDrawOps())
     return;
   EnsureSkiaCanvas();
   last_recording_ = recorder_->finishRecordingAsPicture();
@@ -1156,7 +1156,7 @@ bool CanvasResourceProvider::WritePixels(const SkImageInfo& orig_info,
   TRACE_EVENT0("blink", "CanvasResourceProvider::WritePixels");
 
   DCHECK(IsValid());
-  DCHECK(!recorder_->ListHasDrawOps());
+  DCHECK(!HasRecordedDrawOps());
 
   EnsureSkiaCanvas();
 
@@ -1282,7 +1282,7 @@ void CanvasResourceProvider::SkipQueuedDrawCommands() {
   // so always update the |mode_| to discard the old copy of canvas content.
   mode_ = SkSurface::kDiscard_ContentChangeMode;
 
-  if (!recorder_ || !recorder_->ListHasDrawOps())
+  if (!HasRecordedDrawOps())
     return;
   recorder_->finishRecordingAsPicture();
   cc::PaintCanvas* canvas =
@@ -1306,4 +1306,8 @@ void CanvasResourceProvider::RestoreBackBuffer(const cc::PaintImage& image) {
   skia_canvas_->drawImage(image, 0, 0, &copy_paint);
 }
 
+bool CanvasResourceProvider::HasRecordedDrawOps() const {
+  return recorder_ && recorder_->ListHasDrawOps();
+}
+
 }  // namespace blink

third_party/blink/renderer/platform/graphics/canvas_resource_provider.h

@@ -218,6 +218,7 @@ class PLATFORM_EXPORT CanvasResourceProvider
   void RestoreBackBuffer(const cc::PaintImage&);
 
   ResourceProviderType GetType() const { return type_; }
+  bool HasRecordedDrawOps() const;
 
  protected:
   gpu::gles2::GLES2Interface* ContextGL() const;

third_party/blink/web_tests/fast/canvas/OffscreenCanvas-2d-imageData.html

+<!DOCTYPE html>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<canvas style="background: red"></canvas>
+<script>
+
+test(function() {
+  var canvas = document.querySelector('canvas');
+  var offscreenCtx = (new OffscreenCanvas(canvas.width, canvas.height)).getContext('2d');
+
+  var imageData = offscreenCtx.createImageData(canvas.width, canvas.height);
+  var data = imageData.data;
+
+  // Set all pixels to [0, 255, 0, 255]
+  for (var i = 0; i < data.length; i += 4) {
+    data[i + 1] = data[i + 3] = 255;
+  }
+
+  offscreenCtx.putImageData(imageData, 0, 0);
+
+  assert_array_equals(offscreenCtx.getImageData(0, 0, 1, 1).data, [0, 255, 0, 255]);
+}, "Verify that imageData is properly set");
+
+</script>