Add observers to FrameSinkManagerHost.

content/browser/renderer_host/offscreen_canvas_surface_impl.cc

Index: content/browser/renderer_host/offscreen_canvas_surface_impl.cc
diff --git a/content/browser/renderer_host/offscreen_canvas_surface_impl.cc b/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
index 41454934c212fc4be04c764a02c6a4adab6e08c3..bcd67a4c42192af6bef747e5f202a0a81fea8781 100644
--- a/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
+++ b/content/browser/renderer_host/offscreen_canvas_surface_impl.cc
@@ -7,24 +7,29 @@
 #include <memory>
 #include <utility>
 
-#include "base/bind_helpers.h"
 #include "base/memory/ptr_util.h"
 #include "cc/surfaces/surface_manager.h"
 #include "content/browser/compositor/frame_sink_manager_host.h"
 #include "content/browser/compositor/surface_utils.h"
-#include "content/browser/renderer_host/offscreen_canvas_compositor_frame_sink_manager.h"
+#include "content/browser/renderer_host/offscreen_canvas_provider_impl.h"
 
 namespace content {
 
 OffscreenCanvasSurfaceImpl::OffscreenCanvasSurfaceImpl(
     const cc::FrameSinkId& parent_frame_sink_id,
     const cc::FrameSinkId& frame_sink_id,
-    blink::mojom::OffscreenCanvasSurfaceClientPtr client)
+    OffscreenCanvasProviderImpl* provider,
+    blink::mojom::OffscreenCanvasSurfaceClientPtr client,
+    blink::mojom::OffscreenCanvasSurfaceRequest request)
     : client_(std::move(client)),
+      binding_(this, std::move(request)),
+      provider_(provider),
       frame_sink_id_(frame_sink_id),
       parent_frame_sink_id_(parent_frame_sink_id) {
-  OffscreenCanvasCompositorFrameSinkManager::GetInstance()
-      ->RegisterOffscreenCanvasSurfaceInstance(frame_sink_id_, this);
+  binding_.set_connection_error_handler(
+      base::Bind(&OffscreenCanvasSurfaceImpl::OnSurfaceConnectionClosed,
+                 base::Unretained(this)));
+  GetFrameSinkManagerHost()->AddObserver(this);
 }
 
 OffscreenCanvasSurfaceImpl::~OffscreenCanvasSurfaceImpl() {
@@ -32,22 +37,7 @@ OffscreenCanvasSurfaceImpl::~OffscreenCanvasSurfaceImpl() {
     GetFrameSinkManagerHost()->UnregisterFrameSinkHierarchy(
         parent_frame_sink_id_, frame_sink_id_);
   }
-  OffscreenCanvasCompositorFrameSinkManager::GetInstance()
-      ->UnregisterOffscreenCanvasSurfaceInstance(frame_sink_id_);
-}
-
-// static
-void OffscreenCanvasSurfaceImpl::Create(
-    const cc::FrameSinkId& parent_frame_sink_id,
-    const cc::FrameSinkId& frame_sink_id,
-    blink::mojom::OffscreenCanvasSurfaceClientPtr client,
-    blink::mojom::OffscreenCanvasSurfaceRequest request) {
-  std::unique_ptr<OffscreenCanvasSurfaceImpl> impl =
-      base::MakeUnique<OffscreenCanvasSurfaceImpl>(
-          parent_frame_sink_id, frame_sink_id, std::move(client));
-  OffscreenCanvasSurfaceImpl* surface_service = impl.get();
-  surface_service->binding_ =
-      mojo::MakeStrongBinding(std::move(impl), std::move(request));
+  GetFrameSinkManagerHost()->RemoveObserver(this);
 }
 
 void OffscreenCanvasSurfaceImpl::CreateCompositorFrameSink(
@@ -69,10 +59,11 @@ void OffscreenCanvasSurfaceImpl::CreateCompositorFrameSink(
 
 void OffscreenCanvasSurfaceImpl::OnSurfaceCreated(
     const cc::SurfaceInfo& surface_info) {
-  DCHECK_EQ(surface_info.id().frame_sink_id(), frame_sink_id_);
-  if (!current_local_surface_id_.is_valid() ||
-      surface_info.id().local_surface_id() != current_local_surface_id_) {
-    current_local_surface_id_ = surface_info.id().local_surface_id();
+  if (surface_info.id().frame_sink_id() != frame_sink_id_)
+    return;
+
+  if (surface_info.id().local_surface_id() != local_surface_id_) {

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

When would this ever be the case? What's the point of this condition?

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

Hmmm, in the future when MojoFrameSinkManager is in the GPU and the GPU restarts
it all I can think of? Although in that case the message is still useful? I can
remove the check, it's unlikely to cause problems if did send OnSurfaceCreated
twice.

+    local_surface_id_ = surface_info.id().local_surface_id();
     if (client_)
       client_->OnSurfaceCreated(surface_info);
   }
@@ -87,4 +78,8 @@ void OffscreenCanvasSurfaceImpl::Satisfy(const cc::SurfaceSequence& sequence) {
   GetSurfaceManager()->SatisfySequence(sequence);
 }
 
+void OffscreenCanvasSurfaceImpl::OnSurfaceConnectionClosed() {
+  provider_->DestroyOffscreenCanvasSurface(frame_sink_id_);

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

DestroyOffscreenCanvasSurface should take the framesinkid by value, definitely
to avoid use after free.

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

Good catch.

+}
+
 }  // namespace content