Move swapcomplete callback into OutputSurface

Things like swap are concepts of an OutputSurface, not a context. Moving the
callbacks to OutputSurface will also reduce cc's knowledge of the context
guts and let us swap out the GL interface.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194394

Review URL: https://codereview.chromium.org/12545018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@194700 0039d316-1c4b-4281-b951-d872f2087c98

18 files changed, 80 insertions, 79 deletions

diff --git a/cc/output/delegating_renderer.cc b/cc/output/delegating_renderer.cc
index a1763e3..850ad0f 100644
--- a/cc/output/delegating_renderer.cc
+++ b/cc/output/delegating_renderer.cc
@@ -70,7 +70,6 @@ bool DelegatingRenderer::Initialize() {
   if (!context3d->makeContextCurrent())
     return false;
 
-  context3d->setContextLostCallback(this);
   context3d->pushGroupMarkerEXT("CompositorContext");
 
   std::string extensions_string =
@@ -118,11 +117,7 @@ bool DelegatingRenderer::Initialize() {
   return true;
 }
 
-DelegatingRenderer::~DelegatingRenderer() {
-  WebGraphicsContext3D* context3d = resource_provider_->GraphicsContext3D();
-  if (context3d)
-    context3d->setContextLostCallback(NULL);
-}
+DelegatingRenderer::~DelegatingRenderer() {}
 
 const RendererCapabilities& DelegatingRenderer::Capabilities() const {
   return capabilities_;
@@ -173,8 +168,6 @@ void DelegatingRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
 void DelegatingRenderer::ReceiveCompositorFrameAck(
     const CompositorFrameAck& ack) {
   resource_provider_->ReceiveFromParent(ack.resources);
-  if (client_->HasImplThread())
-    client_->OnSwapBuffersComplete();
 }
 
 
@@ -189,8 +182,4 @@ void DelegatingRenderer::SetVisible(bool visible) {
   visible_ = visible;
 }
 
-void DelegatingRenderer::onContextLost() {
-  client_->DidLoseOutputSurface();
-}
-
 }  // namespace cc
diff --git a/cc/output/delegating_renderer.h b/cc/output/delegating_renderer.h
index a1bac0e..59d3c37 100644
--- a/cc/output/delegating_renderer.h
+++ b/cc/output/delegating_renderer.h
@@ -15,10 +15,8 @@ namespace cc {
 class OutputSurface;
 class ResourceProvider;
 
-class CC_EXPORT DelegatingRenderer
-    : public Renderer,
-      public NON_EXPORTED_BASE(
-          WebKit::WebGraphicsContext3D::WebGraphicsContextLostCallback) {
+class CC_EXPORT DelegatingRenderer :
+    public Renderer {
  public:
   static scoped_ptr<DelegatingRenderer> Create(
       RendererClient* client,
@@ -46,9 +44,6 @@ class CC_EXPORT DelegatingRenderer
                                       size_t bytes_visible_and_nearby,
                                       size_t bytes_allocated) OVERRIDE {}
 
-  // WebGraphicsContext3D::WebGraphicsContextLostCallback implementation.
-  virtual void onContextLost() OVERRIDE;
-
  private:
   DelegatingRenderer(RendererClient* client,
                      OutputSurface* output_surface,
diff --git a/cc/output/gl_renderer.cc b/cc/output/gl_renderer.cc
index 5a70f38..dcd952a 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -119,7 +119,6 @@ bool GLRenderer::Initialize() {
   if (!context_->makeContextCurrent())
     return false;
 
-  context_->setContextLostCallback(this);
   context_->pushGroupMarkerEXT("CompositorContext");
 
   std::string extensions_string =
@@ -144,8 +143,6 @@ bool GLRenderer::Initialize() {
   if (client_->HasImplThread())
     capabilities_.using_swap_complete_callback =
         extensions.count("GL_CHROMIUM_swapbuffers_complete_callback") > 0;
-  if (capabilities_.using_swap_complete_callback)
-    context_->setSwapBuffersCompleteCallbackCHROMIUM(this);
 
   capabilities_.using_set_visibility =
       extensions.count("GL_CHROMIUM_set_visibility") > 0;
@@ -191,9 +188,7 @@ bool GLRenderer::Initialize() {
 }
 
 GLRenderer::~GLRenderer() {
-  context_->setSwapBuffersCompleteCallbackCHROMIUM(NULL);
   context_->setMemoryAllocationChangedCallbackCHROMIUM(NULL);
-  context_->setContextLostCallback(NULL);
   CleanupSharedObjects();
 }
 
@@ -1889,12 +1884,6 @@ bool GLRenderer::SwapBuffers() {
   return true;
 }
 
-void GLRenderer::ReceiveCompositorFrameAck(const CompositorFrameAck& ack) {
-  onSwapBuffersComplete();
-}
-
-void GLRenderer::onSwapBuffersComplete() { client_->OnSwapBuffersComplete(); }
-
 void GLRenderer::onMemoryAllocationChanged(
     WebGraphicsMemoryAllocation allocation) {
   // Just ignore the memory manager when it says to set the limit to zero
@@ -1972,8 +1961,6 @@ void GLRenderer::EnsureBackbuffer() {
   is_backbuffer_discarded_ = false;
 }
 
-void GLRenderer::onContextLost() { client_->DidLoseOutputSurface(); }
-
 void GLRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
   DCHECK(rect.right() <= ViewportWidth());
   DCHECK(rect.bottom() <= ViewportHeight());
diff --git a/cc/output/gl_renderer.h b/cc/output/gl_renderer.h
index b5f8a46..8023486 100644
--- a/cc/output/gl_renderer.h
+++ b/cc/output/gl_renderer.h
@@ -32,16 +32,11 @@ class GeometryBinding;
 class ScopedEnsureFramebufferAllocation;
 
 // Class that handles drawing of composited render layers using GL.
-class CC_EXPORT GLRenderer
-    : public DirectRenderer,
-      public NON_EXPORTED_BASE(
-          WebKit::WebGraphicsContext3D::
-              WebGraphicsSwapBuffersCompleteCallbackCHROMIUM),
-      public NON_EXPORTED_BASE(
-          WebKit::WebGraphicsContext3D::
-              WebGraphicsMemoryAllocationChangedCallbackCHROMIUM),
-      public NON_EXPORTED_BASE(
-          WebKit::WebGraphicsContext3D::WebGraphicsContextLostCallback) {
+class CC_EXPORT GLRenderer :
+    public DirectRenderer,
+    public NON_EXPORTED_BASE(
+        WebKit::WebGraphicsContext3D::
+            WebGraphicsMemoryAllocationChangedCallbackCHROMIUM) {
  public:
   static scoped_ptr<GLRenderer> Create(RendererClient* client,
                                        OutputSurface* output_surface,
@@ -56,9 +51,6 @@ class CC_EXPORT GLRenderer
 
   virtual void ViewportChanged() OVERRIDE;
 
-  virtual void ReceiveCompositorFrameAck(const CompositorFrameAck& ack)
-      OVERRIDE;
-
   // Waits for rendering to finish.
   virtual void Finish() OVERRIDE;
 
@@ -186,11 +178,6 @@ class CC_EXPORT GLRenderer
   void CleanupSharedObjects();
 
   // WebKit::
-  // WebGraphicsContext3D::WebGraphicsSwapBuffersCompleteCallbackCHROMIUM
-  // implementation.
-  virtual void onSwapBuffersComplete() OVERRIDE;
-
-  // WebKit::
   // WebGraphicsContext3D::WebGraphicsMemoryAllocationChangedCallbackCHROMIUM
   // implementation.
   virtual void onMemoryAllocationChanged(
@@ -199,9 +186,6 @@ class CC_EXPORT GLRenderer
   void EnsureBackbuffer();
   void EnforceMemoryPolicy();
 
-  // WebGraphicsContext3D::WebGraphicsContextLostCallback implementation.
-  virtual void onContextLost() OVERRIDE;
-
   RendererCapabilities capabilities_;
 
   unsigned offscreen_framebuffer_id_;
diff --git a/cc/output/gl_renderer_unittest.cc b/cc/output/gl_renderer_unittest.cc
index 7196266..6400a8b 100644
--- a/cc/output/gl_renderer_unittest.cc
+++ b/cc/output/gl_renderer_unittest.cc
@@ -165,8 +165,6 @@ class FakeRendererClient : public RendererClient {
     static LayerTreeSettings fake_settings;
     return fake_settings;
   }
-  virtual void DidLoseOutputSurface() OVERRIDE {}
-  virtual void OnSwapBuffersComplete() OVERRIDE {}
   virtual void SetFullRootLayerDamage() OVERRIDE {
     set_full_root_layer_damage_count_++;
   }
diff --git a/cc/output/output_surface.cc b/cc/output/output_surface.cc
index 4164402..8312790 100644
--- a/cc/output/output_surface.cc
+++ b/cc/output/output_surface.cc
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/strings/string_split.h"
+#include "cc/output/output_surface_client.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebGraphicsContext3D.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
@@ -23,6 +24,27 @@ using std::vector;
 
 namespace cc {
 
+class OutputSurfaceCallbacks :
+  public WebKit::WebGraphicsContext3D::WebGraphicsSwapBuffersCompleteCallbackCHROMIUM,
+  public WebKit::WebGraphicsContext3D::WebGraphicsContextLostCallback {
+ public:
+   explicit OutputSurfaceCallbacks(OutputSurfaceClient* client)
+     : client_(client) {}
+
+   // WK:WGC3D::WGSwapBuffersCompleteCallbackCHROMIUM implementation.
+   virtual void onSwapBuffersComplete() {
+     client_->OnSwapBuffersComplete();
+   }
+
+   // WK:WGC3D::WGContextLostCallback implementation.
+   virtual void onContextLost() {
+     client_->DidLoseOutputSurface();
+   }
+
+ private:
+  OutputSurfaceClient* client_;
+};
+
 OutputSurface::OutputSurface(
     scoped_ptr<WebKit::WebGraphicsContext3D> context3d)
     : client_(NULL),
@@ -66,6 +88,10 @@ bool OutputSurface::BindToClient(
   has_gl_discard_backbuffer_ =
       extensions.count("GL_CHROMIUM_discard_backbuffer") > 0;
 
+  callbacks_.reset(new OutputSurfaceCallbacks(client_));
+  context3d_->setSwapBuffersCompleteCallbackCHROMIUM(callbacks_.get());
+  context3d_->setContextLostCallback(callbacks_.get());
+
   return true;
 }
 
diff --git a/cc/output/output_surface.h b/cc/output/output_surface.h
index 2e7a190..8ebf01c 100644
--- a/cc/output/output_surface.h
+++ b/cc/output/output_surface.h
@@ -20,6 +20,7 @@ namespace cc {
 
 class CompositorFrame;
 class OutputSurfaceClient;
+class OutputSurfaceCallbacks;
 struct LatencyInfo;
 
 // Represents the output surface for a compositor. The compositor owns
@@ -102,6 +103,8 @@ class CC_EXPORT OutputSurface {
   scoped_ptr<cc::SoftwareOutputDevice> software_device_;
   bool has_gl_discard_backbuffer_;
 
+  scoped_ptr<OutputSurfaceCallbacks> callbacks_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(OutputSurface);
 };
diff --git a/cc/output/output_surface_client.h b/cc/output/output_surface_client.h
index 1ba5a43..1e2b542 100644
--- a/cc/output/output_surface_client.h
+++ b/cc/output/output_surface_client.h
@@ -18,6 +18,8 @@ class CC_EXPORT OutputSurfaceClient {
                                         base::TimeDelta interval) = 0;
   virtual void DidVSync(base::TimeTicks frame_time) = 0;
   virtual void OnSendFrameToParentCompositorAck(const CompositorFrameAck&) = 0;
+  virtual void OnSwapBuffersComplete() = 0;
+  virtual void DidLoseOutputSurface() = 0;
 
  protected:
   virtual ~OutputSurfaceClient() {}
diff --git a/cc/output/renderer.h b/cc/output/renderer.h
index 3d26407..6a7d01e 100644
--- a/cc/output/renderer.h
+++ b/cc/output/renderer.h
@@ -21,8 +21,6 @@ class CC_EXPORT RendererClient {
  public:
   virtual gfx::Size DeviceViewportSize() const = 0;
   virtual const LayerTreeSettings& Settings() const = 0;
-  virtual void DidLoseOutputSurface() = 0;
-  virtual void OnSwapBuffersComplete() = 0;
   virtual void SetFullRootLayerDamage() = 0;
   virtual void SetManagedMemoryPolicy(const ManagedMemoryPolicy& policy) = 0;
   virtual void EnforceManagedMemoryPolicy(
diff --git a/cc/output/software_renderer.cc b/cc/output/software_renderer.cc
index 977e7f2..94f00ad 100644
--- a/cc/output/software_renderer.cc
+++ b/cc/output/software_renderer.cc
@@ -125,8 +125,6 @@ bool SoftwareRenderer::SwapBuffers() {
 
 void SoftwareRenderer::ReceiveCompositorFrameAck(
     const CompositorFrameAck& ack) {
-  if (capabilities_.using_swap_complete_callback)
-    client_->OnSwapBuffersComplete();
   output_device_->ReclaimDIB(ack.last_dib_id);
 }
 
diff --git a/cc/output/software_renderer_unittest.cc b/cc/output/software_renderer_unittest.cc
index 839cc05..4584757 100644
--- a/cc/output/software_renderer_unittest.cc
+++ b/cc/output/software_renderer_unittest.cc
@@ -55,8 +55,6 @@ class SoftwareRendererTest : public testing::Test, public RendererClient {
   virtual const LayerTreeSettings& Settings() const OVERRIDE {
     return settings_;
   }
-  virtual void DidLoseOutputSurface() OVERRIDE {}
-  virtual void OnSwapBuffersComplete() OVERRIDE {}
   virtual void SetFullRootLayerDamage() OVERRIDE {}
   virtual void SetManagedMemoryPolicy(const ManagedMemoryPolicy& policy)
       OVERRIDE {}
diff --git a/cc/test/fake_output_surface.cc b/cc/test/fake_output_surface.cc
index 6369f61..2fe4b5a 100644
--- a/cc/test/fake_output_surface.cc
+++ b/cc/test/fake_output_surface.cc
@@ -25,15 +25,6 @@ FakeOutputSurface::FakeOutputSurface(
 
 FakeOutputSurface::~FakeOutputSurface() {}
 
-bool FakeOutputSurface::BindToClient(
-    cc::OutputSurfaceClient* client) {
-  DCHECK(client);
-  client_ = client;
-  if (!context3d_)
-    return true;
-  return context3d_->makeContextCurrent();
-}
-
 void FakeOutputSurface::SendFrameToParentCompositor(
     CompositorFrame* frame) {
   frame->AssignTo(&last_sent_frame_);
diff --git a/cc/test/fake_output_surface.h b/cc/test/fake_output_surface.h
index e23f8cb..8af8709 100644
--- a/cc/test/fake_output_surface.h
+++ b/cc/test/fake_output_surface.h
@@ -56,7 +56,6 @@ class FakeOutputSurface : public OutputSurface {
         new FakeOutputSurface(software_device.Pass(), true));
   }
 
-  virtual bool BindToClient(OutputSurfaceClient* client) OVERRIDE;
   virtual void SendFrameToParentCompositor(CompositorFrame* frame) OVERRIDE;
 
   CompositorFrame& last_sent_frame() { return last_sent_frame_; }
diff --git a/cc/test/pixel_test.cc b/cc/test/pixel_test.cc
index 7d03606..8d0e64e 100644
--- a/cc/test/pixel_test.cc
+++ b/cc/test/pixel_test.cc
@@ -30,8 +30,6 @@ class PixelTest::PixelTestRendererClient : public RendererClient {
   virtual const LayerTreeSettings& Settings() const OVERRIDE {
     return settings_;
   }
-  virtual void DidLoseOutputSurface() OVERRIDE {}
-  virtual void OnSwapBuffersComplete() OVERRIDE {}
   virtual void SetFullRootLayerDamage() OVERRIDE {}
   virtual void SetManagedMemoryPolicy(
       const ManagedMemoryPolicy& policy) OVERRIDE {}
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index a500663..bfa05e3 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -990,6 +990,9 @@ void LayerTreeHostImpl::OnSendFrameToParentCompositorAck(
   // TODO(piman): We may need to do some validation on this ack before
   // processing it.
   renderer_->ReceiveCompositorFrameAck(ack);
+
+  // When using compositor frame data, the ack doubles as a swap complete ack.
+  OnSwapBuffersComplete();
 }
 
 void LayerTreeHostImpl::OnCanDrawStateChangedForTree() {
@@ -1149,7 +1152,11 @@ const LayerTreeSettings& LayerTreeHostImpl::Settings() const {
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
-  client_->DidLoseOutputSurfaceOnImplThread();
+  // TODO(jamesr): The renderer_ check is needed to make some of the
+  // LayerTreeHostContextTest tests pass, but shouldn't be necessary (or
+  // important) in production. We should adjust the test to not need this.
+  if (renderer_)
+    client_->DidLoseOutputSurfaceOnImplThread();
 }
 
 void LayerTreeHostImpl::OnSwapBuffersComplete() {
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index bc97f41..dae900b 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -23,6 +23,7 @@
 #include "cc/layers/texture_layer_impl.h"
 #include "cc/layers/tiled_layer_impl.h"
 #include "cc/layers/video_layer_impl.h"
+#include "cc/output/compositor_frame_ack.h"
 #include "cc/output/compositor_frame_metadata.h"
 #include "cc/output/gl_renderer.h"
 #include "cc/quads/render_pass_draw_quad.h"
@@ -74,9 +75,12 @@ class LayerTreeHostImplTest : public testing::Test,
     media::InitializeMediaLibraryForTesting();
   }
 
+  virtual void OverrideSettings(LayerTreeSettings* settings) {}
+
   virtual void SetUp() OVERRIDE {
     LayerTreeSettings settings;
     settings.minimum_occlusion_tracking_size = gfx::Size();
+    OverrideSettings(&settings);
 
     host_impl_ = LayerTreeHostImpl::Create(settings,
                                            this,
@@ -4289,8 +4293,6 @@ class TestRenderer : public GLRenderer, public RendererClient {
   virtual const LayerTreeSettings& Settings() const OVERRIDE {
     return settings_;
   }
-  virtual void DidLoseOutputSurface() OVERRIDE {}
-  virtual void OnSwapBuffersComplete() OVERRIDE {}
   virtual void SetFullRootLayerDamage() OVERRIDE {}
   virtual void SetManagedMemoryPolicy(const ManagedMemoryPolicy& policy)
       OVERRIDE {}
@@ -5407,5 +5409,33 @@ TEST_F(LayerTreeHostImplTest, MaskLayerForSurfaceWithClippedLayer) {
   }
 }
 
+class CompositorFrameMetadataTest : public LayerTreeHostImplTest {
+ public:
+  CompositorFrameMetadataTest()
+      : swap_buffers_complete_(0) {}
+
+  virtual void OverrideSettings(LayerTreeSettings* settings) OVERRIDE {
+    settings->compositor_frame_message = true;
+  }
+  virtual void OnSwapBuffersCompleteOnImplThread() OVERRIDE {
+    swap_buffers_complete_++;
+  }
+
+  int swap_buffers_complete_;
+};
+
+TEST_F(CompositorFrameMetadataTest, CompositorFrameAckCountsAsSwapComplete) {
+  SetupRootLayerImpl(FakeLayerWithQuads::Create(host_impl_->active_tree(), 1));
+  {
+    LayerTreeHostImpl::FrameData frame;
+    EXPECT_TRUE(host_impl_->PrepareToDraw(&frame, gfx::Rect()));
+    host_impl_->DrawLayers(&frame, base::TimeTicks());
+    host_impl_->DidDrawAllLayers(frame);
+  }
+  CompositorFrameAck ack;
+  host_impl_->OnSendFrameToParentCompositorAck(ack);
+  EXPECT_EQ(swap_buffers_complete_, 1);
+}
+
 }  // namespace
 }  // namespace cc
diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
index 26e47f7..24bfae8 100644
--- a/cc/trees/single_thread_proxy.cc
+++ b/cc/trees/single_thread_proxy.cc
@@ -369,8 +369,6 @@ void SingleThreadProxy::ForceSerializeOnSwapBuffers() {
   }
 }
 
-void SingleThreadProxy::OnSwapBuffersCompleteOnImplThread() { NOTREACHED(); }
-
 bool SingleThreadProxy::CommitAndComposite(
     base::TimeTicks frame_begin_time,
     gfx::Rect device_viewport_damage_rect,
diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h
index fa5d5fe..cf29574 100644
--- a/cc/trees/single_thread_proxy.h
+++ b/cc/trees/single_thread_proxy.h
@@ -49,7 +49,7 @@ class SingleThreadProxy : public Proxy, LayerTreeHostImplClient {
 
   // LayerTreeHostImplClient implementation
   virtual void DidLoseOutputSurfaceOnImplThread() OVERRIDE;
-  virtual void OnSwapBuffersCompleteOnImplThread() OVERRIDE;
+  virtual void OnSwapBuffersCompleteOnImplThread() OVERRIDE {}
   virtual void OnVSyncParametersChanged(base::TimeTicks timebase,
                                         base::TimeDelta interval) OVERRIDE {}
   virtual void DidVSync(base::TimeTicks frame_time) OVERRIDE {}