Change HitTestAggregator to be owned by root CFSSupport.

Currently, HitTestAggregator is owned by RootCompositorFrameSinkImpl,
which would only work for the viz display compositor (OOP-D) case. In
non-viz, we use DirectLayerTreeFrameSink, and it calls to create a
CompositorFrameSinkSupport with |is_root| equal to true through
HostFrameSinkManager.

1. This changes CompositorFrameSinkSupport to own the HitTestAggregator
when it is a root CFSS, and pass its FrameSinkId to the HTA.
2. Change FrameSinkManagerImpl to be a HitTestAggregatorDelegate.
3. Update DirectLayerTreeFrameSink to activate HitTestAggregator::
Aggregate().

Bug: 805581
Test: viz_unittests
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I0d3d93405dc99e7a5a6edb8f0587c5d0d24ffad7
Reviewed-on: https://chromium-review.googlesource.com/884324
Commit-Queue: Ria Jiang <riajiang@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534186}

components/viz/host/host_frame_sink_manager_unittest.cc

@@ -493,9 +493,8 @@ TEST_F(HostFrameSinkManagerLocalTest, DisplayRootTemporaryReference) {
   GetFrameSinkManagerClient()->OnSurfaceCreated(surface_id);
 }
 
-// Test the creation and desctruction of HitTestQuery, which is stored in
-// HostFrameSinkManager::display_hit_test_query_.
-TEST_F(HostFrameSinkManagerLocalTest, DisplayHitTestQueryMap) {
+// Test the creation and desctruction of HitTestAggregator and HitTestQuery.
+TEST_F(HostFrameSinkManagerLocalTest, HitTestAggregatorQuery) {
   FakeHostFrameSinkClient client;
   EXPECT_FALSE(FrameSinkDataExists(kFrameSinkChild1));
   host().RegisterFrameSinkId(kFrameSinkChild1, &client);
@@ -504,7 +503,9 @@ TEST_F(HostFrameSinkManagerLocalTest, DisplayHitTestQueryMap) {
   EXPECT_FALSE(DisplayHitTestQueryExists(kFrameSinkChild1));
   auto support =
       CreateCompositorFrameSinkSupport(kFrameSinkChild1, true /* is_root */);
+  support->SetUpHitTest();
   EXPECT_TRUE(DisplayHitTestQueryExists(kFrameSinkChild1));
+  EXPECT_TRUE(support->GetHitTestAggregator());
 
   host().InvalidateFrameSinkId(kFrameSinkChild1);
   support.reset();

components/viz/service/frame_sinks/compositor_frame_sink_support.cc

@@ -58,6 +58,13 @@ CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
   DCHECK(capture_clients_.empty());
 }
 
+void CompositorFrameSinkSupport::SetUpHitTest() {
+  DCHECK(is_root_);
+  hit_test_aggregator_ = std::make_unique<HitTestAggregator>(
+      frame_sink_manager_->hit_test_manager(), frame_sink_manager_,
+      frame_sink_id_);
+}
+
 void CompositorFrameSinkSupport::SetAggregatedDamageCallback(
     AggregatedDamageCallback callback) {
   aggregated_damage_callback_ = std::move(callback);
@@ -441,6 +448,11 @@ void CompositorFrameSinkSupport::RequestCopyOfSurface(
     surface_manager_->SurfaceModified(current_surface->surface_id(), ack);
 }
 
+HitTestAggregator* CompositorFrameSinkSupport::GetHitTestAggregator() {
+  DCHECK(is_root_);
+  return hit_test_aggregator_.get();
+}
+
 Surface* CompositorFrameSinkSupport::GetCurrentSurfaceForTesting() {
   return surface_manager_->GetSurfaceForId(current_surface_id_);
 }

components/viz/service/frame_sinks/compositor_frame_sink_support.h

@@ -18,6 +18,7 @@
 #include "components/viz/service/frame_sinks/surface_resource_holder.h"
 #include "components/viz/service/frame_sinks/surface_resource_holder_client.h"
 #include "components/viz/service/frame_sinks/video_capture/capturable_frame_sink.h"
+#include "components/viz/service/hit_test/hit_test_aggregator.h"
 #include "components/viz/service/surfaces/surface_client.h"
 #include "components/viz/service/viz_service_export.h"
 #include "services/viz/public/interfaces/compositing/compositor_frame_sink.mojom.h"
@@ -59,6 +60,10 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
 
   FrameSinkManagerImpl* frame_sink_manager() { return frame_sink_manager_; }
 
+  // Viz hit-test setup is only called when |is_root_| is true (except on
+  // android webview).
+  void SetUpHitTest();
+
   // The provided callback will be run every time a surface owned by this object
   // or one of its descendents is determined to be damaged at aggregation time.
   void SetAggregatedDamageCallback(AggregatedDamageCallback callback);
@@ -108,6 +113,8 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
   void RequestCopyOfSurface(
       std::unique_ptr<CopyOutputRequest> request) override;
 
+  HitTestAggregator* GetHitTestAggregator();
+
   Surface* GetCurrentSurfaceForTesting();
 
  private:
@@ -158,6 +165,9 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
 
   SurfaceResourceHolder surface_resource_holder_;
 
+  // This has a HitTestAggregator if and only if |is_root_| is true.
+  std::unique_ptr<HitTestAggregator> hit_test_aggregator_;
+
   // Counts the number of CompositorFrames that have been submitted and have not
   // yet received an ACK.
   int ack_pending_count_ = 0;

components/viz/service/frame_sinks/direct_layer_tree_frame_sink.cc

@@ -78,6 +78,8 @@ bool DirectLayerTreeFrameSink::BindToClient(
   support_ = support_manager_->CreateCompositorFrameSinkSupport(
       this, frame_sink_id_, is_root,
       capabilities_.delegated_sync_points_required);
+  // TODO(riajiang): Check if viz hit-test is enabled and do setup work if it
+  // is turned on.
   begin_frame_source_ = std::make_unique<ExternalBeginFrameSource>(this);
   client_->SetBeginFrameSource(begin_frame_source_.get());
 
@@ -129,7 +131,8 @@ void DirectLayerTreeFrameSink::DisplayOutputSurfaceLost() {
 void DirectLayerTreeFrameSink::DisplayWillDrawAndSwap(
     bool will_draw_and_swap,
     const RenderPassList& render_passes) {
-  // This notification is not relevant to our client outside of tests.
+  if (support_->GetHitTestAggregator())
+    support_->GetHitTestAggregator()->Aggregate(display_->CurrentSurfaceId());
 }
 
 void DirectLayerTreeFrameSink::DisplayDidDrawAndSwap() {

components/viz/service/frame_sinks/frame_sink_manager_impl.cc

@@ -241,6 +241,66 @@ void FrameSinkManagerImpl::CreateVideoCapturer(
       std::make_unique<FrameSinkVideoCapturerImpl>(this, std::move(request)));
 }
 
+void FrameSinkManagerImpl::OnSurfaceCreated(const SurfaceId& surface_id) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  if (client_) {
+    client_->OnSurfaceCreated(surface_id);
+  } else {
+    // There is no client to assign an owner for the temporary reference, so we
+    // can drop the temporary reference safely.
+    surface_manager_.DropTemporaryReference(surface_id);
+  }
+}
+
+void FrameSinkManagerImpl::OnFirstSurfaceActivation(
+    const SurfaceInfo& surface_info) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_GT(surface_info.device_scale_factor(), 0.0f);
+
+  if (client_)
+    client_->OnFirstSurfaceActivation(surface_info);
+}
+
+void FrameSinkManagerImpl::OnSurfaceActivated(const SurfaceId& surface_id) {}
+
+bool FrameSinkManagerImpl::OnSurfaceDamaged(const SurfaceId& surface_id,
+                                            const BeginFrameAck& ack) {
+  return false;
+}
+
+void FrameSinkManagerImpl::OnSurfaceDiscarded(const SurfaceId& surface_id) {}
+
+void FrameSinkManagerImpl::OnSurfaceDestroyed(const SurfaceId& surface_id) {}
+
+void FrameSinkManagerImpl::OnSurfaceDamageExpected(const SurfaceId& surface_id,
+                                                   const BeginFrameArgs& args) {
+}
+
+void FrameSinkManagerImpl::OnAggregatedHitTestRegionListUpdated(
+    const FrameSinkId& frame_sink_id,
+    mojo::ScopedSharedBufferHandle active_handle,
+    uint32_t active_handle_size,
+    mojo::ScopedSharedBufferHandle idle_handle,
+    uint32_t idle_handle_size) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (client_) {
+    client_->OnAggregatedHitTestRegionListUpdated(
+        frame_sink_id, std::move(active_handle), active_handle_size,
+        std::move(idle_handle), idle_handle_size);
+  }
+}
+
+void FrameSinkManagerImpl::SwitchActiveAggregatedHitTestRegionList(
+    const FrameSinkId& frame_sink_id,
+    uint8_t active_handle_index) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (client_) {
+    client_->SwitchActiveAggregatedHitTestRegionList(frame_sink_id,
+                                                     active_handle_index);
+  }
+}
+
 void FrameSinkManagerImpl::RegisterCompositorFrameSinkSupport(
     const FrameSinkId& frame_sink_id,
     CompositorFrameSinkSupport* support) {
@@ -384,42 +444,6 @@ bool FrameSinkManagerImpl::ChildContains(
   return false;
 }
 
-void FrameSinkManagerImpl::OnSurfaceCreated(const SurfaceId& surface_id) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  if (client_) {
-    client_->OnSurfaceCreated(surface_id);
-  } else {
-    // There is no client to assign an owner for the temporary reference, so we
-    // can drop the temporary reference safely.
-    surface_manager_.DropTemporaryReference(surface_id);
-  }
-}
-
-void FrameSinkManagerImpl::OnFirstSurfaceActivation(
-    const SurfaceInfo& surface_info) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK_GT(surface_info.device_scale_factor(), 0.0f);
-
-  if (client_)
-    client_->OnFirstSurfaceActivation(surface_info);
-}
-
-void FrameSinkManagerImpl::OnSurfaceActivated(const SurfaceId& surface_id) {}
-
-bool FrameSinkManagerImpl::OnSurfaceDamaged(const SurfaceId& surface_id,
-                                            const BeginFrameAck& ack) {
-  return false;
-}
-
-void FrameSinkManagerImpl::OnSurfaceDiscarded(const SurfaceId& surface_id) {}
-
-void FrameSinkManagerImpl::OnSurfaceDestroyed(const SurfaceId& surface_id) {}
-
-void FrameSinkManagerImpl::OnSurfaceDamageExpected(const SurfaceId& surface_id,
-                                                   const BeginFrameArgs& args) {
-}
-
 void FrameSinkManagerImpl::OnClientConnectionLost(
     const FrameSinkId& frame_sink_id) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -446,30 +470,6 @@ void FrameSinkManagerImpl::OnFrameTokenChanged(const FrameSinkId& frame_sink_id,
     client_->OnFrameTokenChanged(frame_sink_id, frame_token);
 }
 
