aw: Submit frames in RootFrameSink for viz

In some cases when we transit from one HardwareRenderer to another two
of them can be alive simultaneously. This is incompatible with holding
reference to CompositorFrameSinkSupport as only one can exist for
specific FrameSinkId.

This CL moves the logic of frame submission for viz from
HardwareRendererViz::OnViz to RootFrameSink to avoid problems as there
is always one RootFrameSink.

Bug: 805739
Change-Id: Ibe45227bf0e72ee4a39932345087ded448e3c506
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2028732
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736555}

android_webview/browser/gfx/hardware_renderer_viz.cc

@@ -79,35 +79,6 @@ class HardwareRendererViz::OnViz : public viz::DisplayClient {
       const viz::FrameSinkId& id) override;
 
  private:
-  class CompositorFrameSinkClient
-      : public viz::mojom::CompositorFrameSinkClient {
-   public:
-    CompositorFrameSinkClient(OnViz* owner,
-                              uint32_t layer_tree_frame_sink_id,
-                              viz::FrameSinkId frame_sink_id)
-        : owner_(owner),
-          layer_tree_frame_sink_id_(layer_tree_frame_sink_id),
-          frame_sink_id_(frame_sink_id) {}
-
-    void DidReceiveCompositorFrameAck(
-        const std::vector<viz::ReturnedResource>& resources) override {
-      ReclaimResources(resources);
-    }
-    void OnBeginFrame(const viz::BeginFrameArgs& args,
-                      const viz::FrameTimingDetailsMap& feedbacks) override {}
-    void OnBeginFramePausedChanged(bool paused) override {}
-    void ReclaimResources(
-        const std::vector<viz::ReturnedResource>& resources) override {
-      owner_->without_gpu_->ReturnResources(
-          frame_sink_id_, layer_tree_frame_sink_id_, resources);
-    }
-
-   private:
-    OnViz* const owner_;
-    const uint32_t layer_tree_frame_sink_id_;
-    const viz::FrameSinkId frame_sink_id_;
-  };
-
   viz::FrameSinkManagerImpl* GetFrameSinkManager();
 
   scoped_refptr<RootFrameSink> without_gpu_;
