Delete viz::FrameSinkManagerClient and cleanup.

This CL deletes viz::FrameSinkManagerClient. FrameSinkManagerImpl has a
pointer to each CompositorFrameSinkSupport directly now since this is
needed anyways. The FrameSinkManagerImpl::clients_ map is removed as a
result.

SinkAndSupport will always contain a CompositorFrameSinkSupport* while
the CompositorFrameSinkSupport object exists. SinkAndSupport::sink will
be non-null when FrameSinkManagerImpl owns the compositor frame sink.

CompositorFrameSinkSupport::Init()/Create() are also no longer needed
since SetBeginFrameSource() is no longer virtual. Remove Init() and make
the constructor public. Create() is left since it's widely used for now
but is marked deprecated.

Also remove some old/wrong forward definitions and fix some style guide
violations.

Bug: 792192
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I14f9662c9db485ce18edf1d358493648997d3648
Reviewed-on: https://chromium-review.googlesource.com/810866Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522323}

components/viz/service/BUILD.gn

@@ -99,7 +99,6 @@ viz_component("service") {
     "frame_sinks/compositor_frame_sink_support_manager.h",
     "frame_sinks/direct_layer_tree_frame_sink.cc",
     "frame_sinks/direct_layer_tree_frame_sink.h",
-    "frame_sinks/frame_sink_manager_client.h",
     "frame_sinks/frame_sink_manager_impl.cc",
     "frame_sinks/frame_sink_manager_impl.h",
     "frame_sinks/primary_begin_frame_source.cc",

components/viz/service/frame_sinks/compositor_frame_sink_support.cc

@@ -24,17 +24,29 @@ std::unique_ptr<CompositorFrameSinkSupport> CompositorFrameSinkSupport::Create(
     const FrameSinkId& frame_sink_id,
     bool is_root,
     bool needs_sync_tokens) {
-  std::unique_ptr<CompositorFrameSinkSupport> support =
-      base::WrapUnique(new CompositorFrameSinkSupport(
-          client, frame_sink_id, is_root, needs_sync_tokens));
-  support->Init(frame_sink_manager);
-  return support;
+  return std::make_unique<CompositorFrameSinkSupport>(
+      client, frame_sink_manager, frame_sink_id, is_root, needs_sync_tokens);
 }
 
-CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
-  // No video capture clients should remain at this point.
-  DCHECK(capture_clients_.empty());
+CompositorFrameSinkSupport::CompositorFrameSinkSupport(
+    mojom::CompositorFrameSinkClient* client,
+    FrameSinkManagerImpl* frame_sink_manager,
+    const FrameSinkId& frame_sink_id,
+    bool is_root,
+    bool needs_sync_tokens)
+    : client_(client),
+      frame_sink_manager_(frame_sink_manager),
+      surface_manager_(frame_sink_manager->surface_manager()),
+      frame_sink_id_(frame_sink_id),
+      surface_resource_holder_(this),
+      is_root_(is_root),
+      needs_sync_tokens_(needs_sync_tokens),
+      weak_factory_(this) {
+  // This may result in SetBeginFrameSource() being called.
+  frame_sink_manager_->RegisterCompositorFrameSinkSupport(frame_sink_id_, this);
+}
 
+CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
   if (!destruction_callback_.is_null())
     std::move(destruction_callback_).Run();
 
@@ -51,7 +63,10 @@ CompositorFrameSinkSupport::~CompositorFrameSinkSupport() {
   }
 
   EvictCurrentSurface();
-  frame_sink_manager_->UnregisterFrameSinkManagerClient(frame_sink_id_);
+  frame_sink_manager_->UnregisterCompositorFrameSinkSupport(frame_sink_id_);
+
+  // No video capture clients should remain at this point.
+  DCHECK(capture_clients_.empty());
 }
 
 void CompositorFrameSinkSupport::SetAggregatedDamageCallback(
@@ -64,6 +79,16 @@ void CompositorFrameSinkSupport::SetDestructionCallback(
   destruction_callback_ = std::move(callback);
 }
 
+void CompositorFrameSinkSupport::SetBeginFrameSource(
+    BeginFrameSource* begin_frame_source) {
+  if (begin_frame_source_ && added_frame_observer_) {
+    begin_frame_source_->RemoveObserver(this);
+    added_frame_observer_ = false;
+  }
+  begin_frame_source_ = begin_frame_source;
+  UpdateNeedsBeginFramesInternal();
+}
+
 void CompositorFrameSinkSupport::OnSurfaceActivated(Surface* surface) {
   DCHECK(surface);
   DCHECK(surface->HasActiveFrame());
@@ -103,16 +128,6 @@ void CompositorFrameSinkSupport::ReceiveFromChild(
   surface_resource_holder_.ReceiveFromChild(resources);
 }
 
-void CompositorFrameSinkSupport::SetBeginFrameSource(
-    BeginFrameSource* begin_frame_source) {
-  if (begin_frame_source_ && added_frame_observer_) {
-    begin_frame_source_->RemoveObserver(this);
-    added_frame_observer_ = false;
-  }
-  begin_frame_source_ = begin_frame_source;
-  UpdateNeedsBeginFramesInternal();
-}
-
 void CompositorFrameSinkSupport::EvictCurrentSurface() {
   if (!current_surface_id_.is_valid())
     return;
@@ -330,25 +345,6 @@ void CompositorFrameSinkSupport::DidPresentCompositorFrame(
   }
 }
 
-CompositorFrameSinkSupport::CompositorFrameSinkSupport(
-    mojom::CompositorFrameSinkClient* client,
-    const FrameSinkId& frame_sink_id,
-    bool is_root,
-    bool needs_sync_tokens)
-    : client_(client),
-      frame_sink_id_(frame_sink_id),
-      surface_resource_holder_(this),
-      is_root_(is_root),
-      needs_sync_tokens_(needs_sync_tokens),
-      weak_factory_(this) {}
-
-void CompositorFrameSinkSupport::Init(
-    FrameSinkManagerImpl* frame_sink_manager) {
-  frame_sink_manager_ = frame_sink_manager;
-  surface_manager_ = frame_sink_manager->surface_manager();
-  frame_sink_manager_->RegisterFrameSinkManagerClient(frame_sink_id_, this);
-}
-
 void CompositorFrameSinkSupport::OnBeginFrame(const BeginFrameArgs& args) {
   UpdateNeedsBeginFramesInternal();
   if (current_surface_id_.is_valid())

components/viz/service/frame_sinks/compositor_frame_sink_support.h

@@ -15,7 +15,6 @@
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "components/viz/common/quads/compositor_frame.h"
 #include "components/viz/common/surfaces/surface_info.h"
-#include "components/viz/service/frame_sinks/frame_sink_manager_client.h"
 #include "components/viz/service/frame_sinks/referenced_surface_tracker.h"
 #include "components/viz/service/frame_sinks/surface_resource_holder.h"
 #include "components/viz/service/frame_sinks/surface_resource_holder_client.h"
@@ -34,7 +33,6 @@ class SurfaceManager;
 class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
     : public BeginFrameObserver,
       public SurfaceResourceHolderClient,
-      public FrameSinkManagerClient,
       public SurfaceClient,
       public CapturableFrameSink,
       public mojom::CompositorFrameSink {
@@ -45,6 +43,7 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
 
   static const uint64_t kFrameIndexStart = 2;
 
+  // DEPRECATED(kylechar): It's now possible to use the constructor directly.
   static std::unique_ptr<CompositorFrameSinkSupport> Create(
       mojom::CompositorFrameSinkClient* client,
       FrameSinkManagerImpl* frame_sink_manager,
@@ -52,6 +51,11 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
       bool is_root,
       bool needs_sync_tokens);
 
+  CompositorFrameSinkSupport(mojom::CompositorFrameSinkClient* client,
+                             FrameSinkManagerImpl* frame_sink_manager,
+                             const FrameSinkId& frame_sink_id,
+                             bool is_root,
+                             bool needs_sync_tokens);
   ~CompositorFrameSinkSupport() override;
 
   const FrameSinkId& frame_sink_id() const { return frame_sink_id_; }
@@ -71,6 +75,9 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
   // Sets callback called on destruction.
   void SetDestructionCallback(base::OnceCallback<void()> callback);
 
+  // This allows the FrameSinkManagerImpl to pass a BeginFrameSource to use.
+  void SetBeginFrameSource(BeginFrameSource* begin_frame_source);
+
   // SurfaceClient implementation.
   void OnSurfaceActivated(Surface* surface) override;
   void RefResources(
@@ -80,9 +87,6 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
   void ReceiveFromChild(
       const std::vector<TransferableResource>& resources) override;
 
-  // FrameSinkManagerClient implementation.
-  void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override;
-
   // mojom::CompositorFrameSink implementation.
   void SetNeedsBeginFrame(bool needs_begin_frame) override;
   void DidNotProduceFrame(const BeginFrameAck& ack) override;
@@ -114,13 +118,6 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
  private:
   friend class FrameSinkManagerTest;
 
-  CompositorFrameSinkSupport(mojom::CompositorFrameSinkClient* client,
-                             const FrameSinkId& frame_sink_id,
-                             bool is_root,
-                             bool needs_sync_tokens);
-
-  void Init(FrameSinkManagerImpl* frame_sink_manager);
-
   // Updates surface references using |active_referenced_surfaces| from the most
   // recent CompositorFrame. This will add and remove top-level root references
   // if |is_root_| is true and |local_surface_id| has changed. Modifies surface
@@ -152,8 +149,8 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
 
   mojom::CompositorFrameSinkClient* const client_;
 
-  FrameSinkManagerImpl* frame_sink_manager_ = nullptr;
-  SurfaceManager* surface_manager_ = nullptr;
+  FrameSinkManagerImpl* const frame_sink_manager_;
+  SurfaceManager* const surface_manager_;
 
   const FrameSinkId frame_sink_id_;
   SurfaceId current_surface_id_;

components/viz/service/frame_sinks/frame_sink_manager_client.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 COMPONENTS_VIZ_SERVICE_FRAME_SINKS_FRAME_SINK_MANAGER_CLIENT_H_
-#define COMPONENTS_VIZ_SERVICE_FRAME_SINKS_FRAME_SINK_MANAGER_CLIENT_H_
-
-#include "components/viz/service/viz_service_export.h"
-
-namespace cc {
-class BeginFrameSource;
-}
-
-namespace viz {
-
-class VIZ_SERVICE_EXPORT FrameSinkManagerClient {
- public:
-  virtual ~FrameSinkManagerClient() = default;
-
-  // This allows the FrameSinkManagerImpl to pass a BeginFrameSource to use.
-  virtual void SetBeginFrameSource(BeginFrameSource* begin_frame_source) = 0;
-};
-
-}  // namespace viz
-
-#endif  // COMPONENTS_VIZ_SERVICE_FRAME_SINKS_FRAME_SINK_MANAGER_CLIENT_H_

components/viz/service/frame_sinks/frame_sink_manager_impl.cc

@@ -12,7 +12,6 @@
 #include "components/viz/service/display_embedder/display_provider.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_impl.h"
 #include "components/viz/service/frame_sinks/compositor_frame_sink_support.h"
-#include "components/viz/service/frame_sinks/frame_sink_manager_client.h"
 #include "components/viz/service/frame_sinks/primary_begin_frame_source.h"
 #include "components/viz/service/frame_sinks/root_compositor_frame_sink_impl.h"
 #include "components/viz/service/frame_sinks/video_capture/capturable_frame_sink.h"
@@ -34,8 +33,14 @@ FrameSinkManagerImpl::FrameSinkSourceMapping::~FrameSinkSourceMapping() =
 
 FrameSinkManagerImpl::SinkAndSupport::SinkAndSupport() = default;
 
+FrameSinkManagerImpl::SinkAndSupport::SinkAndSupport(SinkAndSupport&& other) =
+    default;
+
 FrameSinkManagerImpl::SinkAndSupport::~SinkAndSupport() = default;
 
+FrameSinkManagerImpl::SinkAndSupport& FrameSinkManagerImpl::SinkAndSupport::
+operator=(SinkAndSupport&& other) = default;
+
 FrameSinkManagerImpl::FrameSinkManagerImpl(
     SurfaceManager::LifetimeType lifetime_type,
     DisplayProvider* display_provider)
@@ -52,7 +57,6 @@ FrameSinkManagerImpl::~FrameSinkManagerImpl() {
   // All FrameSinks should be unregistered prior to FrameSinkManager
   // destruction.
   compositor_frame_sinks_.clear();
-  DCHECK_EQ(clients_.size(), 0u);
   DCHECK_EQ(registered_sources_.size(), 0u);
   surface_manager_.RemoveObserver(this);
   surface_manager_.RemoveObserver(&hit_test_manager_);
@@ -88,10 +92,16 @@ void FrameSinkManagerImpl::RegisterFrameSinkId(
 void FrameSinkManagerImpl::InvalidateFrameSinkId(
     const FrameSinkId& frame_sink_id) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  compositor_frame_sinks_.erase(frame_sink_id);
   surface_manager_.InvalidateFrameSinkId(frame_sink_id);
   if (video_detector_)
     video_detector_->OnFrameSinkIdInvalidated(frame_sink_id);
+
+  // Destroy the [Root]CompositorFrameSinkImpl if there is one. This will result
+  // in UnregisterCompositorFrameSinkSupport() being called and |iter| will be
+  // invalidated afterwards
+  auto iter = compositor_frame_sinks_.find(frame_sink_id);
+  if (iter != compositor_frame_sinks_.end())
+    iter->second.sink.reset();
 }
 
 void FrameSinkManagerImpl::SetFrameSinkDebugLabel(
@@ -123,7 +133,7 @@ void FrameSinkManagerImpl::CreateRootCompositorFrameSink(
       std::move(request), std::move(client),
       std::move(display_private_request));
   SinkAndSupport& entry = compositor_frame_sinks_[frame_sink_id];
-  entry.support = frame_sink->support();
+  DCHECK(entry.support);  // |entry| was created by RootCompositorFrameSinkImpl.
   entry.sink = std::move(frame_sink);
 }
 
@@ -137,7 +147,7 @@ void FrameSinkManagerImpl::CreateCompositorFrameSink(
   auto frame_sink = std::make_unique<CompositorFrameSinkImpl>(
       this, frame_sink_id, std::move(request), std::move(client));
   SinkAndSupport& entry = compositor_frame_sinks_[frame_sink_id];
-  entry.support = frame_sink->support();
+  DCHECK(entry.support);  // |entry| was created by CompositorFrameSinkImpl.
   entry.sink = std::move(frame_sink);
 }
 
@@ -150,8 +160,7 @@ void FrameSinkManagerImpl::RegisterFrameSinkHierarchy(
 
   std::vector<FrameSinkId>& children =
       frame_sink_source_map_[parent_frame_sink_id].children;
-  for (size_t i = 0; i < children.size(); ++i)
-    DCHECK(children[i] != child_frame_sink_id);
+  DCHECK(!base::ContainsValue(children, child_frame_sink_id));
   children.push_back(child_frame_sink_id);
 
   // If the parent has no source, then attaching it to this child will
@@ -192,8 +201,8 @@ void FrameSinkManagerImpl::UnregisterFrameSinkHierarchy(
   // The CompositorFrameSinkSupport and hierarchy can be registered/unregistered
   // in either order, so empty frame_sink_source_map entries need to be
   // checked when removing either clients or relationships.
-  if (!iter->second.has_children() && !clients_.count(parent_frame_sink_id) &&
-      !iter->second.source) {
+  if (!iter->second.has_children() && !iter->second.source &&
+      !compositor_frame_sinks_.count(parent_frame_sink_id)) {
     frame_sink_source_map_.erase(iter);
     return;
   }
@@ -219,31 +228,24 @@ void FrameSinkManagerImpl::DropTemporaryReference(const SurfaceId& surface_id) {
   surface_manager_.DropTemporaryReference(surface_id);
 }
 
-void FrameSinkManagerImpl::RegisterFrameSinkManagerClient(
+void FrameSinkManagerImpl::RegisterCompositorFrameSinkSupport(
     const FrameSinkId& frame_sink_id,
-    FrameSinkManagerClient* client) {
-  DCHECK(client);
+    CompositorFrameSinkSupport* support) {
+  DCHECK(support);
 
-  clients_[frame_sink_id] = client;
+  SinkAndSupport& entry = compositor_frame_sinks_[frame_sink_id];
+  DCHECK(!entry.support);
+  entry.support = support;
 
   auto it = frame_sink_source_map_.find(frame_sink_id);
-  if (it != frame_sink_source_map_.end()) {
-    if (it->second.source)
-      client->SetBeginFrameSource(it->second.source);
-  }
+  if (it != frame_sink_source_map_.end() && it->second.source)
+    support->SetBeginFrameSource(it->second.source);
 }
 
-void FrameSinkManagerImpl::UnregisterFrameSinkManagerClient(
+void FrameSinkManagerImpl::UnregisterCompositorFrameSinkSupport(
     const FrameSinkId& frame_sink_id) {
-  auto client_iter = clients_.find(frame_sink_id);
-  DCHECK(client_iter != clients_.end());
-
-  auto source_iter = frame_sink_source_map_.find(frame_sink_id);
-  if (source_iter != frame_sink_source_map_.end()) {
-    if (source_iter->second.source)
-      client_iter->second->SetBeginFrameSource(nullptr);
-  }
-  clients_.erase(client_iter);
+  DCHECK_EQ(compositor_frame_sinks_.count(frame_sink_id), 1u);
+  compositor_frame_sinks_.erase(frame_sink_id);
 }
 
 void FrameSinkManagerImpl::RegisterBeginFrameSource(
@@ -290,9 +292,9 @@ void FrameSinkManagerImpl::RecursivelyAttachBeginFrameSource(
   FrameSinkSourceMapping& mapping = frame_sink_source_map_[frame_sink_id];
   if (!mapping.source) {
     mapping.source = source;
-    auto client_iter = clients_.find(frame_sink_id);
-    if (client_iter != clients_.end())
-      client_iter->second->SetBeginFrameSource(source);
+    auto client_iter = compositor_frame_sinks_.find(frame_sink_id);
+    if (client_iter != compositor_frame_sinks_.end())
+      client_iter->second.support->SetBeginFrameSource(source);
   }
   for (size_t i = 0; i < mapping.children.size(); ++i) {
     // |frame_sink_source_map_| is a container that can allocate new memory and
@@ -312,12 +314,13 @@ void FrameSinkManagerImpl::RecursivelyDetachBeginFrameSource(
     return;
   if (iter->second.source == source) {
     iter->second.source = nullptr;
-    auto client_iter = clients_.find(frame_sink_id);
-    if (client_iter != clients_.end())
-      client_iter->second->SetBeginFrameSource(nullptr);
+    auto client_iter = compositor_frame_sinks_.find(frame_sink_id);
+    if (client_iter != compositor_frame_sinks_.end())
+      client_iter->second.support->SetBeginFrameSource(nullptr);
   }
 
-  if (!iter->second.has_children() && !clients_.count(frame_sink_id)) {
+  if (!iter->second.has_children() &&
+      !compositor_frame_sinks_.count(frame_sink_id)) {
     frame_sink_source_map_.erase(iter);
     return;
   }

components/viz/service/frame_sinks/frame_sink_manager_impl.h

@@ -26,22 +26,11 @@
 #include "services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom.h"
 #include "services/viz/public/interfaces/compositing/video_detector_observer.mojom.h"
 
-namespace cc {
-
-class BeginFrameSource;
-
-namespace test {
-class SurfaceSynchronizationTest;
-}
-
-}  // namespace cc
-
 namespace viz {
 
 class CapturableFrameSink;
 class CompositorFrameSinkSupport;
 class DisplayProvider;
-class FrameSinkManagerClient;
 
 // FrameSinkManagerImpl manages BeginFrame hierarchy. This is the implementation
 // detail for FrameSinkManagerImpl.
@@ -100,12 +89,11 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl : public SurfaceObserver,
   // However, DelegatedFrameHost can register itself as a client before its
   // relationship with the ui::Compositor is known.
 
-  // Associates a FrameSinkManagerClient with the frame_sink_id it uses.
-  // FrameSinkManagerClient and framesink allocators have a 1:1 mapping.
-  // Caller guarantees the client is alive between register/unregister.
-  void RegisterFrameSinkManagerClient(const FrameSinkId& frame_sink_id,
-                                      FrameSinkManagerClient* client);
-  void UnregisterFrameSinkManagerClient(const FrameSinkId& frame_sink_id);
+  // Registers a CompositorFrameSinkSupport for |frame_sink_id|. |frame_sink_id|
+  // must be unregistered when |support| is destroyed.
+  void RegisterCompositorFrameSinkSupport(const FrameSinkId& frame_sink_id,
+                                          CompositorFrameSinkSupport* support);
+  void UnregisterCompositorFrameSinkSupport(const FrameSinkId& frame_sink_id);
 
   // Associates a |source| with a particular framesink.  That framesink and
   // any children of that framesink with valid clients can potentially use
@@ -170,7 +158,31 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl : public SurfaceObserver,
                            uint32_t frame_token);
 
  private:
-  friend class cc::test::SurfaceSynchronizationTest;
+  // BeginFrameSource routing information for a FrameSinkId.
+  struct FrameSinkSourceMapping {
+    FrameSinkSourceMapping();
+    FrameSinkSourceMapping(const FrameSinkSourceMapping& other);
+    ~FrameSinkSourceMapping();
+    bool has_children() const { return !children.empty(); }
+    // The currently assigned begin frame source for this client.
+    BeginFrameSource* source = nullptr;
+    // This represents a dag of parent -> children mapping.
+    std::vector<FrameSinkId> children;
+  };
+
+  struct SinkAndSupport {
+    SinkAndSupport();
+    SinkAndSupport(SinkAndSupport&& other);
+    ~SinkAndSupport();
+    SinkAndSupport& operator=(SinkAndSupport&& other);
+
+    // CompositorFrameSinks owned here. This will be null if a
+    // CompositorFrameSinkSupport is owned externally.
+    std::unique_ptr<mojom::CompositorFrameSink> sink;
+
+    // This can be owned by |sink| or owned externally.
+    CompositorFrameSinkSupport* support = nullptr;
+  };
 
   void RecursivelyAttachBeginFrameSource(const FrameSinkId& frame_sink_id,
                                          BeginFrameSource* source);
@@ -187,24 +199,9 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl : public SurfaceObserver,
   bool ChildContains(const FrameSinkId& child_frame_sink_id,
                      const FrameSinkId& search_frame_sink_id) const;
 
-  // Begin frame source routing. Both BeginFrameSource and
-  // CompositorFrameSinkSupport pointers guaranteed alive by callers until
-  // unregistered.
-  struct FrameSinkSourceMapping {
-    FrameSinkSourceMapping();
-    FrameSinkSourceMapping(const FrameSinkSourceMapping& other);
-    ~FrameSinkSourceMapping();
-    bool has_children() const { return !children.empty(); }
-    // The currently assigned begin frame source for this client.
-    BeginFrameSource* source = nullptr;
-    // This represents a dag of parent -> children mapping.
-    std::vector<FrameSinkId> children;
-  };
-
   // Provides a Display for CreateRootCompositorFrameSink().
   DisplayProvider* const display_provider_;
 
-  base::flat_map<FrameSinkId, FrameSinkManagerClient*> clients_;
   std::unordered_map<FrameSinkId, FrameSinkSourceMapping, FrameSinkIdHash>
       frame_sink_source_map_;
 
@@ -221,14 +218,7 @@ class VIZ_SERVICE_EXPORT FrameSinkManagerImpl : public SurfaceObserver,
 
   HitTestManager hit_test_manager_;
 
-  struct SinkAndSupport {
-    SinkAndSupport();
-    ~SinkAndSupport();
-    std::unique_ptr<mojom::CompositorFrameSink> sink;
-    CompositorFrameSinkSupport* support;  // Owned by |sink|.
-  };
-  std::unordered_map<FrameSinkId, SinkAndSupport, FrameSinkIdHash>
-      compositor_frame_sinks_;
+  base::flat_map<FrameSinkId, SinkAndSupport> compositor_frame_sinks_;
 
   THREAD_CHECKER(thread_checker_);