Reland: Complete the plumbing of CanvasResource through CanvasResourceDispatcher

This reverts commit 56151bcd.

This reland fixes the MSAN error from the original CL by making sure the
filter_quality field of CanvasResource is always initialized

TBR=zmo@chromium.org

Bug: 788439
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7dd29f123cbfd75598005532fc5b2135e6bfebc9
Reviewed-on: https://chromium-review.googlesource.com/1147317
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577294}

content/test/gpu/gpu_tests/pixel_expectations.py

@@ -45,6 +45,10 @@ class PixelExpectations(GpuTestExpectations):
     self.Fail('Pixel_ScissorTestWithPreserveDrawingBuffer',
         ['android'], bug=521588)
 
+    # Tests crashing on marshmallow bot
+    self.Fail('Pixel_CanvasLowLatency2D', ['android'], bug=865957)
+    self.Fail('Pixel_CanvasUnacceleratedLowLatency2D', ['android'], bug=865957)
+
     # TODO(vmiura) check / generate reference images for Android devices
     self.Fail('Pixel_SolidColorBackground', ['mac', 'android'], bug=624256)
 

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

@@ -420,14 +420,15 @@ void HTMLCanvasElement::FinalizeFrame() {
         GetOrCreateCanvasResourceProvider(kPreferAcceleration)) {
       // Push a frame
       base::TimeTicks start_time = WTF::CurrentTimeTicks();
-      scoped_refptr<StaticBitmapImage> image =
-          canvas2d_bridge_->NewImageSnapshot(kPreferAcceleration);
+      scoped_refptr<CanvasResource> canvas_resource =
+          ResourceProvider()->ProduceFrame();
       FloatRect src_rect(0, 0, Size().Width(), Size().Height());
       dirty_rect_.Intersect(src_rect);
       IntRect int_dirty = EnclosingIntRect(dirty_rect_);
       SkIRect damage_rect = SkIRect::MakeXYWH(
           int_dirty.X(), int_dirty.Y(), int_dirty.Width(), int_dirty.Height());
-      frame_dispatcher_->DispatchFrameSync(image, start_time, damage_rect);
+      frame_dispatcher_->DispatchFrameSync(std::move(canvas_resource),
+                                           start_time, damage_rect);
       (void)start_time;
       (void)damage_rect;
       dirty_rect_ = FloatRect();

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

@@ -51,7 +51,8 @@ void OffscreenCanvas::Commit(scoped_refptr<CanvasResource> canvas_resource,
   base::TimeTicks commit_start_time = WTF::CurrentTimeTicks();
   current_frame_damage_rect_.join(damage_rect);
   GetOrCreateResourceDispatcher()->DispatchFrameSync(
-      canvas_resource->Bitmap(), commit_start_time, current_frame_damage_rect_);
+      std::move(canvas_resource), commit_start_time,
+      current_frame_damage_rect_);
   current_frame_damage_rect_ = SkIRect::MakeEmpty();
 }
 
@@ -390,8 +391,9 @@ void OffscreenCanvas::PushFrame(scoped_refptr<CanvasResource> canvas_resource,
   if (current_frame_damage_rect_.isEmpty())
     return;
   base::TimeTicks commit_start_time = WTF::CurrentTimeTicks();
-  GetOrCreateResourceDispatcher()->DispatchFrame(
-      canvas_resource->Bitmap(), commit_start_time, current_frame_damage_rect_);
+  GetOrCreateResourceDispatcher()->DispatchFrame(std::move(canvas_resource),
+                                                 commit_start_time,
+                                                 current_frame_damage_rect_);
   current_frame_damage_rect_ = SkIRect::MakeEmpty();
 }
 

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