@@ -118,9 +89,6 @@ class HardwareRendererViz::OnViz : public viz::DisplayClient {
   std::unique_ptr<viz::BeginFrameSource> stub_begin_frame_source_;
   std::unique_ptr<viz::Display> display_;
 
-  std::unique_ptr<CompositorFrameSinkClient> child_sink_client_;
-  std::unique_ptr<viz::CompositorFrameSinkSupport> child_sink_support_;
-
   std::unique_ptr<viz::HitTestAggregator> hit_test_aggregator_;
   viz::SurfaceId child_surface_id_;
   viz::FrameTokenGenerator next_frame_token_;
@@ -181,33 +149,7 @@ void HardwareRendererViz::OnViz::DrawAndSwapOnViz(
 
   if (child_frame->frame) {
     DCHECK(!viz_frame_submission_);
-    if (!child_sink_support_ ||
-        child_sink_support_->frame_sink_id() != child_frame->frame_sink_id) {
-      child_sink_support_.reset();
-      child_sink_client_.reset();
-
-      child_sink_client_ = std::make_unique<CompositorFrameSinkClient>(
-          this, child_frame->layer_tree_frame_sink_id,
-          child_frame->frame_sink_id);
-
-      child_sink_support_ = std::make_unique<viz::CompositorFrameSinkSupport>(
-          child_sink_client_.get(), GetFrameSinkManager(),
-          child_frame->frame_sink_id, false);
-
-      child_sink_support_->SetBeginFrameSource(nullptr);
-    }
-
-    child_sink_support_->SubmitCompositorFrame(
-        child_frame->local_surface_id, std::move(*child_frame->frame),
-        std::move(child_frame->hit_test_region_list));
-    child_frame->frame.reset();
-
-    CopyOutputRequestQueue requests;
-    requests.swap(child_frame->copy_requests);
-    for (auto& copy_request : requests) {
-      child_sink_support_->RequestCopyOfOutput(child_frame->local_surface_id,
-                                               std::move(copy_request));
-    }
+    without_gpu_->SubmitChildCompositorFrame(child_frame);
   }
 
   gfx::DisplayColorSpaces display_color_spaces(
@@ -278,9 +220,7 @@ void HardwareRendererViz::OnViz::DrawAndSwapOnViz(
 
 void HardwareRendererViz::OnViz::PostDrawOnViz(
     viz::FrameTimingDetailsMap* timing_details) {
-  if (child_sink_support_) {
-    *timing_details = child_sink_support_->TakeFrameTimingDetailsMap();
-  }
+  *timing_details = without_gpu_->TakeChildFrameTimingDetailsMap();
 }
 
 viz::FrameSinkManagerImpl* HardwareRendererViz::OnViz::GetFrameSinkManager() {

android_webview/browser/gfx/root_frame_sink.cc

@@ -4,6 +4,7 @@
 
 #include "android_webview/browser/gfx/root_frame_sink.h"
 
+#include "android_webview/browser/gfx/child_frame.h"
 #include "android_webview/browser/gfx/display_scheduler_webview.h"
 #include "android_webview/browser/gfx/viz_compositor_thread_runner_webview.h"
 #include "base/no_destructor.h"
@@ -23,6 +24,49 @@ viz::FrameSinkId AllocateParentSinkId() {
 
 }  // namespace
 
+class RootFrameSink::ChildCompositorFrameSink
+    : public viz::mojom::CompositorFrameSinkClient {
+ public:
+  ChildCompositorFrameSink(RootFrameSink* owner,
+                           uint32_t layer_tree_frame_sink_id,
+                           viz::FrameSinkId frame_sink_id)
+      : owner_(owner),
+        layer_tree_frame_sink_id_(layer_tree_frame_sink_id),
+        frame_sink_id_(frame_sink_id),
+        support_(std::make_unique<viz::CompositorFrameSinkSupport>(
+            this,
+            owner->GetFrameSinkManager(),
+            frame_sink_id,
+            false)) {
+    support_->SetBeginFrameSource(nullptr);
+  }
+
+  void DidReceiveCompositorFrameAck(
+      const std::vector<viz::ReturnedResource>& resources) override {
+    ReclaimResources(resources);
+  }
+  void OnBeginFrame(const viz::BeginFrameArgs& args,
+                    const viz::FrameTimingDetailsMap& feedbacks) override {}
+  void OnBeginFramePausedChanged(bool paused) override {}
+  void ReclaimResources(
+      const std::vector<viz::ReturnedResource>& resources) override {
+    owner_->ReturnResources(frame_sink_id_, layer_tree_frame_sink_id_,
+                            resources);
+  }
+
+  const viz::FrameSinkId frame_sink_id() { return frame_sink_id_; }
+
+  uint32_t layer_tree_frame_sink_id() { return layer_tree_frame_sink_id_; }
+
+  viz::CompositorFrameSinkSupport* support() { return support_.get(); }
+
+ private:
+  RootFrameSink* const owner_;
+  const uint32_t layer_tree_frame_sink_id_;
+  const viz::FrameSinkId frame_sink_id_;
+  std::unique_ptr<viz::CompositorFrameSinkSupport> support_;
+};
+
 RootFrameSink::RootFrameSink(RootFrameSinkClient* client)
     : root_frame_sink_id_(AllocateParentSinkId()), client_(client) {
   constexpr bool is_root = true;
@@ -134,4 +178,36 @@ void RootFrameSink::DettachClient() {
   client_ = nullptr;
 }
 
+void RootFrameSink::SubmitChildCompositorFrame(ChildFrame* child_frame) {
+  DCHECK(child_frame->frame);
+  if (!child_sink_support_ ||
+      child_sink_support_->frame_sink_id() != child_frame->frame_sink_id ||
+      child_sink_support_->layer_tree_frame_sink_id() !=
+          child_frame->layer_tree_frame_sink_id) {
+    child_sink_support_.reset();
+
+    child_sink_support_ = std::make_unique<ChildCompositorFrameSink>(
+        this, child_frame->layer_tree_frame_sink_id,
+        child_frame->frame_sink_id);
+  }
+
+  child_sink_support_->support()->SubmitCompositorFrame(
+      child_frame->local_surface_id, std::move(*child_frame->frame),
+      std::move(child_frame->hit_test_region_list));
+  child_frame->frame.reset();
+
+  CopyOutputRequestQueue requests;
+  requests.swap(child_frame->copy_requests);
+  for (auto& copy_request : requests) {
+    child_sink_support_->support()->RequestCopyOfOutput(
+        child_frame->local_surface_id, std::move(copy_request));
+  }
+}
+
+viz::FrameTimingDetailsMap RootFrameSink::TakeChildFrameTimingDetailsMap() {
+  if (child_sink_support_)
+    return child_sink_support_->support()->TakeFrameTimingDetailsMap();
+  return viz::FrameTimingDetailsMap();
+}
+
 }  // namespace android_webview

android_webview/browser/gfx/root_frame_sink.h

@@ -21,6 +21,7 @@ class ExternalBeginFrameSource;
 }  // namespace viz
 
 namespace android_webview {
+class ChildFrame;
 
 class RootFrameSinkClient {
  public:
@@ -57,9 +58,8 @@ class RootFrameSink : public base::RefCounted<RootFrameSink>,
   bool IsChildSurface(const viz::FrameSinkId& frame_sink_id);
   void DettachClient();
 
-  void ReturnResources(viz::FrameSinkId frame_sink_id,
-                       uint32_t layer_tree_frame_sink_id,
-                       std::vector<viz::ReturnedResource> resources);
+  void SubmitChildCompositorFrame(ChildFrame* child_frame);
+  viz::FrameTimingDetailsMap TakeChildFrameTimingDetailsMap();
 
   // viz::mojom::CompositorFrameSinkClient implementation.
   void DidReceiveCompositorFrameAck(
@@ -75,14 +75,21 @@ class RootFrameSink : public base::RefCounted<RootFrameSink>,
 
  private:
   friend class base::RefCounted<RootFrameSink>;
+  class ChildCompositorFrameSink;
+
   ~RootFrameSink() override;
   viz::FrameSinkManagerImpl* GetFrameSinkManager();
+  void ReturnResources(viz::FrameSinkId frame_sink_id,
+                       uint32_t layer_tree_frame_sink_id,
+                       std::vector<viz::ReturnedResource> resources);
 
   const viz::FrameSinkId root_frame_sink_id_;
   base::flat_set<viz::FrameSinkId> child_frame_sink_ids_;
   std::unique_ptr<viz::CompositorFrameSinkSupport> support_;
   std::unique_ptr<viz::ExternalBeginFrameSource> begin_frame_source_;
 
+  std::unique_ptr<ChildCompositorFrameSink> child_sink_support_;
+
   bool needs_begin_frames_ = false;
   bool needs_draw_ = false;
   RootFrameSinkClient* client_;