-void FrameSinkManagerImpl::OnAggregatedHitTestRegionListUpdated(
-    const FrameSinkId& frame_sink_id,
-    mojo::ScopedSharedBufferHandle active_handle,
-    uint32_t active_handle_size,
-    mojo::ScopedSharedBufferHandle idle_handle,
-    uint32_t idle_handle_size) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (client_) {
-    client_->OnAggregatedHitTestRegionListUpdated(
-        frame_sink_id, std::move(active_handle), active_handle_size,
-        std::move(idle_handle), idle_handle_size);
-  }
-}
-
-void FrameSinkManagerImpl::SwitchActiveAggregatedHitTestRegionList(
-    const FrameSinkId& frame_sink_id,
-    uint8_t active_handle_index) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  if (client_) {
-    client_->SwitchActiveAggregatedHitTestRegionList(frame_sink_id,
-                                                     active_handle_index);
-  }
-}
-
 VideoDetector* FrameSinkManagerImpl::CreateVideoDetectorForTesting(
     std::unique_ptr<base::TickClock> tick_clock,
     scoped_refptr<base::SequencedTaskRunner> task_runner) {

components/viz/service/frame_sinks/frame_sink_manager_impl.h

@@ -20,6 +20,7 @@
 #include "components/viz/service/frame_sinks/primary_begin_frame_source.h"
 #include "components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_manager.h"
 #include "components/viz/service/frame_sinks/video_detector.h"
+#include "components/viz/service/hit_test/hit_test_aggregator_delegate.h"
 #include "components/viz/service/hit_test/hit_test_manager.h"
 #include "components/viz/service/surfaces/surface_manager.h"
 #include "components/viz/service/surfaces/surface_observer.h"
@@ -41,7 +42,8 @@ class DisplayProvider;
 class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
     : public SurfaceObserver,
       public FrameSinkVideoCapturerManager,
-      public mojom::FrameSinkManager {
+      public mojom::FrameSinkManager,
+      public HitTestAggregatorDelegate {
  public:
   FrameSinkManagerImpl(uint32_t number_of_frames_to_activation_deadline = 4u,
                        DisplayProvider* display_provider = nullptr);
@@ -85,6 +87,28 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
   void CreateVideoCapturer(
       mojom::FrameSinkVideoCapturerRequest request) override;
 
+  // SurfaceObserver implementation.
+  void OnSurfaceCreated(const SurfaceId& surface_id) override;
+  void OnFirstSurfaceActivation(const SurfaceInfo& surface_info) override;
+  void OnSurfaceActivated(const SurfaceId& surface_id) override;
+  bool OnSurfaceDamaged(const SurfaceId& surface_id,
+                        const BeginFrameAck& ack) override;
+  void OnSurfaceDiscarded(const SurfaceId& surface_id) override;
+  void OnSurfaceDestroyed(const SurfaceId& surface_id) override;
+  void OnSurfaceDamageExpected(const SurfaceId& surface_id,
+                               const BeginFrameArgs& args) override;
+
+  // HitTestAggregatorDelegate implementation:
+  void OnAggregatedHitTestRegionListUpdated(
+      const FrameSinkId& frame_sink_id,
+      mojo::ScopedSharedBufferHandle active_handle,
+      uint32_t active_handle_size,
+      mojo::ScopedSharedBufferHandle idle_handle,
+      uint32_t idle_handle_size) override;
+  void SwitchActiveAggregatedHitTestRegionList(
+      const FrameSinkId& frame_sink_id,
+      uint8_t active_handle_index) override;
+
   // CompositorFrameSinkSupport, hierarchy, and BeginFrameSource can be
   // registered and unregistered in any order with respect to each other.
   //
@@ -114,28 +138,8 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl
 
   const HitTestManager* hit_test_manager() { return &hit_test_manager_; }
 
-  // SurfaceObserver implementation.
-  void OnSurfaceCreated(const SurfaceId& surface_id) override;
-  void OnFirstSurfaceActivation(const SurfaceInfo& surface_info) override;
-  void OnSurfaceActivated(const SurfaceId& surface_id) override;
-  bool OnSurfaceDamaged(const SurfaceId& surface_id,
-                        const BeginFrameAck& ack) override;
-  void OnSurfaceDiscarded(const SurfaceId& surface_id) override;
-  void OnSurfaceDestroyed(const SurfaceId& surface_id) override;
-  void OnSurfaceDamageExpected(const SurfaceId& surface_id,
-                               const BeginFrameArgs& args) override;
-
   void OnClientConnectionLost(const FrameSinkId& frame_sink_id);
 
-  void OnAggregatedHitTestRegionListUpdated(
-      const FrameSinkId& frame_sink_id,
-      mojo::ScopedSharedBufferHandle active_handle,
-      uint32_t active_handle_size,
-      mojo::ScopedSharedBufferHandle idle_handle,
-      uint32_t idle_handle_size);
-  void SwitchActiveAggregatedHitTestRegionList(const FrameSinkId& frame_sink_id,
-                                               uint8_t active_handle_index);
-
   void SubmitHitTestRegionList(
       const SurfaceId& surface_id,
       uint64_t frame_index,

components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc

@@ -10,6 +10,7 @@
 #include "components/viz/service/display/display.h"
 #include "components/viz/service/display_embedder/external_begin_frame_controller_impl.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
+#include "components/viz/service/hit_test/hit_test_aggregator.h"
 
 namespace viz {
 
@@ -37,8 +38,7 @@ RootCompositorFrameSinkImpl::RootCompositorFrameSinkImpl(
       synthetic_begin_frame_source_(std::move(synthetic_begin_frame_source)),
       external_begin_frame_controller_(
           std::move(external_begin_frame_controller)),
-      display_(std::move(display)),
-      hit_test_aggregator_(frame_sink_manager->hit_test_manager(), this) {
+      display_(std::move(display)) {
   DCHECK(begin_frame_source());
   DCHECK(display_);
 
@@ -50,6 +50,7 @@ RootCompositorFrameSinkImpl::RootCompositorFrameSinkImpl(
   frame_sink_manager->RegisterBeginFrameSource(begin_frame_source(),
                                                frame_sink_id);
   display_->Initialize(this, frame_sink_manager->surface_manager());
+  support_->SetUpHitTest();
 }
 
 RootCompositorFrameSinkImpl::~RootCompositorFrameSinkImpl() {
@@ -118,22 +119,6 @@ void RootCompositorFrameSinkImpl::DidNotProduceFrame(
   support_->DidNotProduceFrame(begin_frame_ack);
 }
 
-void RootCompositorFrameSinkImpl::OnAggregatedHitTestRegionListUpdated(
-    mojo::ScopedSharedBufferHandle active_handle,
-    uint32_t active_handle_size,
-    mojo::ScopedSharedBufferHandle idle_handle,
-    uint32_t idle_handle_size) {
-  support_->frame_sink_manager()->OnAggregatedHitTestRegionListUpdated(
-      support_->frame_sink_id(), std::move(active_handle), active_handle_size,
-      std::move(idle_handle), idle_handle_size);
-}
-
-void RootCompositorFrameSinkImpl::SwitchActiveAggregatedHitTestRegionList(
-    uint8_t active_handle_index) {
-  support_->frame_sink_manager()->SwitchActiveAggregatedHitTestRegionList(
-      support_->frame_sink_id(), active_handle_index);
-}
-
 void RootCompositorFrameSinkImpl::DisplayOutputSurfaceLost() {
   // TODO(staraz): Implement this. Client should hear about context/output
   // surface lost.
@@ -142,7 +127,8 @@ void RootCompositorFrameSinkImpl::DisplayOutputSurfaceLost() {
 void RootCompositorFrameSinkImpl::DisplayWillDrawAndSwap(
     bool will_draw_and_swap,
     const RenderPassList& render_pass) {
-  hit_test_aggregator_.Aggregate(display_->CurrentSurfaceId());
+  DCHECK(support_->GetHitTestAggregator());
+  support_->GetHitTestAggregator()->Aggregate(display_->CurrentSurfaceId());
 }
 
 void RootCompositorFrameSinkImpl::DisplayDidReceiveCALayerParams(

components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h

@@ -11,8 +11,6 @@
 #include "components/viz/common/surfaces/local_surface_id.h"
 #include "components/viz/service/display/display_client.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support.h"
-#include "components/viz/service/hit_test/hit_test_aggregator.h"
-#include "components/viz/service/hit_test/hit_test_aggregator_delegate.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
 #include "services/viz/privileged/interfaces/compositing/display_private.mojom.h"
 #include "services/viz/public/interfaces/compositing/compositor_frame_sink.mojom.h"
@@ -28,8 +26,7 @@ class SyntheticBeginFrameSource;
 // for the mojom::CompositorFrameSink interface and owns the Display.
 class RootCompositorFrameSinkImpl : public mojom::CompositorFrameSink,
                                     public mojom::DisplayPrivate,
-                                    public DisplayClient,
-                                    public HitTestAggregatorDelegate {
+                                    public DisplayClient {
  public:
   RootCompositorFrameSinkImpl(
       FrameSinkManagerImpl* frame_sink_manager,
@@ -64,15 +61,6 @@ class RootCompositorFrameSinkImpl : public mojom::CompositorFrameSink,
                              uint64_t submit_time) override;
   void DidNotProduceFrame(const BeginFrameAck& begin_frame_ack) override;
 
-  // HitTestAggregatorDelegate:
-  void OnAggregatedHitTestRegionListUpdated(
-      mojo::ScopedSharedBufferHandle active_handle,
-      uint32_t active_handle_size,
-      mojo::ScopedSharedBufferHandle idle_handle,
-      uint32_t idle_handle_size) override;
-  void SwitchActiveAggregatedHitTestRegionList(
-      uint8_t active_handle_index) override;
-
  private:
   // DisplayClient:
   void DisplayOutputSurfaceLost() override;
@@ -105,8 +93,6 @@ class RootCompositorFrameSinkImpl : public mojom::CompositorFrameSink,
       external_begin_frame_controller_;
   std::unique_ptr<Display> display_;
 
-  HitTestAggregator hit_test_aggregator_;
-
   DISALLOW_COPY_AND_ASSIGN(RootCompositorFrameSinkImpl);
 };
 

components/viz/service/hit_test/hit_test_aggregator.cc

@@ -31,9 +31,11 @@ void PrepareTransformForReadOnlySharedMemory(gfx::Transform* transform) {
 }  // namespace
 
 HitTestAggregator::HitTestAggregator(const HitTestManager* hit_test_manager,
-                                     HitTestAggregatorDelegate* delegate)
+                                     HitTestAggregatorDelegate* delegate,
+                                     const FrameSinkId& frame_sink_id)
     : hit_test_manager_(hit_test_manager),
       delegate_(delegate),
+      root_frame_sink_id_(frame_sink_id),
       weak_ptr_factory_(this) {
   AllocateHitTestRegionArray();
 }
@@ -52,11 +54,13 @@ void HitTestAggregator::GrowRegionList() {
 void HitTestAggregator::Swap() {
   SwapHandles();
   if (!handle_replaced_) {
-    delegate_->SwitchActiveAggregatedHitTestRegionList(active_handle_index_);
+    delegate_->SwitchActiveAggregatedHitTestRegionList(root_frame_sink_id_,
+                                                       active_handle_index_);
     return;
   }
 
   delegate_->OnAggregatedHitTestRegionListUpdated(
+      root_frame_sink_id_,
       read_handle_->Clone(mojo::SharedBufferHandle::AccessMode::READ_ONLY),
       read_size_,
       write_handle_->Clone(mojo::SharedBufferHandle::AccessMode::READ_ONLY),
@@ -74,7 +78,9 @@ void HitTestAggregator::AllocateHitTestRegionArray() {
 void HitTestAggregator::ResizeHitTestRegionArray(uint32_t size) {
   size_t num_bytes = size * sizeof(AggregatedHitTestRegion);
   write_handle_ = mojo::SharedBufferHandle::Create(num_bytes);
+  DCHECK(write_handle_.is_valid());
   auto new_buffer_ = write_handle_->Map(num_bytes);
+  DCHECK(new_buffer_);
   handle_replaced_ = true;
 
   AggregatedHitTestRegion* region = (AggregatedHitTestRegion*)new_buffer_.get();

components/viz/service/hit_test/hit_test_aggregator.h

@@ -27,7 +27,8 @@ class VIZ_SERVICE_EXPORT HitTestAggregator {
  public:
   // |delegate| owns and outlives HitTestAggregator.
   HitTestAggregator(const HitTestManager* hit_test_manager,
-                    HitTestAggregatorDelegate* delegate);
+                    HitTestAggregatorDelegate* delegate,
+                    const FrameSinkId& frame_sink_id);
   ~HitTestAggregator();
 
   // Called after surfaces have been aggregated into the DisplayFrame.
@@ -43,25 +44,6 @@ class VIZ_SERVICE_EXPORT HitTestAggregator {
  private:
   friend class TestHitTestAggregator;
 
-  const HitTestManager* const hit_test_manager_;
-
-  mojo::ScopedSharedBufferHandle read_handle_;
-  mojo::ScopedSharedBufferHandle write_handle_;
-
-  // The number of elements allocated.
-  uint32_t read_size_ = 0;
-  uint32_t write_size_ = 0;
-
-  mojo::ScopedSharedBufferMapping read_buffer_;
-  mojo::ScopedSharedBufferMapping write_buffer_;
-
-  bool handle_replaced_ = false;
-
-  // Can only be 0 or 1 when we only have two buffers.
-  uint8_t active_handle_index_ = 0;
-
-  HitTestAggregatorDelegate* const delegate_;
-
   // Allocates memory for the AggregatedHitTestRegion array.
   void AllocateHitTestRegionArray();
 
@@ -93,6 +75,28 @@ class VIZ_SERVICE_EXPORT HitTestAggregator {
   // Marks the element at the given index as the end of list.
   void MarkEndAt(size_t index);
 
+  const HitTestManager* const hit_test_manager_;
+
+  mojo::ScopedSharedBufferHandle read_handle_;
+  mojo::ScopedSharedBufferHandle write_handle_;
+
+  // The number of elements allocated.
+  uint32_t read_size_ = 0;
+  uint32_t write_size_ = 0;
+
+  mojo::ScopedSharedBufferMapping read_buffer_;
+  mojo::ScopedSharedBufferMapping write_buffer_;
+
+  bool handle_replaced_ = false;
+
+  // Can only be 0 or 1 when we only have two buffers.
+  uint8_t active_handle_index_ = 0;
+
+  HitTestAggregatorDelegate* const delegate_;
+
+  // This is the FrameSinkId for the corresponding root CompositorFrameSink.
+  const FrameSinkId root_frame_sink_id_;
+
   // Handles the case when this object is deleted after
   // the PostTaskAggregation call is scheduled but before invocation.
   base::WeakPtrFactory<HitTestAggregator> weak_ptr_factory_;

components/viz/service/hit_test/hit_test_aggregator_delegate.h

@@ -6,19 +6,21 @@
 #define COMPONENTS_VIZ_SERVICE_HIT_TEST_HIT_TEST_AGGREGATOR_DELEGATE_H_
 
 namespace viz {
-// Used by HitTestAggregator to talk to GpuRootCompositorFrameSink.
+// Used by HitTestAggregator to talk to FrameSinkManagerImpl.
 class HitTestAggregatorDelegate {
  public:
   // Called if any of the buffer that stores the aggregated hit-test data is
   // updated (e.g. destroyed, reallocated etc.). |active_handle| and
   // |idle_handle| both must be valid.
   virtual void OnAggregatedHitTestRegionListUpdated(
+      const FrameSinkId& frame_sink_id,
       mojo::ScopedSharedBufferHandle active_handle,
       uint32_t active_handle_size,
       mojo::ScopedSharedBufferHandle idle_handle,
       uint32_t idle_handle_size) = 0;
 
   virtual void SwitchActiveAggregatedHitTestRegionList(
+      const FrameSinkId& frame_sink_id,
       uint8_t active_handle_index) = 0;
 
  protected:

components/viz/service/hit_test/hit_test_aggregator_unittest.cc

@@ -32,8 +32,10 @@ SurfaceId MakeSurfaceId(const FrameSinkId& frame_sink_id, uint32_t parent_id) {
 class TestHitTestAggregator final : public HitTestAggregator {
  public:
   TestHitTestAggregator(HitTestManager* manager,
-                        HitTestAggregatorDelegate* delegate)
-      : HitTestAggregator(manager, delegate) {}
+                        HitTestAggregatorDelegate* delegate,
+                        const FrameSinkId& frame_sink_id)
+      : HitTestAggregator(manager, delegate, frame_sink_id),
+        frame_sink_id_(frame_sink_id) {}
   ~TestHitTestAggregator() = default;
 
   int GetRegionCount() const {
@@ -46,7 +48,8 @@ class TestHitTestAggregator final : public HitTestAggregator {
   }
   int GetHitTestRegionListSize() { return read_size_; }
   void SwapHandles() {
-    delegate_->SwitchActiveAggregatedHitTestRegionList(active_handle_index_);
+    delegate_->SwitchActiveAggregatedHitTestRegionList(frame_sink_id_,
+                                                       active_handle_index_);
   }
 
   void Reset() {
@@ -57,6 +60,9 @@ class TestHitTestAggregator final : public HitTestAggregator {
     regions = static_cast<AggregatedHitTestRegion*>(read_buffer_.get());
     regions[0].child_count = kEndOfList;
   }
+
+ private:
+  const FrameSinkId frame_sink_id_;
 };
 
 namespace {
@@ -89,36 +95,8 @@ class TestHitTestManager : public HitTestManager {
 
 namespace {
 
-class TestGpuRootCompositorFrameSink : public HitTestAggregatorDelegate {
- public:
-  TestGpuRootCompositorFrameSink(TestHitTestManager* hit_test_manager,
-                                 TestFrameSinkManagerImpl* frame_sink_manager,
-                                 const FrameSinkId& frame_sink_id)
-      : frame_sink_manager_(frame_sink_manager),
-        frame_sink_id_(frame_sink_id),
-        aggregator_(
-            std::make_unique<TestHitTestAggregator>(hit_test_manager, this)) {}
-  ~TestGpuRootCompositorFrameSink() override = default;
-
-  // HitTestAggregatorDelegate:
-  void OnAggregatedHitTestRegionListUpdated(
-      mojo::ScopedSharedBufferHandle active_handle,
-      uint32_t active_handle_size,
-      mojo::ScopedSharedBufferHandle idle_handle,
-      uint32_t idle_handle_size) override;
-  void SwitchActiveAggregatedHitTestRegionList(
-      uint8_t active_handle_index) override;
-
-  TestHitTestAggregator* aggregator() { return aggregator_.get(); }
-
- private:
-  TestFrameSinkManagerImpl* frame_sink_manager_;
-  FrameSinkId frame_sink_id_;
-  std::unique_ptr<TestHitTestAggregator> aggregator_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestGpuRootCompositorFrameSink);
-};
-
+// TODO(riajiang): TestHostFrameSinkManager should be based on
+// mojom::FrameSinkManagerClient instead.
 class TestHostFrameSinkManager : public HostFrameSinkManager {
  public:
   TestHostFrameSinkManager() = default;
@@ -172,7 +150,7 @@ class TestFrameSinkManagerImpl : public FrameSinkManagerImpl {
       mojo::ScopedSharedBufferHandle active_handle,
       uint32_t active_handle_size,
       mojo::ScopedSharedBufferHandle idle_handle,
-      uint32_t idle_handle_size) {
+      uint32_t idle_handle_size) override {
     // Do not check if it's on valid thread for tests.
     if (host_client_) {
       host_client_->OnAggregatedHitTestRegionListUpdated(
@@ -181,8 +159,9 @@ class TestFrameSinkManagerImpl : public FrameSinkManagerImpl {
     }
   }
 
-  void SwitchActiveAggregatedHitTestRegionList(const FrameSinkId& frame_sink_id,
-                                               uint8_t active_handle_index) {
+  void SwitchActiveAggregatedHitTestRegionList(
+      const FrameSinkId& frame_sink_id,
+      uint8_t active_handle_index) override {
     // Do not check if it's on valid thread for tests.
     if (host_client_) {
       host_client_->SwitchActiveAggregatedHitTestRegionList(
@@ -190,16 +169,16 @@ class TestFrameSinkManagerImpl : public FrameSinkManagerImpl {
     }
   }
 
-  void CreateRootCompositorFrameSinkLocal(TestHitTestManager* hit_test_manager,
-                                          const FrameSinkId& frame_sink_id) {
-    compositor_frame_sinks_[frame_sink_id] =
-        std::make_unique<TestGpuRootCompositorFrameSink>(hit_test_manager, this,
-                                                         frame_sink_id);
+  void CreateHitTestAggregator(TestHitTestManager* hit_test_manager,
+                               const FrameSinkId& frame_sink_id) {
+    hit_test_aggregators_[frame_sink_id] =
+        std::make_unique<TestHitTestAggregator>(hit_test_manager, this,
+                                                frame_sink_id);
   }
 
-  const std::map<FrameSinkId, std::unique_ptr<TestGpuRootCompositorFrameSink>>&
-  compositor_frame_sinks() {
-    return compositor_frame_sinks_;
+  const std::map<FrameSinkId, std::unique_ptr<TestHitTestAggregator>>&
+  hit_test_aggregators() {
+    return hit_test_aggregators_;
   }
 
   uint64_t GetActiveFrameIndex(const SurfaceId& surface_id) override {
@@ -207,29 +186,13 @@ class TestFrameSinkManagerImpl : public FrameSinkManagerImpl {
   }
 
  private:
-  std::map<FrameSinkId, std::unique_ptr<TestGpuRootCompositorFrameSink>>
-      compositor_frame_sinks_;
+  std::map<FrameSinkId, std::unique_ptr<TestHitTestAggregator>>
+      hit_test_aggregators_;
   TestHostFrameSinkManager* host_client_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(TestFrameSinkManagerImpl);
 };
 
-void TestGpuRootCompositorFrameSink::OnAggregatedHitTestRegionListUpdated(
-    mojo::ScopedSharedBufferHandle active_handle,
-    uint32_t active_handle_size,
-    mojo::ScopedSharedBufferHandle idle_handle,
-    uint32_t idle_handle_size) {
-  frame_sink_manager_->OnAggregatedHitTestRegionListUpdated(
-      frame_sink_id_, std::move(active_handle), active_handle_size,
-      std::move(idle_handle), idle_handle_size);
-}
-
-void TestGpuRootCompositorFrameSink::SwitchActiveAggregatedHitTestRegionList(
-    uint8_t active_handle_index) {
-  frame_sink_manager_->SwitchActiveAggregatedHitTestRegionList(
-      frame_sink_id_, active_handle_index);
-}
-
 }  // namespace
 
 TestHitTestManager::TestHitTestManager(
@@ -248,8 +211,8 @@ class HitTestAggregatorTest : public testing::Test {
     hit_test_manager_ =
         std::make_unique<TestHitTestManager>(frame_sink_manager_.get());
     frame_sink_manager_->SetLocalClient(host_frame_sink_manager_.get());
-    frame_sink_manager_->CreateRootCompositorFrameSinkLocal(
-        hit_test_manager_.get(), kDisplayFrameSink);
+    frame_sink_manager_->CreateHitTestAggregator(hit_test_manager_.get(),
+                                                 kDisplayFrameSink);
   }
   void TearDown() override {
     frame_sink_manager_.reset();
@@ -291,10 +254,8 @@ class HitTestAggregatorTest : public testing::Test {
 
  protected:
   TestHitTestAggregator* GetAggregator(const FrameSinkId& frame_sink_id) {
-    DCHECK(frame_sink_manager_->compositor_frame_sinks().count(frame_sink_id));
-    return frame_sink_manager_->compositor_frame_sinks()
-        .at(frame_sink_id)
-        ->aggregator();
+    DCHECK(frame_sink_manager_->hit_test_aggregators().count(frame_sink_id));
+    return frame_sink_manager_->hit_test_aggregators().at(frame_sink_id).get();
   }
 
   AggregatedHitTestRegion* host_regions() {

ui/message_center/BUILD.gn

@@ -213,6 +213,7 @@ if (enable_message_center) {
       ":test_support",
       "//base",
       "//base/test:test_support",
+      "//mojo/edk/system",
       "//skia",
       "//testing/gmock",
       "//testing/gtest",

ui/message_center/test/run_all_unittests.cc

@@ -10,6 +10,7 @@
 #include "base/test/test_discardable_memory_allocator.h"
 #include "base/test/test_suite.h"
 #include "build/build_config.h"
+#include "mojo/edk/embedder/embedder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/ui_base_paths.h"
@@ -20,10 +21,6 @@
 #include "ui/gl/test/gl_surface_test_support.h"
 #endif
 
-#if defined(OS_CHROMEOS)
-#include "mojo/edk/embedder/embedder.h"  // nogncheck
-#endif
-
 namespace {
 
 class MessageCenterTestSuite : public base::TestSuite {
@@ -63,10 +60,7 @@ class MessageCenterTestSuite : public base::TestSuite {
 
 int main(int argc, char** argv) {
   MessageCenterTestSuite test_suite(argc, argv);
-
-#if defined(OS_CHROMEOS)
   mojo::edk::Init();
-#endif
 
   return base::LaunchUnitTests(
       argc,

ui/views/BUILD.gn

@@ -1188,6 +1188,7 @@ test("views_perftests") {
     ":test_support",
     "//base/test:test_support",
     "//cc/base:base",
+    "//mojo/edk/system",
     "//testing/perf",
     "//ui/resources:ui_test_pak",
   ]

ui/views/views_perftests.cc

@@ -3,10 +3,12 @@
 // found in the LICENSE file.
 
 #include "base/test/launcher/unit_test_launcher.h"
+#include "mojo/edk/embedder/embedder.h"
 #include "ui/views/views_test_suite.h"
 
 int main(int argc, char** argv) {
   views::ViewsTestSuite test_suite(argc, argv);
+  mojo::edk::Init();
   return base::LaunchUnitTestsSerially(
       argc, argv,
       base::Bind(&views::ViewsTestSuite::Run, base::Unretained(&test_suite)));