Fold OffscreenCanvasResourceProvider into CanvasResourceDispatcher

OffscreenCanvasResourceProvider is only used for accountancy of
viz::{Transferable/Returned}Resource; moreover it's not only used
for Offscreen canvases (but also for e.g. lowLatency ones). This
CL folds the logic into its only client, CanvasResourceDispatcher.

Also, the resource recycling inside OffscreenCRP is removed. This
is (all) inspired by junov@'s crrev.com/c/1103011 and follows up
on crrev.com/c/1167474.

CQ-DEPEND=CL:1169829

Bug: 839970
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ifed24992a6aded5c48f5e808cd61986906b64f7a
Reviewed-on: https://chromium-review.googlesource.com/1167907
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582217}

third_party/blink/renderer/platform/BUILD.gn

@@ -987,8 +987,6 @@ jumbo_component("platform") {
     "graphics/mailbox_texture_holder.h",
     "graphics/offscreen_canvas_placeholder.cc",
     "graphics/offscreen_canvas_placeholder.h",
-    "graphics/offscreen_canvas_resource_provider.cc",
-    "graphics/offscreen_canvas_resource_provider.h",
     "graphics/paint/clip_paint_property_node.cc",
     "graphics/paint/clip_paint_property_node.h",
     "graphics/paint/cull_rect.cc",

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

@@ -9,6 +9,7 @@
 #include "components/viz/common/quads/compositor_frame.h"
 #include "components/viz/common/quads/texture_draw_quad.h"
 #include "components/viz/common/resources/resource_format.h"
+#include "components/viz/common/resources/single_release_callback.h"
 #include "third_party/blink/public/platform/interface_provider.h"
 #include "third_party/blink/public/platform/modules/frame_sinks/embedded_frame_sink.mojom-blink.h"
 #include "third_party/blink/public/platform/platform.h"
@@ -28,6 +29,21 @@ enum {
   kMaxUnreclaimedPlaceholderFrames = 3,
 };
 
+struct CanvasResourceDispatcher::FrameResource {
+  FrameResource() = default;
+  ~FrameResource() {
+    if (release_callback)
+      release_callback->Run(sync_token, is_lost);
+  }
+
+  // TODO(junov):  What does this do?
+  bool spare_lock = true;
+
+  std::unique_ptr<viz::SingleReleaseCallback> release_callback;
+  gpu::SyncToken sync_token;
+  bool is_lost = false;
+};
+
 CanvasResourceDispatcher::CanvasResourceDispatcher(
     CanvasResourceDispatcherClient* client,
     uint32_t client_id,
@@ -43,21 +59,20 @@ CanvasResourceDispatcher::CanvasResourceDispatcher(
       num_unreclaimed_frames_posted_(0),
       client_(client),
       weak_ptr_factory_(this) {
-  if (frame_sink_id_.is_valid()) {
-    // Only frameless canvas pass an invalid frame sink id; we don't create
-    // mojo channel for this special case.
-    DCHECK(!sink_.is_bound());
-    mojom::blink::EmbeddedFrameSinkProviderPtr provider;
-    Platform::Current()->GetInterfaceProvider()->GetInterface(
-        mojo::MakeRequest(&provider));
-    DCHECK(provider);
-
-    binding_.Bind(mojo::MakeRequest(&client_ptr_));
-    provider->CreateCompositorFrameSink(frame_sink_id_, std::move(client_ptr_),
-                                        mojo::MakeRequest(&sink_));
-  }
-  offscreen_canvas_resource_provider_ =
-      std::make_unique<OffscreenCanvasResourceProvider>();
+  // Frameless canvas pass an invalid |frame_sink_id_|; don't create mojo
+  // channel for this special case.
+  if (!frame_sink_id_.is_valid())
+    return;
+
+  DCHECK(!sink_.is_bound());
+  mojom::blink::EmbeddedFrameSinkProviderPtr provider;
+  Platform::Current()->GetInterfaceProvider()->GetInterface(
+      mojo::MakeRequest(&provider));
+  DCHECK(provider);
+
+  binding_.Bind(mojo::MakeRequest(&client_ptr_));
+  provider->CreateCompositorFrameSink(frame_sink_id_, std::move(client_ptr_),
+                                      mojo::MakeRequest(&sink_));
 }
 
 CanvasResourceDispatcher::~CanvasResourceDispatcher() = default;
@@ -86,7 +101,7 @@ void CanvasResourceDispatcher::PostImageToPlaceholderIfNotBlocked(
     scoped_refptr<CanvasResource> image,
     viz::ResourceId resource_id) {
   if (placeholder_canvas_id_ == kInvalidPlaceholderCanvasId) {
-    offscreen_canvas_resource_provider_->ReclaimResource(resource_id);
+    ReclaimResourceInternal(resource_id);
     return;
   }
   // Determines whether the main thread may be blocked. If unblocked, post the
@@ -98,8 +113,7 @@ void CanvasResourceDispatcher::PostImageToPlaceholderIfNotBlocked(
     DCHECK(num_unreclaimed_frames_posted_ == kMaxUnreclaimedPlaceholderFrames);
     if (latest_unposted_image_) {
       // The previous unposted image becomes obsolete now.
-      offscreen_canvas_resource_provider_->ReclaimResource(
-          latest_unposted_resource_id_);
+      ReclaimResourceInternal(latest_unposted_resource_id_);
     }
 
     latest_unposted_image_ = std::move(image);
@@ -173,13 +187,12 @@ bool CanvasResourceDispatcher::PrepareFrame(
   if (!canvas_resource || !VerifyImageSize(canvas_resource->Size()))
     return false;
 
-  offscreen_canvas_resource_provider_->IncNextResourceId();
+  next_resource_id_++;
 
   // For frameless canvas, we don't get a valid frame_sink_id and should drop.
   if (!frame_sink_id_.is_valid()) {
-    PostImageToPlaceholderIfNotBlocked(
-        std::move(canvas_resource),
-        offscreen_canvas_resource_provider_->GetNextResourceId());
+    PostImageToPlaceholderIfNotBlocked(std::move(canvas_resource),
+                                       next_resource_id_);
     return false;
   }
 
@@ -238,16 +251,21 @@ bool CanvasResourceDispatcher::PrepareFrame(
   }
 
   viz::TransferableResource resource;
-  offscreen_canvas_resource_provider_->SetTransferableResource(&resource,
-                                                               canvas_resource);
+  auto frame_resource = std::make_unique<FrameResource>();
+
+  canvas_resource->PrepareTransferableResource(
+      &resource, &frame_resource->release_callback, kVerifiedSyncToken);
+  resource.id = next_resource_id_;
+
+  resources_.insert(next_resource_id_, std::move(frame_resource));
+
   // TODO(crbug.com/869913): add unit testing for this.
   const gfx::Size canvas_resource_size(canvas_resource->Size());
 
   commit_type_histogram.Count(commit_type);
 
-  PostImageToPlaceholderIfNotBlocked(
-      std::move(canvas_resource),
-      offscreen_canvas_resource_provider_->GetNextResourceId());
+  PostImageToPlaceholderIfNotBlocked(std::move(canvas_resource),
+                                     next_resource_id_);
 
   frame->resource_list.push_back(std::move(resource));
 
@@ -428,11 +446,22 @@ void CanvasResourceDispatcher::OnBeginFrame(
 
 void CanvasResourceDispatcher::ReclaimResources(
     const WTF::Vector<viz::ReturnedResource>& resources) {
-  offscreen_canvas_resource_provider_->ReclaimResources(resources);
+  for (const auto& resource : resources) {
+    auto it = resources_.find(resource.id);
+
+    DCHECK(it != resources_.end());
+    if (it == resources_.end())
+      continue;
+
+    it->value->sync_token = resource.sync_token;
+    it->value->is_lost = resource.lost;
+    ReclaimResourceInternal(it);
+  }
 }
 
 void CanvasResourceDispatcher::ReclaimResource(viz::ResourceId resource_id) {
-  offscreen_canvas_resource_provider_->ReclaimResource(resource_id);
+  ReclaimResourceInternal(resource_id);
+
   num_unreclaimed_frames_posted_--;
 
   // The main thread has become unblocked recently and we have an image that
@@ -470,4 +499,20 @@ void CanvasResourceDispatcher::DidDeleteSharedBitmap(
     sink_->DidDeleteSharedBitmap(std::move(id));
 }
 
+void CanvasResourceDispatcher::ReclaimResourceInternal(
+    viz::ResourceId resource_id) {
+  auto it = resources_.find(resource_id);
+  if (it != resources_.end())
+    ReclaimResourceInternal(it);
+}
+
+void CanvasResourceDispatcher::ReclaimResourceInternal(
+    const ResourceMap::iterator& it) {
+  if (it->value->spare_lock) {
+    it->value->spare_lock = false;
+    return;
+  }
+  resources_.erase(it);
+}
+
 }  // namespace blink

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

@@ -11,7 +11,6 @@
 #include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "services/viz/public/interfaces/compositing/compositor_frame_sink.mojom-blink.h"
-#include "third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h"
 #include "third_party/blink/renderer/platform/wtf/compiler.h"
 
 namespace blink {
@@ -81,11 +80,12 @@ class PLATFORM_EXPORT CanvasResourceDispatcher
     kCommitSoftwareCanvasGPUCompositing = 2,
     kCommitSoftwareCanvasSoftwareCompositing = 3,
     kOffscreenCanvasCommitTypeCount,
-
   };
 
  private:
   friend class CanvasResourceDispatcherTest;
+  struct FrameResource;
+  using ResourceMap = HashMap<unsigned, std::unique_ptr<FrameResource>>;
 
   bool PrepareFrame(scoped_refptr<CanvasResource>,
                     base::TimeTicks commit_start_time,
@@ -112,12 +112,18 @@ class PLATFORM_EXPORT CanvasResourceDispatcher
   virtual void PostImageToPlaceholder(scoped_refptr<CanvasResource>,
                                       viz::ResourceId resource_id);
 
+  void ReclaimResourceInternal(viz::ResourceId resource_id);
+  void ReclaimResourceInternal(const ResourceMap::iterator&);
+
   viz::mojom::blink::CompositorFrameSinkPtr sink_;
   mojo::Binding<viz::mojom::blink::CompositorFrameSinkClient> binding_;
   viz::mojom::blink::CompositorFrameSinkClientPtr client_ptr_;
 
   int placeholder_canvas_id_;
 
+  unsigned next_resource_id_ = 0;
+  ResourceMap resources_;
+
   // The latest_unposted_resource_id_ always refers to the Id of the frame
   // resource used by the latest_unposted_image_.
   scoped_refptr<CanvasResource> latest_unposted_image_;
@@ -128,9 +134,6 @@ class PLATFORM_EXPORT CanvasResourceDispatcher
 
   CanvasResourceDispatcherClient* client_;
 
-  std::unique_ptr<OffscreenCanvasResourceProvider>
-      offscreen_canvas_resource_provider_;
-
   base::WeakPtrFactory<CanvasResourceDispatcher> weak_ptr_factory_;
 };
 

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

@@ -30,9 +30,6 @@ class MockCanvasResourceDispatcher : public CanvasResourceDispatcher {
 class CanvasResourceDispatcherTest : public testing::Test {
  public:
   void DispatchOneFrame();
-  OffscreenCanvasResourceProvider* GetResourceProvider() {
-    return dispatcher_->offscreen_canvas_resource_provider_.get();
-  }
 
   unsigned GetNumUnreclaimedFramesPosted() {
     return dispatcher_->num_unreclaimed_frames_posted_;
@@ -42,10 +39,14 @@ class CanvasResourceDispatcherTest : public testing::Test {
     return dispatcher_->latest_unposted_image_.get();
   }
 
-  unsigned GetLatestUnpostedResourceId() {
+  viz::ResourceId GetLatestUnpostedResourceId() {
     return dispatcher_->latest_unposted_resource_id_;
   }
 
+  viz::ResourceId GetCurrentResourceId() {
+    return dispatcher_->next_resource_id_;
+  }
+
  protected:
   CanvasResourceDispatcherTest() {
     dispatcher_ = std::make_unique<MockCanvasResourceDispatcher>();
@@ -80,7 +81,7 @@ TEST_F(CanvasResourceDispatcherTest, PlaceholderRunsNormally) {
   EXPECT_CALL(*(Dispatcher()), PostImageToPlaceholder(_, post_resource_id));
   DispatchOneFrame();
   EXPECT_EQ(1u, GetNumUnreclaimedFramesPosted());
-  EXPECT_EQ(1u, GetResourceProvider()->GetNextResourceId());
+  EXPECT_EQ(1u, GetCurrentResourceId());
   Mock::VerifyAndClearExpectations(Dispatcher());
 
   // Post second frame
@@ -88,7 +89,7 @@ TEST_F(CanvasResourceDispatcherTest, PlaceholderRunsNormally) {
   EXPECT_CALL(*(Dispatcher()), PostImageToPlaceholder(_, post_resource_id));
   DispatchOneFrame();
   EXPECT_EQ(2u, GetNumUnreclaimedFramesPosted());
-  EXPECT_EQ(2u, GetResourceProvider()->GetNextResourceId());
+  EXPECT_EQ(2u, GetCurrentResourceId());
   Mock::VerifyAndClearExpectations(Dispatcher());
 
   // Post third frame
@@ -96,7 +97,7 @@ TEST_F(CanvasResourceDispatcherTest, PlaceholderRunsNormally) {
   EXPECT_CALL(*(Dispatcher()), PostImageToPlaceholder(_, post_resource_id));
   DispatchOneFrame();
   EXPECT_EQ(3u, GetNumUnreclaimedFramesPosted());
-  EXPECT_EQ(3u, GetResourceProvider()->GetNextResourceId());
+  EXPECT_EQ(3u, GetCurrentResourceId());
   EXPECT_EQ(nullptr, GetLatestUnpostedImage());
   Mock::VerifyAndClearExpectations(Dispatcher());
 
@@ -131,7 +132,7 @@ TEST_F(CanvasResourceDispatcherTest, PlaceholderBeingBlocked) {
   DispatchOneFrame();
   unsigned post_resource_id = 4u;
   EXPECT_EQ(3u, GetNumUnreclaimedFramesPosted());
-  EXPECT_EQ(post_resource_id, GetResourceProvider()->GetNextResourceId());
+  EXPECT_EQ(post_resource_id, GetCurrentResourceId());
   EXPECT_TRUE(GetLatestUnpostedImage());
   EXPECT_EQ(post_resource_id, GetLatestUnpostedResourceId());
 
@@ -139,7 +140,7 @@ TEST_F(CanvasResourceDispatcherTest, PlaceholderBeingBlocked) {
   post_resource_id++;
   DispatchOneFrame();
   EXPECT_EQ(3u, GetNumUnreclaimedFramesPosted());
-  EXPECT_EQ(post_resource_id, GetResourceProvider()->GetNextResourceId());
+  EXPECT_EQ(post_resource_id, GetCurrentResourceId());
   EXPECT_TRUE(GetLatestUnpostedImage());
   EXPECT_EQ(post_resource_id, GetLatestUnpostedResourceId());
 
@@ -154,7 +155,7 @@ TEST_F(CanvasResourceDispatcherTest, PlaceholderBeingBlocked) {
   // Reclaim 1 frame and post 1 frame, so numPostImagesUnresponded remains as 3
   EXPECT_EQ(3u, GetNumUnreclaimedFramesPosted());
   // Not generating new resource Id
-  EXPECT_EQ(post_resource_id, GetResourceProvider()->GetNextResourceId());
+  EXPECT_EQ(post_resource_id, GetCurrentResourceId());
   EXPECT_FALSE(GetLatestUnpostedImage());
   EXPECT_EQ(0u, GetLatestUnpostedResourceId());
   Mock::VerifyAndClearExpectations(Dispatcher());

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

-// Copyright 2017 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 "third_party/blink/renderer/platform/graphics/offscreen_canvas_resource_provider.h"
-
-#include "components/viz/common/resources/returned_resource.h"
-#include "components/viz/common/resources/single_release_callback.h"
-#include "components/viz/common/resources/transferable_resource.h"
-#include "third_party/blink/renderer/platform/graphics/canvas_resource.h"
-#include "third_party/blink/renderer/platform/graphics/canvas_resource_dispatcher.h"
-
-namespace blink {
-
-OffscreenCanvasResourceProvider::OffscreenCanvasResourceProvider() = default;
-
-OffscreenCanvasResourceProvider::~OffscreenCanvasResourceProvider() = default;
-
-std::unique_ptr<OffscreenCanvasResourceProvider::FrameResource>
-OffscreenCanvasResourceProvider::CreateOrRecycleFrameResource() {
-  if (!recyclable_resource_)
-    return std::make_unique<FrameResource>();
-
-  recyclable_resource_->spare_lock = true;
-  return std::move(recyclable_resource_);
-}
-
-void OffscreenCanvasResourceProvider::SetTransferableResource(
-    viz::TransferableResource* out_resource,
-    scoped_refptr<CanvasResource> canvas_resource) {
-  DCHECK(canvas_resource->IsValid());
-
-  std::unique_ptr<FrameResource> frame_resource =
-      CreateOrRecycleFrameResource();
-
-  // TODO(junov): Using verified sync tokens for each offscreencanvas is
-  // suboptimal in the case where there are multiple offscreen canvases
-  // commiting frames.  Would be more efficient to batch the verifications.
-  canvas_resource->PrepareTransferableResource(
-      out_resource, &frame_resource->release_callback, kVerifiedSyncToken);
-  out_resource->id = next_resource_id_;
-
-  resources_.insert(next_resource_id_, std::move(frame_resource));
-}
-
-void OffscreenCanvasResourceProvider::ReclaimResources(
-    const WTF::Vector<viz::ReturnedResource>& resources) {
-  for (const auto& resource : resources) {
-    auto it = resources_.find(resource.id);
-
-    DCHECK(it != resources_.end());
-    if (it == resources_.end())
-      continue;
-
-    it->value->sync_token = resource.sync_token;
-    it->value->is_lost = resource.lost;
-    ReclaimResourceInternal(it);
-  }
-}
-
-void OffscreenCanvasResourceProvider::ReclaimResource(unsigned resource_id) {
-  auto it = resources_.find(resource_id);
-  if (it != resources_.end())
-    ReclaimResourceInternal(it);
-}
-
-void OffscreenCanvasResourceProvider::ReclaimResourceInternal(
-    const ResourceMap::iterator& it) {
-  if (it->value->spare_lock) {
-    it->value->spare_lock = false;
-    return;
-  }
-
-  if (it->value->release_callback)
-    it->value->release_callback->Run(it->value->sync_token, it->value->is_lost);
-
-  // Recycle resource.
-  recyclable_resource_ = std::move(it->value);
-  recyclable_resource_->release_callback = nullptr;
-  recyclable_resource_->sync_token.Clear();
-  recyclable_resource_->is_lost = false;
-  resources_.erase(it);
-}
-
-OffscreenCanvasResourceProvider::FrameResource::~FrameResource() {
-  if (release_callback)
-    release_callback->Run(sync_token, is_lost);
-}
-
-}  // namespace blink

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

-// Copyright 2017 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.
-
-#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_OFFSCREEN_CANVAS_RESOURCE_PROVIDER_H_
-#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_OFFSCREEN_CANVAS_RESOURCE_PROVIDER_H_
-
-#include "gpu/command_buffer/common/sync_token.h"
-#include "third_party/blink/renderer/platform/platform_export.h"
-#include "third_party/blink/renderer/platform/wtf/hash_map.h"
-#include "third_party/blink/renderer/platform/wtf/ref_counted.h"
-#include "third_party/blink/renderer/platform/wtf/vector.h"
-
-namespace viz {
-struct ReturnedResource;
-class SingleReleaseCallback;
-struct TransferableResource;
-}
-
-namespace blink {
-
-class CanvasResource;
-
-class PLATFORM_EXPORT OffscreenCanvasResourceProvider {
- public:
-  OffscreenCanvasResourceProvider();
-  ~OffscreenCanvasResourceProvider();
-
-  void SetTransferableResource(viz::TransferableResource* out_resource,
-                               scoped_refptr<CanvasResource>);
-
-  void ReclaimResource(unsigned resource_id);
-  void ReclaimResources(const WTF::Vector<viz::ReturnedResource>& resources);
-  void IncNextResourceId() { next_resource_id_++; }
-  unsigned GetNextResourceId() { return next_resource_id_; }
-
- private:
-  struct FrameResource {
-    FrameResource() = default;
-    ~FrameResource();
-
-    // TODO(junov):  What does this do?
-    bool spare_lock = true;
-
-    std::unique_ptr<viz::SingleReleaseCallback> release_callback;
-    gpu::SyncToken sync_token;
-    bool is_lost = false;
-  };
-
-  using ResourceMap = HashMap<unsigned, std::unique_ptr<FrameResource>>;
-
-  void SetNeedsBeginFrameInternal();
-  std::unique_ptr<FrameResource> CreateOrRecycleFrameResource();
-  void ReclaimResourceInternal(const ResourceMap::iterator&);
-
-  unsigned next_resource_id_ = 0;
-  std::unique_ptr<FrameResource> recyclable_resource_;
-  ResourceMap resources_;
-};
-
-}  // namespace blink
-
-#endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_GRAPHICS_OFFSCREEN_CANVAS_RESOURCE_PROVIDER_H_