Create manager to track OffscreenCanvasSurfaceImpl instances

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 "base/bind_helpers.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_manager.h"
 #include "content/browser/compositor/surface_utils.h"
+#include "content/browser/renderer_host/offscreen_canvas_surface_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 
 namespace content {
 
 OffscreenCanvasSurfaceImpl::OffscreenCanvasSurfaceImpl()
-    : id_allocator_(new cc::SurfaceIdAllocator()) {}
+    : id_allocator_(new cc::SurfaceIdAllocator()), weak_factory_(this) {}
 
 OffscreenCanvasSurfaceImpl::~OffscreenCanvasSurfaceImpl() {}
 
 // static
 void OffscreenCanvasSurfaceImpl::Create(
     mojo::InterfaceRequest<blink::mojom::OffscreenCanvasSurface> request) {
   mojo::MakeStrongBinding(base::MakeUnique<OffscreenCanvasSurfaceImpl>(),
                           std::move(request));
 }
 
 void OffscreenCanvasSurfaceImpl::GetSurfaceId(
+    uint32_t canvas_id,
     const GetSurfaceIdCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   cc::LocalFrameId local_frame_id = id_allocator_->GenerateId();
+
+  // This IPC should be only called once for each HTMLCanvasElement. In this
+  // case, surface_id_ is still unset.
+  DCHECK(surface_id_.is_null());

[dcheng, 2016/11/08 18:30:23]

This can't DCHECK, since the renderer could do this an arbitrary number of
times.

Will we violate an invariant if we allow this to be called multiple times?

[xlai (Olivia), 2016/11/10 15:45:11]

This IPC call is supposed to be called only once for the initial set up of
Surface Layer, for each HTMLCanvasElement that has been transferred to
OffscreenCanvas. It is called twice, it means the caller does it wrongly. It's
more like an implementation check to prevent future developers from making a
mistake.

[dcheng, 2016/11/11 08:36:40]

Right, but this is browser code being invoked by the renderer via a remote Mojo
IPC call. The browser can make no assumptions that the renderer is behaving
correctly. Given that, is it safe for this to be a DCHECK, or will this break
important invariants?

   surface_id_ = cc::SurfaceId(AllocateFrameSinkId(), local_frame_id);
+  canvas_id_ = canvas_id;
+
+  OffscreenCanvasSurfaceManager::GetInstance()
+      ->RegisterOffscreenCanvasSurfaceInstance(canvas_id_, this);
 
   callback.Run(surface_id_);
 }
 
 void OffscreenCanvasSurfaceImpl::Require(const cc::SurfaceId& surface_id,
                                          const cc::SurfaceSequence& sequence) {
   cc::SurfaceManager* manager = GetSurfaceManager();
   cc::Surface* surface = manager->GetSurfaceForId(surface_id);
   if (!surface) {
     DLOG(ERROR) << "Attempting to require callback on nonexistent surface";
     return;
   }
   surface->AddDestructionDependency(sequence);
 }
 
 void OffscreenCanvasSurfaceImpl::Satisfy(const cc::SurfaceSequence& sequence) {
   std::vector<uint32_t> sequences;
   sequences.push_back(sequence.sequence);
   cc::SurfaceManager* manager = GetSurfaceManager();
   manager->DidSatisfySequences(sequence.frame_sink_id, &sequences);
 }
 
 }  // namespace content