Revert 194394 "Move swapcomplete callback into OutputSurface"

It broke android in a weird manner, like it shows in your trybots.

> 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.
> 
> Review URL: https://codereview.chromium.org/12545018

TBR=jamesr@chromium.org

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

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

18 files changed, 79 insertions, 45 deletions

diff --git a/cc/output/delegating_renderer.cc b/cc/output/delegating_renderer.cc
index 850ad0f..a1763e3 100644
--- a/cc/output/delegating_renderer.cc
+++ b/cc/output/delegating_renderer.cc
@@ -70,6 +70,7 @@ bool DelegatingRenderer::Initialize() {
   if (!context3d->makeContextCurrent())
     return false;
 
+  context3d->setContextLostCallback(this);
   context3d->pushGroupMarkerEXT("CompositorContext");
 
   std::string extensions_string =
@@ -117,7 +118,11 @@ bool DelegatingRenderer::Initialize() {
   return true;
 }
 
-DelegatingRenderer::~DelegatingRenderer() {}
+DelegatingRenderer::~DelegatingRenderer() {
+  WebGraphicsContext3D* context3d = resource_provider_->GraphicsContext3D();
+  if (context3d)
+    context3d->setContextLostCallback(NULL);
+}
 
 const RendererCapabilities& DelegatingRenderer::Capabilities() const {
   return capabilities_;
@@ -168,6 +173,8 @@ void DelegatingRenderer::GetFramebufferPixels(void* pixels, gfx::Rect rect) {
 void DelegatingRenderer::ReceiveCompositorFrameAck(
     const CompositorFrameAck& ack) {
   resource_provider_->ReceiveFromParent(ack.resources);
+  if (client_->HasImplThread())
+    client_->OnSwapBuffersComplete();
 }
 
 
@@ -182,4 +189,8 @@ 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 59d3c37..a1bac0e 100644
--- a/cc/output/delegating_renderer.h
+++ b/cc/output/delegating_renderer.h
@@ -15,8 +15,10 @@ namespace cc {
 class OutputSurface;
 class ResourceProvider;
 
-class CC_EXPORT DelegatingRenderer :
-    public Renderer {
+class CC_EXPORT DelegatingRenderer
+    : public Renderer,
+      public NON_EXPORTED_BASE(
+          WebKit::WebGraphicsContext3D::WebGraphicsContextLostCallback) {
  public:
   static scoped_ptr<DelegatingRenderer> Create(
       RendererClient* client,
@@ -44,6 +46,9 @@ 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 dcd952a..5a70f38 100644
--- a/cc/output/gl_renderer.cc
+++ b/cc/output/gl_renderer.cc
@@ -119,6 +119,7 @@ bool GLRenderer::Initialize() {
   if (!context_->makeContextCurrent())
     return false;
 
+  context_->setContextLostCallback(this);
   context_->pushGroupMarkerEXT("CompositorContext");
 
   std::string extensions_string =
@@ -143,6 +144,8 @@ 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;
@@ -188,7 +191,9 @@ bool GLRenderer::Initialize() {
 }
 
 GLRenderer::~GLRenderer() {
+  context_->setSwapBuffersCompleteCallbackCHROMIUM(NULL);
   context_->setMemoryAllocationChangedCallbackCHROMIUM(NULL);
+  context_->setContextLostCallback(NULL);
   CleanupSharedObjects();
 }
 
@@ -1884,6 +1889,12 @@ 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
@@ -1961,6 +1972,8 @@ 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 8023486..b5f8a46 100644
--- a/cc/output/gl_renderer.h
+++ b/cc/output/gl_renderer.h
@@ -32,11 +32,16 @@ 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::
-            WebGraphicsMemoryAllocationChangedCallbackCHROMIUM) {
+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) {
  public:
   static scoped_ptr<GLRenderer> Create(RendererClient* client,
                                        OutputSurface* output_surface,
@@ -51,6 +56,9 @@ class CC_EXPORT GLRenderer :
 
   virtual void ViewportChanged() OVERRIDE;
 
+  virtual void ReceiveCompositorFrameAck(const CompositorFrameAck& ack)
+      OVERRIDE;
+
   // Waits for rendering to finish.
   virtual void Finish() OVERRIDE;
 
@@ -178,6 +186,11 @@ class CC_EXPORT GLRenderer :
   void CleanupSharedObjects();
 
   // WebKit::
+  // WebGraphicsContext3D::WebGraphicsSwapBuffersCompleteCallbackCHROMIUM
+  // implementation.
+  virtual void onSwapBuffersComplete() OVERRIDE;
+
+  // WebKit::
   // WebGraphicsContext3D::WebGraphicsMemoryAllocationChangedCallbackCHROMIUM
   // implementation.
   virtual void onMemoryAllocationChanged(
@@ -186,6 +199,9 @@ 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 5dd8d0b..2dfdb39 100644
--- a/cc/output/gl_renderer_unittest.cc
+++ b/cc/output/gl_renderer_unittest.cc
@@ -165,6 +165,8 @@ 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 8312790..4164402 100644
--- a/cc/output/output_surface.cc
+++ b/cc/output/output_surface.cc
@@ -11,7 +11,6 @@
 #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"
@@ -24,27 +23,6 @@ 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),
@@ -88,10 +66,6 @@ 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 8ebf01c..2e7a190 100644
--- a/cc/output/output_surface.h
+++ b/cc/output/output_surface.h
@@ -20,7 +20,6 @@ namespace cc {
 
 class CompositorFrame;
 class OutputSurfaceClient;
-class OutputSurfaceCallbacks;
 struct LatencyInfo;
 
 // Represents the output surface for a compositor. The compositor owns
@@ -103,8 +102,6 @@ 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 1e2b542..1ba5a43 100644
--- a/cc/output/output_surface_client.h
+++ b/cc/output/output_surface_client.h
@@ -18,8 +18,6 @@ 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 6a7d01e..3d26407 100644
--- a/cc/output/renderer.h
+++ b/cc/output/renderer.h
@@ -21,6 +21,8 @@ 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 94f00ad..977e7f2 100644
--- a/cc/output/software_renderer.cc
+++ b/cc/output/software_renderer.cc
@@ -125,6 +125,8 @@ 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 4584757..839cc05 100644
--- a/cc/output/software_renderer_unittest.cc
+++ b/cc/output/software_renderer_unittest.cc
@@ -55,6 +55,8 @@ 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 2fe4b5a..6369f61 100644
--- a/cc/test/fake_output_surface.cc
+++ b/cc/test/fake_output_surface.cc
@@ -25,6 +25,15 @@ 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 8af8709..e23f8cb 100644
--- a/cc/test/fake_output_surface.h
+++ b/cc/test/fake_output_surface.h
@@ -56,6 +56,7 @@ 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 8d0e64e..7d03606 100644
--- a/cc/test/pixel_test.cc
+++ b/cc/test/pixel_test.cc
@@ -30,6 +30,8 @@ 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 a276363..a500663 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -1149,11 +1149,7 @@ const LayerTreeSettings& LayerTreeHostImpl::Settings() const {
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
-  // 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();
+  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 6d335f0..301bf27 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -4289,6 +4289,8 @@ 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 {}
diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
index 98d0a8e..14bdddd 100644
--- a/cc/trees/single_thread_proxy.cc
+++ b/cc/trees/single_thread_proxy.cc
@@ -368,6 +368,8 @@ 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 cf29574..fa5d5fe 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 {}