@@ -127,14 +127,9 @@ void CanvasResourceDispatcher::PostImageToPlaceholder(
 }
 
 void CanvasResourceDispatcher::DispatchFrameSync(
-    scoped_refptr<StaticBitmapImage> image,
+    scoped_refptr<CanvasResource> canvas_resource,
     base::TimeTicks commit_start_time,
     const SkIRect& damage_rect) {
-  scoped_refptr<CanvasResource> canvas_resource = CanvasResourceBitmap::Create(
-      std::move(image),
-      nullptr,  // Resource provider not specified -> recycling will not work
-      kLow_SkFilterQuality, CanvasColorParams());
-
   viz::CompositorFrame frame;
   if (!PrepareFrame(std::move(canvas_resource), commit_start_time, damage_rect,
                     &frame))
@@ -149,14 +144,9 @@ void CanvasResourceDispatcher::DispatchFrameSync(
 }
 
 void CanvasResourceDispatcher::DispatchFrame(
-    scoped_refptr<StaticBitmapImage> image,
+    scoped_refptr<CanvasResource> canvas_resource,
     base::TimeTicks commit_start_time,
     const SkIRect& damage_rect) {
-  scoped_refptr<CanvasResource> canvas_resource = CanvasResourceBitmap::Create(
-      std::move(image),
-      nullptr,  // Resource provider not specified -> recycling will not work
-      kLow_SkFilterQuality, CanvasColorParams());
-
   viz::CompositorFrame frame;
   if (!PrepareFrame(std::move(canvas_resource), commit_start_time, damage_rect,
                     &frame))
@@ -229,38 +219,24 @@ bool CanvasResourceDispatcher::PrepareFrame(
     if (SharedGpuContext::IsGpuCompositingEnabled()) {
       // Case 1: both canvas and compositor are gpu accelerated.
       commit_type = kCommitGPUCanvasGPUCompositing;
-      offscreen_canvas_resource_provider_
-          ->SetTransferableResourceToStaticBitmapImage(&resource,
-                                                       canvas_resource);
       yflipped = true;
     } else {
       // Case 2: canvas is accelerated but gpu compositing is disabled.
       commit_type = kCommitGPUCanvasSoftwareCompositing;
-      offscreen_canvas_resource_provider_
-          ->SetTransferableResourceToSharedBitmap(resource,
-                                                  canvas_resource->Bitmap());
     }
   } else {
     if (SharedGpuContext::IsGpuCompositingEnabled()) {
       // Case 3: canvas is not gpu-accelerated, but compositor is.
       commit_type = kCommitSoftwareCanvasGPUCompositing;
-      scoped_refptr<CanvasResource> accelerated_resource =
-          canvas_resource->MakeAccelerated(
-              SharedGpuContext::ContextProviderWrapper());
-      if (!accelerated_resource)
-        return false;
-      offscreen_canvas_resource_provider_
-          ->SetTransferableResourceToStaticBitmapImage(&resource,
-                                                       accelerated_resource);
     } else {
       // Case 4: both canvas and compositor are not gpu accelerated.
       commit_type = kCommitSoftwareCanvasSoftwareCompositing;
-      offscreen_canvas_resource_provider_
-          ->SetTransferableResourceToSharedBitmap(resource,
-                                                  canvas_resource->Bitmap());
     }
   }
 
+  offscreen_canvas_resource_provider_->SetTransferableResource(&resource,
+                                                               canvas_resource);
+
   commit_type_histogram.Count(commit_type);
 
   PostImageToPlaceholderIfNotBlocked(
@@ -477,12 +453,14 @@ void CanvasResourceDispatcher::Reshape(const IntSize& size) {
 void CanvasResourceDispatcher::DidAllocateSharedBitmap(
     mojo::ScopedSharedBufferHandle buffer,
     ::gpu::mojom::blink::MailboxPtr id) {
-  sink_->DidAllocateSharedBitmap(std::move(buffer), std::move(id));
+  if (sink_)
+    sink_->DidAllocateSharedBitmap(std::move(buffer), std::move(id));
 }
 
 void CanvasResourceDispatcher::DidDeleteSharedBitmap(
     ::gpu::mojom::blink::MailboxPtr id) {
-  sink_->DidDeleteSharedBitmap(std::move(id));
+  if (sink_)
+    sink_->DidDeleteSharedBitmap(std::move(id));
 }
 
 }  // namespace blink

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

@@ -47,11 +47,11 @@ class PLATFORM_EXPORT CanvasResourceDispatcher
   void SetSuspendAnimation(bool);
   bool NeedsBeginFrame() const { return needs_begin_frame_; }
   bool IsAnimationSuspended() const { return suspend_animation_; }
-  void DispatchFrame(scoped_refptr<StaticBitmapImage>,
+  void DispatchFrame(scoped_refptr<CanvasResource>,
                      base::TimeTicks commit_start_time,
                      const SkIRect& damage_rect);
   void ReclaimResource(viz::ResourceId);
-  void DispatchFrameSync(scoped_refptr<StaticBitmapImage>,
+  void DispatchFrameSync(scoped_refptr<CanvasResource>,
                          base::TimeTicks commit_start_time,
                          const SkIRect& damage_rect);
 

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

@@ -179,7 +179,7 @@ class PLATFORM_EXPORT CanvasResourceProvider
   std::unique_ptr<cc::SkiaPaintCanvas> canvas_;
   mutable sk_sp<SkSurface> surface_;  // mutable for lazy init
   std::unique_ptr<SkCanvas> xform_canvas_;
-  SkFilterQuality filter_quality_;
+  SkFilterQuality filter_quality_ = kLow_SkFilterQuality;
 
   const cc::PaintImage::Id snapshot_paint_image_id_;
   cc::PaintImage::ContentId snapshot_paint_image_content_id_ =

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

@@ -10,6 +10,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/platform/graphics/canvas_resource.h"
+#include "third_party/blink/renderer/platform/graphics/canvas_resource_provider.h"
 #include "third_party/skia/include/core/SkSurface.h"
 
 using testing::_;
@@ -48,6 +49,13 @@ class CanvasResourceDispatcherTest : public testing::Test {
  protected:
   CanvasResourceDispatcherTest() {
     dispatcher_ = std::make_unique<MockCanvasResourceDispatcher>();
+    resource_provider_ = CanvasResourceProvider::Create(
+        IntSize(10, 10),
+        CanvasResourceProvider::kSoftwareCompositedResourceUsage,
+        nullptr,  // context_provider_wrapper
+        0,        // msaa_sample_count
+        CanvasColorParams(), CanvasResourceProvider::kDefaultPresentationMode,
+        dispatcher_->GetWeakPtr());
   }
 
   MockCanvasResourceDispatcher* Dispatcher() { return dispatcher_.get(); }
@@ -55,13 +63,12 @@ class CanvasResourceDispatcherTest : public testing::Test {
  private:
   scoped_refptr<StaticBitmapImage> PrepareStaticBitmapImage();
   std::unique_ptr<MockCanvasResourceDispatcher> dispatcher_;
+  std::unique_ptr<CanvasResourceProvider> resource_provider_;
 };
 
 void CanvasResourceDispatcherTest::DispatchOneFrame() {
-  sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(10, 10);
-  dispatcher_->DispatchFrame(
-      StaticBitmapImage::Create(surface->makeImageSnapshot()),
-      base::TimeTicks(), SkIRect::MakeEmpty());
+  dispatcher_->DispatchFrame(resource_provider_->ProduceFrame(),
+                             base::TimeTicks(), SkIRect::MakeEmpty());
 }
 
 TEST_F(CanvasResourceDispatcherTest, PlaceholderRunsNormally) {

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

@@ -4,28 +4,9 @@
 
 #include "third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h"
 
-#include "base/memory/shared_memory.h"
-#include "base/numerics/checked_math.h"
-#include "components/viz/common/resources/bitmap_allocation.h"
-#include "components/viz/common/resources/shared_bitmap.h"
 #include "components/viz/common/resources/single_release_callback.h"
-#include "gpu/command_buffer/client/gles2_interface.h"
-#include "gpu/command_buffer/common/capabilities.h"
-#include "services/viz/public/interfaces/compositing/compositor_frame_sink.mojom-blink.h"
-#include "third_party/blink/public/platform/platform.h"
-#include "third_party/blink/public/platform/web_graphics_context_3d_provider.h"
 #include "third_party/blink/renderer/platform/graphics/canvas_resource.h"
 #include "third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.h"
-#include "third_party/blink/renderer/platform/graphics/gpu/shared_gpu_context.h"
-#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
-#include "third_party/blink/renderer/platform/wtf/typed_arrays/array_buffer.h"
-#include "third_party/blink/renderer/platform/wtf/typed_arrays/uint8_array.h"
-#include "third_party/khronos/GLES2/gl2.h"
-#include "third_party/khronos/GLES2/gl2ext.h"
-#include "third_party/skia/include/core/SkColor.h"
-#include "third_party/skia/include/core/SkImage.h"
-#include "third_party/skia/include/core/SkSwizzle.h"
-#include "third_party/skia/include/gpu/GrContext.h"
 
 namespace blink {
 
@@ -46,68 +27,9 @@ OffscreenCanvasResourceProvider::CreateOrRecycleFrameResource() {
   return std::make_unique<FrameResource>();
 }
 
-void OffscreenCanvasResourceProvider::SetTransferableResourceToSharedBitmap(
-    viz::TransferableResource& resource,
-    scoped_refptr<StaticBitmapImage> image) {
-  std::unique_ptr<FrameResource> frame_resource =
-      CreateOrRecycleFrameResource();
-  if (!frame_resource->shared_memory) {
-    frame_resource->provider = this;
-    frame_resource->shared_bitmap_id = viz::SharedBitmap::GenerateId();
-    frame_resource->shared_memory =
-        viz::bitmap_allocation::AllocateMappedBitmap(gfx::Size(width_, height_),
-                                                     resource.format);
-    frame_dispatcher_->DidAllocateSharedBitmap(
-        viz::bitmap_allocation::DuplicateAndCloseMappedBitmap(
-            frame_resource->shared_memory.get(), gfx::Size(width_, height_),
-            resource.format),
-        SharedBitmapIdToGpuMailboxPtr(frame_resource->shared_bitmap_id));
-  }
-  void* pixels = frame_resource->shared_memory->memory();
-  DCHECK(pixels);
-  // When |image| is texture backed, this function does a GPU readback which is
-  // required.
-  sk_sp<SkImage> sk_image = image->PaintImageForCurrentFrame().GetSkImage();
-  if (sk_image->bounds().isEmpty())
-    return;
-  SkImageInfo image_info = SkImageInfo::Make(
-      width_, height_, kN32_SkColorType,
-      image->IsPremultiplied() ? kPremul_SkAlphaType : kUnpremul_SkAlphaType,
-      sk_image->refColorSpace());
-  if (image_info.isEmpty())
-    return;
-
-  if (RuntimeEnabledFeatures::CanvasColorManagementEnabled()) {
-    image_info = image_info.makeColorType(sk_image->colorType());
-  }
-
-  // TODO(junov): Optimize to avoid copying pixels for non-texture-backed
-  // sk_image. See crbug.com/651456.
-  bool read_pixels_successful =
-      sk_image->readPixels(image_info, pixels, image_info.minRowBytes(), 0, 0);
-  DCHECK(read_pixels_successful);
-  if (!read_pixels_successful)
-    return;
-  resource.mailbox_holder.mailbox = frame_resource->shared_bitmap_id;
-  resource.mailbox_holder.texture_target = 0;
-  resource.is_software = true;
-  resource.id = next_resource_id_;
-  resource.format = viz::ResourceFormat::RGBA_8888;
-  resource.size = gfx::Size(width_, height_);
-  // This indicates the filtering on the resource inherently, not the desired
-  // filtering effect on the quad.
-  resource.filter = GL_NEAREST;
-  // TODO(crbug.com/646022): making this overlay-able.
-  resource.is_overlay_candidate = false;
-
-  resources_.insert(next_resource_id_, std::move(frame_resource));
-}
-
-void OffscreenCanvasResourceProvider::
-    SetTransferableResourceToStaticBitmapImage(
-        viz::TransferableResource* out_resource,
-        scoped_refptr<CanvasResource> image) {
-  DCHECK(image->IsAccelerated());
+void OffscreenCanvasResourceProvider::SetTransferableResource(
+    viz::TransferableResource* out_resource,
+    scoped_refptr<CanvasResource> image) {
   DCHECK(image->IsValid());
 
   std::unique_ptr<FrameResource> frame_resource =
@@ -166,10 +88,6 @@ void OffscreenCanvasResourceProvider::ReclaimResourceInternal(
 OffscreenCanvasResourceProvider::FrameResource::~FrameResource() {
   if (release_callback)
     release_callback->Run(sync_token, is_lost);
-  if (provider && !shared_bitmap_id.IsZero()) {
-    provider->frame_dispatcher_->DidDeleteSharedBitmap(
-        SharedBitmapIdToGpuMailboxPtr(shared_bitmap_id));
-  }
 }
 
 }  // namespace blink

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

@@ -9,10 +9,6 @@
 #include "components/viz/common/resources/transferable_resource.h"
 #include "third_party/blink/renderer/platform/graphics/static_bitmap_image.h"
 
-namespace base {
-class SharedMemory;
-}
-
 namespace viz {
 class SingleReleaseCallback;
 namespace mojom {
@@ -38,11 +34,8 @@ class PLATFORM_EXPORT OffscreenCanvasResourceProvider {
 
   ~OffscreenCanvasResourceProvider();
 
-  void SetTransferableResourceToSharedBitmap(viz::TransferableResource&,
-                                             scoped_refptr<StaticBitmapImage>);
-  void SetTransferableResourceToStaticBitmapImage(
-      viz::TransferableResource* out_resource,
-      scoped_refptr<CanvasResource>);
+  void SetTransferableResource(viz::TransferableResource* out_resource,
+                               scoped_refptr<CanvasResource>);
 
   void ReclaimResource(unsigned resource_id);
   void ReclaimResources(const WTF::Vector<viz::ReturnedResource>& resources);
@@ -63,12 +56,6 @@ class PLATFORM_EXPORT OffscreenCanvasResourceProvider {
     // TODO(junov):  What does this do?
     bool spare_lock = true;
 
-    // Holds the backing for a software-backed resource.
-    std::unique_ptr<base::SharedMemory> shared_memory;
-    // The id given to  the display compositor to display a software-backed
-    // resource.
-    viz::SharedBitmapId shared_bitmap_id;
-
     // Back-pointer to the OffscreenCanvasResourceProvider. FrameResource does
     // not outlive the provider.
     OffscreenCanvasResourceProvider* provider = nullptr;