Add observers to FrameSinkManagerHost.

content/browser/renderer_host/offscreen_canvas_surface_impl.cc

 // Copyright 2016 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.
 
 #include "content/browser/renderer_host/offscreen_canvas_surface_impl.h"
 
 #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() {
   if (has_created_compositor_frame_sink_) {
     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(
     cc::mojom::MojoCompositorFrameSinkClientPtr client,
     cc::mojom::MojoCompositorFrameSinkRequest request) {
   if (has_created_compositor_frame_sink_) {
     DLOG(ERROR) << "CreateCompositorFrameSink() called more than once.";
     return;
   }
 
   GetFrameSinkManagerHost()->CreateCompositorFrameSink(
       frame_sink_id_, std::move(request),
       mojo::MakeRequest(&compositor_frame_sink_private_), std::move(client));
 
   GetFrameSinkManagerHost()->RegisterFrameSinkHierarchy(parent_frame_sink_id_,
                                                         frame_sink_id_);
   has_created_compositor_frame_sink_ = true;
 }
 
 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);
   }
 }
 
 void OffscreenCanvasSurfaceImpl::Require(const cc::SurfaceId& surface_id,
                                          const cc::SurfaceSequence& sequence) {
   GetSurfaceManager()->RequireSequence(surface_id, sequence);
 }
 
 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