viz: Introduce RegisterFrameSinkId and InvalidateFrameSinkId Host API

Registering a FrameSinkId delimits the beginning of the lifetime of a
FrameSinkId. From Viz's perspective that ID is valid until
InvalidateFrameSinkId is called. RegisterFrameSinkId also bundles
a HostFrameSinkClient (was FrameSinkObserver) that listens for new surfaces
submitted to the registered FrameSinkId.

This CL allows us to replace multiple call sites to RegisterFrameSink /
InvalidateFrameSinkId in content that go directly to FrameSinkManagerImpl
with calls to HostFrameSinkManager, taking us closer to a point where
content does not directly use FrameSinkManagerImpl.

This CL is also a prerequisite for using HostFrameSinkManager in the
window server because it introduces a mechanism for listening for events
pertaining to a particular FrameSinkId which is a requirement for the
window server. This interface will be used for events as well in the
future.

Bug: 657959
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I3e1343ea6cfbe6b3d39ba2fcd23d193dd2c3e4a0
Reviewed-on: https://chromium-review.googlesource.com/595147Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491386}

components/viz/host/BUILD.gn

@@ -8,7 +8,7 @@ viz_component("host") {
   defines = [ "VIZ_HOST_IMPLEMENTATION" ]
 
   sources = [
-    "frame_sink_observer.h",
+    "host_frame_sink_client.h",
     "host_frame_sink_manager.cc",
     "host_frame_sink_manager.h",
     "server_gpu_memory_buffer_manager.cc",

components/viz/host/frame_sink_observer.h → components/viz/host/host_frame_sink_client.h

@@ -2,23 +2,23 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_VIZ_HOST_FRAME_SINK_OBSERVER_H_
-#define COMPONENTS_VIZ_HOST_FRAME_SINK_OBSERVER_H_
+#ifndef COMPONENTS_VIZ_HOST_HOST_FRAME_SINK_CLIENT_H_
+#define COMPONENTS_VIZ_HOST_HOST_FRAME_SINK_CLIENT_H_
 
 namespace viz {
 
 class SurfaceInfo;
 
-class FrameSinkObserver {
+class HostFrameSinkClient {
  public:
   // Runs when a CompositorFrame is received for the given SurfaceInfo for the
   // first time.
   virtual void OnSurfaceCreated(const SurfaceInfo& surface_info) = 0;
 
  protected:
-  ~FrameSinkObserver() {}
+  virtual ~HostFrameSinkClient() {}
 };
 
 }  // namespace viz
 
-#endif  // COMPONENTS_VIZ_HOST_FRAME_SINK_OBSERVER_H_
+#endif  // COMPONENTS_VIZ_HOST_HOST_FRAME_SINK_CLIENT_H_

components/viz/host/host_frame_sink_manager.cc

@@ -40,12 +40,23 @@ void HostFrameSinkManager::BindAndSetManager(
   frame_sink_manager_ = frame_sink_manager_ptr_.get();
 }
 
-void HostFrameSinkManager::AddObserver(FrameSinkObserver* observer) {
-  observers_.AddObserver(observer);
+void HostFrameSinkManager::RegisterFrameSinkId(const FrameSinkId& frame_sink_id,
+                                               HostFrameSinkClient* client) {
+  DCHECK(frame_sink_id.is_valid());
+  DCHECK(client);
+  FrameSinkData& data = frame_sink_data_map_[frame_sink_id];
+  DCHECK(!data.HasCompositorFrameSinkData());
+  data.client = client;
+  frame_sink_manager_->RegisterFrameSinkId(frame_sink_id);
 }
 
-void HostFrameSinkManager::RemoveObserver(FrameSinkObserver* observer) {
-  observers_.RemoveObserver(observer);
+void HostFrameSinkManager::InvalidateFrameSinkId(
+    const FrameSinkId& frame_sink_id) {
+  DCHECK(frame_sink_id.is_valid());
+  auto it = frame_sink_data_map_.find(frame_sink_id);
+  DCHECK(it != frame_sink_data_map_.end());
+  frame_sink_data_map_.erase(it);
+  frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id);
 }
 
 void HostFrameSinkManager::CreateCompositorFrameSink(
@@ -60,26 +71,6 @@ void HostFrameSinkManager::CreateCompositorFrameSink(
 
   frame_sink_manager_->CreateCompositorFrameSink(
       frame_sink_id, std::move(request), std::move(client));
-  frame_sink_manager_->RegisterFrameSinkId(frame_sink_id);
-}
-
-void HostFrameSinkManager::DestroyCompositorFrameSink(
-    const FrameSinkId& frame_sink_id) {
-  auto iter = frame_sink_data_map_.find(frame_sink_id);
-  DCHECK(iter != frame_sink_data_map_.end());
-
-  FrameSinkData& data = iter->second;
-  DCHECK(data.HasCompositorFrameSinkData());
-  if (data.has_created_compositor_frame_sink) {
-    // This will also destroy the CompositorFrameSink pipe to the client.
-    frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id);
-    data.has_created_compositor_frame_sink = false;
-  } else {
-    data.support = nullptr;
-  }
-
-  if (data.IsEmpty())
-    frame_sink_data_map_.erase(iter);
 }
 
 void HostFrameSinkManager::RegisterFrameSinkHierarchy(
@@ -135,6 +126,23 @@ HostFrameSinkManager::CreateCompositorFrameSinkSupport(
   return support;
 }
 
+void HostFrameSinkManager::DestroyCompositorFrameSink(
+    const FrameSinkId& frame_sink_id) {
+  auto iter = frame_sink_data_map_.find(frame_sink_id);
+  DCHECK(iter != frame_sink_data_map_.end());
+
+  FrameSinkData& data = iter->second;
+  DCHECK(data.HasCompositorFrameSinkData());
+  if (data.has_created_compositor_frame_sink) {
+    data.has_created_compositor_frame_sink = false;
+  } else {
+    data.support = nullptr;
+  }
+
+  if (data.IsEmpty())
+    frame_sink_data_map_.erase(iter);
+}
+
 void HostFrameSinkManager::PerformAssignTemporaryReference(
     const SurfaceId& surface_id) {
   // Find the expected embedder for the new surface and assign the temporary
@@ -160,8 +168,14 @@ void HostFrameSinkManager::PerformAssignTemporaryReference(
 }
 
 void HostFrameSinkManager::OnSurfaceCreated(const SurfaceInfo& surface_info) {
-  for (auto& observer : observers_)
-    observer.OnSurfaceCreated(surface_info);
+  auto it = frame_sink_data_map_.find(surface_info.id().frame_sink_id());
+  // If we've received a bogus or stale SurfaceId from Viz then just ignore it.
+  if (it == frame_sink_data_map_.end())
+    return;
+
+  FrameSinkData& frame_sink_data = it->second;
+  if (frame_sink_data.client)
+    frame_sink_data.client->OnSurfaceCreated(surface_info);
 
   if (frame_sink_manager_impl_ &&
       frame_sink_manager_impl_->surface_manager()->using_surface_references()) {

components/viz/host/host_frame_sink_manager.h

@@ -15,7 +15,7 @@
 #include "base/observer_list.h"
 #include "base/optional.h"
 #include "components/viz/common/surfaces/frame_sink_id.h"
-#include "components/viz/host/frame_sink_observer.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "components/viz/host/viz_host_export.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support_manager.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -57,20 +57,24 @@ class VIZ_HOST_EXPORT HostFrameSinkManager
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       mojom::FrameSinkManagerPtr ptr);
 
-  void AddObserver(FrameSinkObserver* observer);
-  void RemoveObserver(FrameSinkObserver* observer);
+  // Registers |frame_sink_id| will be used. This must be called before
+  // CreateCompositorFrameSink(Support) is called.
+  void RegisterFrameSinkId(const FrameSinkId& frame_sink_id,
+                           HostFrameSinkClient* client);
+
+  // Invalidates |frame_sink_id| which cleans up any unsatisified surface
+  // sequences or dangling temporary references assigned to it. If there is a
+  // CompositorFrameSink for |frame_sink_id| then it will be destroyed and the
+  // message pipe to the client will be closed.
+  void InvalidateFrameSinkId(const FrameSinkId& frame_sink_id);
 
   // Creates a connection between client to viz, using |request| and |client|,
   // that allows the client to submit CompositorFrames. When no longer needed,
-  // call DestroyCompositorFrameSink().
+  // call InvalidateFrameSinkId().
   void CreateCompositorFrameSink(const FrameSinkId& frame_sink_id,
                                  mojom::CompositorFrameSinkRequest request,
                                  mojom::CompositorFrameSinkClientPtr client);
 
-  // Destroys a client connection. Will call UnregisterFrameSinkHierarchy() with
-  // the registered parent if there is one.
-  void DestroyCompositorFrameSink(const FrameSinkId& frame_sink_id);
-
   // Registers FrameSink hierarchy. Clients can call this multiple times to
   // reparent without calling UnregisterFrameSinkHierarchy().
   void RegisterFrameSinkHierarchy(const FrameSinkId& parent_frame_sink_id,
@@ -104,9 +108,12 @@ class VIZ_HOST_EXPORT HostFrameSinkManager
 
     // Returns true if there is nothing in FrameSinkData and it can be deleted.
     bool IsEmpty() const {
-      return !HasCompositorFrameSinkData() && !parent.has_value();
+      return !HasCompositorFrameSinkData() && !parent.has_value() && !client;
     }
 
+    // The client to be notified of changes to this FrameSink.
+    HostFrameSinkClient* client = nullptr;
+
     // If the frame sink is a root that corresponds to a Display.
     bool is_root = false;
 
@@ -124,6 +131,10 @@ class VIZ_HOST_EXPORT HostFrameSinkManager
     DISALLOW_COPY_AND_ASSIGN(FrameSinkData);
   };
 
+  // Destroys a client connection. Will call UnregisterFrameSinkHierarchy() with
+  // the registered parent if there is one.
+  void DestroyCompositorFrameSink(const FrameSinkId& frame_sink_id);
+
   // Assigns the temporary reference to the frame sink that is expected to
   // embeded |surface_id|, otherwise drops the temporary reference.
   void PerformAssignTemporaryReference(const SurfaceId& surface_id);
@@ -152,9 +163,6 @@ class VIZ_HOST_EXPORT HostFrameSinkManager
   // Per CompositorFrameSink data.
   base::flat_map<FrameSinkId, FrameSinkData> frame_sink_data_map_;
 
-  // Local observers to that receive OnSurfaceCreated() messages from IPC.
-  base::ObserverList<FrameSinkObserver> observers_;
-
   base::WeakPtrFactory<HostFrameSinkManager> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(HostFrameSinkManager);

components/viz/host/host_frame_sink_manager_unittests.cc

@@ -39,6 +39,19 @@ SurfaceInfo MakeSurfaceInfo(const SurfaceId& surface_id) {
   return SurfaceInfo(surface_id, 1.f, gfx::Size(1, 1));
 }
 
+// A fake (do-nothing) implementation of HostFrameSinkClient.
+class FakeHostFrameSinkClient : public HostFrameSinkClient {
+ public:
+  FakeHostFrameSinkClient() = default;
+  ~FakeHostFrameSinkClient() override = default;
+
+  // HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const SurfaceInfo& surface_info) override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FakeHostFrameSinkClient);
+};
+
 // A mock implementation of mojom::FrameSinkManager.
 class MockFrameSinkManagerImpl : public FrameSinkManagerImpl {
  public:
@@ -101,16 +114,22 @@ class HostFrameSinkManagerTest : public testing::Test {
 
   // testing::Test:
   void SetUp() override {
-    manager_impl_ = base::MakeUnique<MockFrameSinkManagerImpl>();
+    manager_impl_ =
+        base::MakeUnique<testing::NiceMock<MockFrameSinkManagerImpl>>();
     host_manager_ = base::MakeUnique<HostFrameSinkManager>();
 
     manager_impl_->SetLocalClient(host_manager_.get());
     host_manager_->SetLocalManager(manager_impl_.get());
   }
 
+  void TearDown() override {
+    host_manager_.reset();
+    manager_impl_.reset();
+  }
+
  private:
   std::unique_ptr<HostFrameSinkManager> host_manager_;
-  std::unique_ptr<MockFrameSinkManagerImpl> manager_impl_;
+  std::unique_ptr<testing::NiceMock<MockFrameSinkManagerImpl>> manager_impl_;
 
   DISALLOW_COPY_AND_ASSIGN(HostFrameSinkManagerTest);
 };
@@ -121,10 +140,15 @@ TEST_F(HostFrameSinkManagerTest, CreateMojomCompositorFrameSink) {
   // Calling CreateCompositorFrameSink() should first register the frame sink
   // and then request to create it.
   EXPECT_CALL(manager_impl(), RegisterFrameSinkId(kClientFrameSinkId));
+
+  FakeHostFrameSinkClient client;
+  host_manager().RegisterFrameSinkId(kClientFrameSinkId, &client);
+
   EXPECT_CALL(manager_impl(),
               MockCreateCompositorFrameSink(kClientFrameSinkId));
   host_manager().CreateCompositorFrameSink(
       kClientFrameSinkId, nullptr /* request */, nullptr /* client */);
+
   EXPECT_TRUE(FrameSinkDataExists(kClientFrameSinkId));
 
   // Register should call through to FrameSinkManagerImpl and should work even
@@ -136,7 +160,6 @@ TEST_F(HostFrameSinkManagerTest, CreateMojomCompositorFrameSink) {
 
   // Destroying the CompositorFrameSink should invalidate it in viz.
   EXPECT_CALL(manager_impl(), InvalidateFrameSinkId(kClientFrameSinkId));
-  host_manager().DestroyCompositorFrameSink(kClientFrameSinkId);
 
   // We should still have the hierarchy data for |kClientFrameSinkId|.
   EXPECT_TRUE(FrameSinkDataExists(kClientFrameSinkId));
@@ -146,6 +169,7 @@ TEST_F(HostFrameSinkManagerTest, CreateMojomCompositorFrameSink) {
                                                            kClientFrameSinkId));
   host_manager().UnregisterFrameSinkHierarchy(kParentFrameSinkId,
                                               kClientFrameSinkId);
+  host_manager().InvalidateFrameSinkId(kClientFrameSinkId);
 
   // Data for |kClientFrameSinkId| should be deleted now.
   EXPECT_FALSE(FrameSinkDataExists(kClientFrameSinkId));

content/browser/frame_host/render_widget_host_view_child_frame.cc

@@ -66,8 +66,7 @@ RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
       background_color_(SK_ColorWHITE),
       weak_factory_(this) {
   if (!service_manager::ServiceManagerIsRemote()) {
-    GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(
-        frame_sink_id_);
+    GetHostFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_, this);
     CreateCompositorFrameSinkSupport();
   }
 }
@@ -75,10 +74,8 @@ RenderWidgetHostViewChildFrame::RenderWidgetHostViewChildFrame(
 RenderWidgetHostViewChildFrame::~RenderWidgetHostViewChildFrame() {
   if (!service_manager::ServiceManagerIsRemote()) {
     ResetCompositorFrameSinkSupport();
-    if (GetFrameSinkManager()) {
-      GetFrameSinkManager()->surface_manager()->InvalidateFrameSinkId(
-          frame_sink_id_);
-    }
+    if (GetHostFrameSinkManager())
+      GetHostFrameSinkManager()->InvalidateFrameSinkId(frame_sink_id_);
   }
 }
 
@@ -769,6 +766,12 @@ void RenderWidgetHostViewChildFrame::OnBeginFramePausedChanged(bool paused) {
   renderer_compositor_frame_sink_->OnBeginFramePausedChanged(paused);
 }
 
+void RenderWidgetHostViewChildFrame::OnSurfaceCreated(
+    const viz::SurfaceInfo& surface_info) {
+  // TODO(fsamuel): Once surface synchronization is turned on, the fallback
+  // surface should be set here.
+}
+
 void RenderWidgetHostViewChildFrame::SetNeedsBeginFrames(
     bool needs_begin_frames) {
   if (support_)

content/browser/frame_host/render_widget_host_view_child_frame.h

@@ -20,6 +20,7 @@
 #include "components/viz/common/resources/returned_resource.h"
 #include "components/viz/common/surfaces/surface_info.h"
 #include "components/viz/common/surfaces/surface_sequence.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support_client.h"
 #include "content/browser/compositor/image_transport_factory.h"
 #include "content/browser/renderer_host/event_with_latency_info.h"
@@ -54,7 +55,8 @@ class TouchSelectionControllerClientChildFrame;
 class CONTENT_EXPORT RenderWidgetHostViewChildFrame
     : public RenderWidgetHostViewBase,
       public TouchSelectionControllerClientManager::Observer,
-      public NON_EXPORTED_BASE(viz::CompositorFrameSinkSupportClient) {
+      public NON_EXPORTED_BASE(viz::CompositorFrameSinkSupportClient),
+      public NON_EXPORTED_BASE(viz::HostFrameSinkClient) {
  public:
   static RenderWidgetHostViewChildFrame* Create(RenderWidgetHost* widget);
   ~RenderWidgetHostViewChildFrame() override;
@@ -181,6 +183,9 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame
                        const gfx::Rect& damage_rect) override {}
   void OnBeginFramePausedChanged(bool paused) override;
 
+  // viz::HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
+
   // Exposed for tests.
   bool IsChildFrameForTesting() const override;
   viz::SurfaceId SurfaceIdForTesting() const override;

content/browser/renderer_host/compositor_impl_android.cc

@@ -465,7 +465,7 @@ CompositorImpl::CompositorImpl(CompositorClient* client,
       num_successive_context_creation_failures_(0),
       layer_tree_frame_sink_request_pending_(false),
       weak_factory_(this) {
-  GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(frame_sink_id_);
+  GetHostFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_, this);
   DCHECK(client);
   DCHECK(root_window);
   DCHECK(root_window->GetLayer() == nullptr);
@@ -483,8 +483,7 @@ CompositorImpl::~CompositorImpl() {
   root_window_->SetLayer(nullptr);
   // Clean-up any surface references.
   SetSurface(NULL);
-  GetFrameSinkManager()->surface_manager()->InvalidateFrameSinkId(
-      frame_sink_id_);
+  GetHostFrameSinkManager()->InvalidateFrameSinkId(frame_sink_id_);
 }
 
 bool CompositorImpl::IsForSubframe() {
@@ -939,6 +938,11 @@ void CompositorImpl::RemoveChildFrameSink(
                                                           frame_sink_id);
 }
 
+void CompositorImpl::OnSurfaceCreated(const viz::SurfaceInfo& surface_info) {
+  // TODO(fsamuel): Once surface synchronization is turned on, the fallback
+  // surface should be set here.
+}
+
 bool CompositorImpl::HavePendingReadbacks() {
   return !readback_layer_tree_->children().empty();
 }

content/browser/renderer_host/compositor_impl_android.h

@@ -18,6 +18,7 @@
 #include "cc/trees/layer_tree_host_client.h"
 #include "cc/trees/layer_tree_host_single_thread_client.h"
 #include "components/viz/common/surfaces/frame_sink_id.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/android/compositor.h"
 #include "gpu/command_buffer/common/capabilities.h"
@@ -57,7 +58,8 @@ class CONTENT_EXPORT CompositorImpl
       public cc::LayerTreeHostClient,
       public cc::LayerTreeHostSingleThreadClient,
       public ui::UIResourceProvider,
-      public ui::WindowAndroidCompositor {
+      public ui::WindowAndroidCompositor,
+      public viz::HostFrameSinkClient {
  public:
   CompositorImpl(CompositorClient* client, gfx::NativeWindow root_window);
   ~CompositorImpl() override;
@@ -121,6 +123,9 @@ class CONTENT_EXPORT CompositorImpl
   void AddChildFrameSink(const viz::FrameSinkId& frame_sink_id) override;
   void RemoveChildFrameSink(const viz::FrameSinkId& frame_sink_id) override;
 
+  // viz::HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
+
   void SetVisible(bool visible);
   void CreateLayerTreeHost();
 

content/browser/renderer_host/delegated_frame_host.cc

@@ -54,10 +54,9 @@ DelegatedFrameHost::DelegatedFrameHost(const viz::FrameSinkId& frame_sink_id,
       frame_evictor_(new viz::FrameEvictor(this)) {
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   factory->GetContextFactory()->AddObserver(this);
-  factory->GetContextFactoryPrivate()
-      ->GetFrameSinkManager()
-      ->surface_manager()
-      ->RegisterFrameSinkId(frame_sink_id_);
+  viz::HostFrameSinkManager* host_frame_sink_manager =
+      factory->GetContextFactoryPrivate()->GetHostFrameSinkManager();
+  host_frame_sink_manager->RegisterFrameSinkId(frame_sink_id_, this);
   CreateCompositorFrameSinkSupport();
 }
 
@@ -514,6 +513,12 @@ void DelegatedFrameHost::OnBeginFramePausedChanged(bool paused) {
     renderer_compositor_frame_sink_->OnBeginFramePausedChanged(paused);
 }
 
+void DelegatedFrameHost::OnSurfaceCreated(
+    const viz::SurfaceInfo& surface_info) {
+  // TODO(fsamuel): Once surface synchronization is turned on, the fallback
+  // surface should be set here.
+}
+
 void DelegatedFrameHost::OnBeginFrame(const viz::BeginFrameArgs& args) {
   if (renderer_compositor_frame_sink_)
     renderer_compositor_frame_sink_->OnBeginFrame(args);
@@ -773,10 +778,9 @@ DelegatedFrameHost::~DelegatedFrameHost() {
 
   ResetCompositorFrameSinkSupport();
 
-  factory->GetContextFactoryPrivate()
-      ->GetFrameSinkManager()
-      ->surface_manager()
-      ->InvalidateFrameSinkId(frame_sink_id_);
+  viz::HostFrameSinkManager* host_frame_sink_manager =
+      factory->GetContextFactoryPrivate()->GetHostFrameSinkManager();
+  host_frame_sink_manager->InvalidateFrameSinkId(frame_sink_id_);
 
   DCHECK(!vsync_manager_.get());
 }

content/browser/renderer_host/delegated_frame_host.h

@@ -13,6 +13,7 @@
 #include "components/viz/common/frame_sinks/begin_frame_args.h"
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "components/viz/common/quads/copy_output_result.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support_client.h"
 #include "components/viz/service/frame_sinks/frame_evictor.h"
 #include "content/browser/compositor/image_transport_factory.h"
@@ -81,6 +82,7 @@ class CONTENT_EXPORT DelegatedFrameHost
       public ui::ContextFactoryObserver,
       public viz::FrameEvictorClient,
       public NON_EXPORTED_BASE(viz::CompositorFrameSinkSupportClient),
+      public NON_EXPORTED_BASE(viz::HostFrameSinkClient),
       public base::SupportsWeakPtr<DelegatedFrameHost> {
  public:
   DelegatedFrameHost(const viz::FrameSinkId& frame_sink_id,
@@ -115,6 +117,9 @@ class CONTENT_EXPORT DelegatedFrameHost
                        const gfx::Rect& damage_rect) override;
   void OnBeginFramePausedChanged(bool paused) override;
 
+  // viz::HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
+
   // Public interface exposed to RenderWidgetHostView.
 
   void DidCreateNewRendererCompositorFrameSink(

content/browser/renderer_host/offscreen_canvas_surface_impl.cc

@@ -31,16 +31,15 @@ OffscreenCanvasSurfaceImpl::OffscreenCanvasSurfaceImpl(
   binding_.set_connection_error_handler(
       base::Bind(&OffscreenCanvasSurfaceImpl::OnSurfaceConnectionClosed,
                  base::Unretained(this)));
-  host_frame_sink_manager_->AddObserver(this);
+  host_frame_sink_manager_->RegisterFrameSinkId(frame_sink_id_, this);
 }
 
 OffscreenCanvasSurfaceImpl::~OffscreenCanvasSurfaceImpl() {
   if (has_created_compositor_frame_sink_) {
     host_frame_sink_manager_->UnregisterFrameSinkHierarchy(
         parent_frame_sink_id_, frame_sink_id_);
-    host_frame_sink_manager_->DestroyCompositorFrameSink(frame_sink_id_);
+    host_frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id_);
   }
-  host_frame_sink_manager_->RemoveObserver(this);
 }
 
 void OffscreenCanvasSurfaceImpl::CreateCompositorFrameSink(
@@ -61,8 +60,7 @@ void OffscreenCanvasSurfaceImpl::CreateCompositorFrameSink(
 
 void OffscreenCanvasSurfaceImpl::OnSurfaceCreated(
     const viz::SurfaceInfo& surface_info) {
-  if (surface_info.id().frame_sink_id() != frame_sink_id_)
-    return;
+  DCHECK_EQ(surface_info.id().frame_sink_id(), frame_sink_id_);
 
   local_surface_id_ = surface_info.id().local_surface_id();
   if (client_)

content/browser/renderer_host/offscreen_canvas_surface_impl.h

@@ -9,7 +9,7 @@
 #include "base/compiler_specific.h"
 #include "components/viz/common/surfaces/frame_sink_id.h"
 #include "components/viz/common/surfaces/surface_info.h"
-#include "components/viz/host/frame_sink_observer.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "components/viz/host/host_frame_sink_manager.h"
 #include "content/common/content_export.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -21,7 +21,7 @@ namespace content {
 // connections to both the renderer and frame sink manager.
 class CONTENT_EXPORT OffscreenCanvasSurfaceImpl
     : public blink::mojom::OffscreenCanvasSurface,
-      public NON_EXPORTED_BASE(viz::FrameSinkObserver) {
+      public NON_EXPORTED_BASE(viz::HostFrameSinkClient) {
  public:
   using DestroyCallback = base::OnceCallback<void()>;
 
@@ -52,7 +52,7 @@ class CONTENT_EXPORT OffscreenCanvasSurfaceImpl
       viz::mojom::CompositorFrameSinkClientPtr client,
       viz::mojom::CompositorFrameSinkRequest request);
 
-  // FrameSinkObserver implementation.
+  // viz::HostFrameSinkClient implementation.
   void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
 
   // blink::mojom::OffscreenCanvasSurface implementation.

content/test/BUILD.gn

@@ -257,6 +257,7 @@ static_library("test_support") {
     "//cc/ipc",
     "//cc/ipc:interfaces",
     "//components/leveldb/public/interfaces",
+    "//components/viz/host",
     "//components/viz/service",
     "//content/app:both_for_content_tests",
     "//content/browser:for_content_tests",

content/test/DEPS

@@ -6,6 +6,7 @@ include_rules = [
   "+components/scheduler/renderer",
   "+components/scheduler/test",
   "+components/viz/common",
+  "+components/viz/host",
   "+components/viz/service",
   "+services/viz/public/interfaces/compositing",
 

content/test/test_render_view_host.cc

@@ -8,6 +8,7 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "components/viz/host/host_frame_sink_manager.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
 #include "components/viz/service/surfaces/surface_manager.h"
 #include "content/browser/compositor/image_transport_factory.h"
@@ -69,13 +70,12 @@ TestRenderWidgetHostView::TestRenderWidgetHostView(RenderWidgetHost* rwh)
       background_color_(SK_ColorWHITE) {
 #if defined(OS_ANDROID)
   frame_sink_id_ = AllocateFrameSinkId();
-  GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(frame_sink_id_);
+  GetHostFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_, this);
 #else
   // Not all tests initialize or need an image transport factory.
   if (ImageTransportFactory::GetInstance()) {
     frame_sink_id_ = AllocateFrameSinkId();
-    GetFrameSinkManager()->surface_manager()->RegisterFrameSinkId(
-        frame_sink_id_);
+    GetHostFrameSinkManager()->RegisterFrameSinkId(frame_sink_id_, this);
   }
 #endif
 
@@ -90,10 +90,9 @@ TestRenderWidgetHostView::TestRenderWidgetHostView(RenderWidgetHost* rwh)
 }
 
 TestRenderWidgetHostView::~TestRenderWidgetHostView() {
-  viz::FrameSinkManagerImpl* manager = GetFrameSinkManager();
-  if (manager) {
-    manager->surface_manager()->InvalidateFrameSinkId(frame_sink_id_);
-  }
+  viz::HostFrameSinkManager* manager = GetHostFrameSinkManager();
+  if (manager)
+    manager->InvalidateFrameSinkId(frame_sink_id_);
 }
 
 RenderWidgetHost* TestRenderWidgetHostView::GetRenderWidgetHost() const {
@@ -221,6 +220,12 @@ viz::FrameSinkId TestRenderWidgetHostView::GetFrameSinkId() {
   return frame_sink_id_;
 }
 
+void TestRenderWidgetHostView::OnSurfaceCreated(
+    const viz::SurfaceInfo& surface_info) {
+  // TODO(fsamuel): Once surface synchronization is turned on, the fallback
+  // surface should be set here.
+}
+
 TestRenderViewHost::TestRenderViewHost(
     SiteInstance* instance,
     std::unique_ptr<RenderWidgetHostImpl> widget,

content/test/test_render_view_host.h

@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "components/viz/common/surfaces/frame_sink_id.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/public/common/web_preferences.h"
@@ -60,7 +61,9 @@ void InitNavigateParams(FrameHostMsg_DidCommitProvisionalLoad_Params* params,
 
 // Subclass the RenderViewHost's view so that we can call Show(), etc.,
 // without having side-effects.
-class TestRenderWidgetHostView : public RenderWidgetHostViewBase {
+class TestRenderWidgetHostView
+    : public RenderWidgetHostViewBase,
+      public NON_EXPORTED_BASE(viz::HostFrameSinkClient) {
  public:
   explicit TestRenderWidgetHostView(RenderWidgetHost* rwh);
   ~TestRenderWidgetHostView() override;
@@ -128,6 +131,9 @@ class TestRenderWidgetHostView : public RenderWidgetHostViewBase {
     did_change_compositor_frame_sink_ = false;
   }
 
+  // viz::HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
+
  protected:
   RenderWidgetHostImpl* rwh_;
   viz::FrameSinkId frame_sink_id_;

ui/android/delegated_frame_host_android.cc

@@ -65,7 +65,7 @@ DelegatedFrameHostAndroid::DelegatedFrameHostAndroid(
   DCHECK(view_);
   DCHECK(client_);
 
-  frame_sink_manager_->surface_manager()->RegisterFrameSinkId(frame_sink_id_);
+  host_frame_sink_manager_->RegisterFrameSinkId(frame_sink_id_, this);
   CreateNewCompositorFrameSinkSupport();
 }
 
@@ -73,7 +73,7 @@ DelegatedFrameHostAndroid::~DelegatedFrameHostAndroid() {
   DestroyDelegatedContent();
   DetachFromCompositor();
   support_.reset();
-  frame_sink_manager_->surface_manager()->InvalidateFrameSinkId(frame_sink_id_);
+  host_frame_sink_manager_->InvalidateFrameSinkId(frame_sink_id_);
 }
 
 void DelegatedFrameHostAndroid::SubmitCompositorFrame(
@@ -202,6 +202,12 @@ void DelegatedFrameHostAndroid::OnNeedsBeginFrames(bool needs_begin_frames) {
   support_->SetNeedsBeginFrame(needs_begin_frames);
 }
 
+void DelegatedFrameHostAndroid::OnSurfaceCreated(
+    const viz::SurfaceInfo& surface_info) {
+  // TODO(fsamuel): Once surface synchronization is turned on, the fallback
+  // surface should be set here.
+}
+
 void DelegatedFrameHostAndroid::CreateNewCompositorFrameSinkSupport() {
   constexpr bool is_root = false;
   constexpr bool handles_frame_sink_id_invalidation = false;

ui/android/delegated_frame_host_android.h

@@ -10,6 +10,7 @@
 #include "components/viz/common/quads/copy_output_request.h"
 #include "components/viz/common/resources/returned_resource.h"
 #include "components/viz/common/surfaces/surface_info.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support_client.h"
 #include "ui/android/ui_android_export.h"
@@ -33,7 +34,8 @@ class WindowAndroidCompositor;
 
 class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
     : public viz::CompositorFrameSinkSupportClient,
-      public viz::ExternalBeginFrameSourceClient {
+      public viz::ExternalBeginFrameSourceClient,
+      public viz::HostFrameSinkClient {
  public:
   class Client {
    public:
@@ -92,6 +94,9 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   // viz::ExternalBeginFrameSourceClient implementation.
   void OnNeedsBeginFrames(bool needs_begin_frames) override;
 
+  // viz::HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
+
   void CreateNewCompositorFrameSinkSupport();
 
   const viz::FrameSinkId frame_sink_id_;

ui/compositor/compositor.cc

@@ -73,9 +73,9 @@ Compositor::Compositor(const viz::FrameSinkId& frame_sink_id,
       weak_ptr_factory_(this),
       lock_timeout_weak_ptr_factory_(this) {
   if (context_factory_private) {
-    context_factory_private->GetFrameSinkManager()
-        ->surface_manager()
-        ->RegisterFrameSinkId(frame_sink_id_);
+    auto* host_frame_sink_manager =
+        context_factory_private_->GetHostFrameSinkManager();
+    host_frame_sink_manager->RegisterFrameSinkId(frame_sink_id_, this);
   }
   root_web_layer_ = cc::Layer::Create();
 
@@ -227,9 +227,7 @@ Compositor::~Compositor() {
       host_frame_sink_manager->UnregisterFrameSinkHierarchy(frame_sink_id_,
                                                             client);
     }
-    context_factory_private_->GetFrameSinkManager()
-        ->surface_manager()
-        ->InvalidateFrameSinkId(frame_sink_id_);
+    host_frame_sink_manager->InvalidateFrameSinkId(frame_sink_id_);
   }
 }
 
@@ -552,6 +550,11 @@ void Compositor::DidSubmitCompositorFrame() {
     observer.OnCompositingStarted(this, start_time);
 }
 
+void Compositor::OnSurfaceCreated(const viz::SurfaceInfo& surface_info) {
+  // TODO(fsamuel): Once surface synchronization is turned on, the fallback
+  // surface should be set here.
+}
+
 void Compositor::SetOutputIsSecure(bool output_is_secure) {
   if (context_factory_private_)
     context_factory_private_->SetOutputIsSecure(this, output_is_secure);

ui/compositor/compositor.h

@@ -21,6 +21,7 @@
 #include "cc/trees/layer_tree_host_single_thread_client.h"
 #include "components/viz/common/frame_sinks/begin_frame_args.h"
 #include "components/viz/common/surfaces/surface_sequence.h"
+#include "components/viz/host/host_frame_sink_client.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/compositor/compositor_animation_observer.h"
 #include "ui/compositor/compositor_export.h"
@@ -185,7 +186,8 @@ class COMPOSITOR_EXPORT ContextFactory {
 class COMPOSITOR_EXPORT Compositor
     : NON_EXPORTED_BASE(public cc::LayerTreeHostClient),
       NON_EXPORTED_BASE(public cc::LayerTreeHostSingleThreadClient),
-      NON_EXPORTED_BASE(public CompositorLockDelegate) {
+      NON_EXPORTED_BASE(public CompositorLockDelegate),
+      NON_EXPORTED_BASE(public viz::HostFrameSinkClient) {
  public:
   Compositor(const viz::FrameSinkId& frame_sink_id,
              ui::ContextFactory* context_factory,
@@ -381,6 +383,9 @@ class COMPOSITOR_EXPORT Compositor
   void DidSubmitCompositorFrame() override;
   void DidLoseLayerTreeFrameSink() override {}
 
+  // viz::HostFrameSinkClient implementation.
+  void OnSurfaceCreated(const viz::SurfaceInfo& surface_info) override;
+
   bool IsLocked() { return !active_locks_.empty(); }
 
   void SetOutputIsSecure(bool output_is_secure);