Add observers to FrameSinkManagerHost.

content/browser/compositor/frame_sink_manager_host.h

Index: content/browser/compositor/frame_sink_manager_host.h
diff --git a/content/browser/compositor/frame_sink_manager_host.h b/content/browser/compositor/frame_sink_manager_host.h
index 8c7865ddbef16db670fc221000383d1d93c2663d..6d600e61223101b32f82acadfc514310eb56fef5 100644
--- a/content/browser/compositor/frame_sink_manager_host.h
+++ b/content/browser/compositor/frame_sink_manager_host.h
@@ -7,9 +7,11 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/observer_list.h"
 #include "cc/ipc/frame_sink_manager.mojom.h"
 #include "cc/surfaces/frame_sink_id.h"
 #include "components/viz/frame_sinks/mojo_frame_sink_manager.h"
+#include "content/browser/compositor/frame_sink_observer.h"
 #include "content/common/content_export.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -33,6 +35,9 @@ class CONTENT_EXPORT FrameSinkManagerHost
   // Start Mojo connection to FrameSinkManager. Most tests won't need this.
   void ConnectToFrameSinkManager();
 
+  void AddObserver(FrameSinkObserver* observer);

[Fady Samuel, 2017/04/28 04:58:00]

I had envisioned this would take in a FrameSinkId

AddObserver(const cc::FrameSinkId& frame_sink_id, FrameSinkObserver* observer);
RemoveObserver(const cc::FrameSinkId& frame_sink_id);

I'm not sure how multi-embedder case would work. Maybe the end point would
forward the SurfaceInfo to other embedders?

[kylechar, 2017/05/01 19:07:48]

After doing a quick implementation with unordered_map<FrameSinkId,
FrameSinkObserver*> I'm not sold on this. Handling the simple 1:1 case, the only
thing that is eliminated is an if statement in each observer.

However, AddObserver() then needs to handle (1) what if there is already an
observer for FrameSinkId and (2) what if the observer is already the observer
for something else. RemoveObserver() needs to find the FrameSinkId that should
be removed.

If we allow a FrameSinkObserver to be an observer for more than one FrameSinkId,
then it adds complexity in the observer and something like
RemoveObserver(FrameSinkId, FrameSinkObserver*) too.

If we want multiple things as an observer for one FrameSinkId one day then it's
more complexity still.

This would all need tests too. So I think using base::ObserverList<> is best way
overall, it works for all cases and it already has tests.

+  void RemoveObserver(FrameSinkObserver* observer);
+
   // See frame_sink_manager.mojom for descriptions.
   void CreateCompositorFrameSink(
       const cc::FrameSinkId& frame_sink_id,
@@ -59,6 +64,9 @@ class CONTENT_EXPORT FrameSinkManagerHost
   // happen using Mojo. See http://crbug.com/657959.
   viz::MojoFrameSinkManager frame_sink_manager_;
 
+  // Local observers to that receive OnSurfaceCreated() messages from IPC.
+  base::ObserverList<FrameSinkObserver> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(FrameSinkManagerHost);
 };