cc: Make OutputSurface::BindToClient pure virtual and not return bool

The function does not need to return bool and we DCHECK it is always
true already, since the ContextProvider given to the OutputSurface can
be initialized before even creating the OutputSurface as it is not
used across threads anymore.

Since it is not cross-thread the base class code in BindToClient can
move to its constructor and the method can become pure virtual.

Drops the DidLoseOutputSurface from the OutputSurfaceClient interface
so that every implementation doesn't need to bind it - esp since only
OutputSurfaces with a context can even get lost. Instead have Display
listen to the context for loss.

And some random code cleanups in AndroidOutputSurface since it was
working around multiple threads that don't exist.

R=enne@chromium.org, piman@chromium.org
BUG=606056
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2443003004
Cr-Commit-Position: refs/heads/master@{#427511}

android_webview/browser/parent_output_surface.cc

@@ -20,10 +20,7 @@ ParentOutputSurface::ParentOutputSurface(
 ParentOutputSurface::~ParentOutputSurface() {
 }
 
-void ParentOutputSurface::DidLoseOutputSurface() {
-  // Android WebView does not handle context loss.
-  LOG(FATAL) << "Render thread context loss";
-}
+void ParentOutputSurface::BindToClient(cc::OutputSurfaceClient* client) {}
 
 void ParentOutputSurface::EnsureBackbuffer() {}
 

android_webview/browser/parent_output_surface.h

@@ -18,7 +18,7 @@ class ParentOutputSurface : NON_EXPORTED_BASE(public cc::OutputSurface) {
   ~ParentOutputSurface() override;
 
   // OutputSurface overrides.
-  void DidLoseOutputSurface() override;
+  void BindToClient(cc::OutputSurfaceClient* client) override;
   void EnsureBackbuffer() override;
   void DiscardBackbuffer() override;
   void BindFramebuffer() override;

android_webview/browser/surfaces_instance.cc

@@ -92,6 +92,11 @@ SurfacesInstance::~SurfacesInstance() {
   surface_manager_->InvalidateFrameSinkId(frame_sink_id_);
 }
 
+void SurfacesInstance::DisplayOutputSurfaceLost() {
+  // Android WebView does not handle context loss.
+  LOG(FATAL) << "Render thread context loss";
+}
+
 cc::FrameSinkId SurfacesInstance::AllocateFrameSinkId() {
   return cc::FrameSinkId(next_client_id_++, 0 /* sink_id */);
 }

android_webview/browser/surfaces_instance.h

@@ -58,7 +58,7 @@ class SurfacesInstance : public base::RefCounted<SurfacesInstance>,
   ~SurfacesInstance() override;
 
   // cc::DisplayClient overrides.
-  void DisplayOutputSurfaceLost() override {}
+  void DisplayOutputSurfaceLost() override;
   void DisplayWillDrawAndSwap(
       bool will_draw_and_swap,
       const cc::RenderPassList& render_passes) override {}

blimp/client/support/compositor/blimp_embedder_compositor.cc

@@ -51,6 +51,10 @@ class DisplayOutputSurface : public cc::OutputSurface {
   ~DisplayOutputSurface() override = default;
 
   // cc::OutputSurface implementation
+  void BindToClient(cc::OutputSurfaceClient* client) override {
+    client_ = client;
+  }
+
   void EnsureBackbuffer() override {}
   void DiscardBackbuffer() override {
     context_provider()->ContextGL()->DiscardBackbufferCHROMIUM();
@@ -90,6 +94,7 @@ class DisplayOutputSurface : public cc::OutputSurface {
  private:
   void SwapBuffersCallback() { client_->DidReceiveSwapBuffersAck(); }
 
+  cc::OutputSurfaceClient* client_ = nullptr;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   base::WeakPtrFactory<DisplayOutputSurface> weak_ptr_factory_;
 
@@ -194,6 +199,7 @@ void BlimpEmbedderCompositor::HandlePendingCompositorFrameSinkRequest() {
     return;
 
   DCHECK(context_provider_);
+  context_provider_->BindToCurrentThread();
 
   gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager =
       compositor_dependencies_->GetGpuMemoryBufferManager();

cc/BUILD.gn

@@ -899,7 +899,6 @@ test("cc_unittests") {
     "output/gl_renderer_unittest.cc",
     "output/layer_quad_unittest.cc",
     "output/managed_memory_policy_unittest.cc",
-    "output/output_surface_unittest.cc",
     "output/overlay_unittest.cc",
     "output/renderer_pixeltest.cc",
     "output/renderer_settings_unittest.cc",

cc/output/context_provider.h

@@ -74,6 +74,10 @@ class ContextProvider : public base::RefCountedThreadSafe<ContextProvider> {
   // Sets a callback to be called when the context is lost. This should be
   // called from the same thread that the context is bound to. To avoid races,
   // it should be called before BindToCurrentThread().
+  // Implementation note: Implementations must avoid post-tasking the provided
+  // |lost_context_callback| directly as clients expect the method to not be
+  // called once they call SetLostContextCallback() again with a different
+  // callback.
   typedef base::Closure LostContextCallback;
   virtual void SetLostContextCallback(
       const LostContextCallback& lost_context_callback) = 0;