LayerTreeHostImpl knows if it has an output surface.

Surface loss is only detected via the callback.

We can tolerate multiple calls to InitializeRenderer without
needing the lost-surface callback in between.

Remove Renderer::IsContextLost, but keep GLRenderer::IsContextLost.

Remove ThreadProxy::CheckOutputSurfaceStatusOnImplThread

BUG=392891

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

Cr-Commit-Position: refs/heads/master@{#288457}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288457 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 41 insertions, 41 deletions

diff --git a/cc/output/delegating_renderer.cc b/cc/output/delegating_renderer.cc
index 206c05c..143596b 100644
--- a/cc/output/delegating_renderer.cc
+++ b/cc/output/delegating_renderer.cc
@@ -112,13 +112,6 @@ void DelegatingRenderer::ReceiveSwapBuffersAck(
   resource_provider_->ReceiveReturnsFromParent(ack.resources);
 }
 
-bool DelegatingRenderer::IsContextLost() {
-  ContextProvider* context_provider = output_surface_->context_provider();
-  if (!context_provider)
-    return false;
-  return context_provider->IsContextLost();
-}
-
 void DelegatingRenderer::DidChangeVisibility() {
   ContextProvider* context_provider = output_surface_->context_provider();
   if (!visible()) {
diff --git a/cc/output/delegating_renderer.h b/cc/output/delegating_renderer.h
index 10d95ce..52dbe01 100644
--- a/cc/output/delegating_renderer.h
+++ b/cc/output/delegating_renderer.h
@@ -37,8 +37,6 @@ class CC_EXPORT DelegatingRenderer : public Renderer {
   virtual void SwapBuffers(const CompositorFrameMetadata& metadata) OVERRIDE;
   virtual void ReceiveSwapBuffersAck(const CompositorFrameAck&) OVERRIDE;
 
-  virtual bool IsContextLost() OVERRIDE;
-
  private:
   DelegatingRenderer(RendererClient* client,
                      const LayerTreeSettings* settings,
diff --git a/cc/output/gl_renderer.h b/cc/output/gl_renderer.h
index d5c4f0a..75a5d6e 100644
--- a/cc/output/gl_renderer.h
+++ b/cc/output/gl_renderer.h
@@ -65,7 +65,7 @@ class CC_EXPORT GLRenderer : public DirectRenderer {
   virtual void DoNoOp() OVERRIDE;
   virtual void SwapBuffers(const CompositorFrameMetadata& metadata) OVERRIDE;
 
-  virtual bool IsContextLost() OVERRIDE;
+  virtual bool IsContextLost();
 
   static void DebugGLCall(gpu::gles2::GLES2Interface* gl,
                           const char* command,
diff --git a/cc/output/renderer.cc b/cc/output/renderer.cc
index dedc204..2de4630 100644
--- a/cc/output/renderer.cc
+++ b/cc/output/renderer.cc
@@ -10,10 +10,6 @@ bool Renderer::HasAllocatedResourcesForTesting(RenderPass::Id id) const {
   return false;
 }
 
-bool Renderer::IsContextLost() {
-  return false;
-}
-
 void Renderer::SetVisible(bool visible) {
   if (visible_ == visible)
     return;
diff --git a/cc/output/renderer.h b/cc/output/renderer.h
index b9376c9..b6d0cd4 100644
--- a/cc/output/renderer.h
+++ b/cc/output/renderer.h
@@ -73,8 +73,6 @@ class CC_EXPORT Renderer {
   virtual void SwapBuffers(const CompositorFrameMetadata& metadata) = 0;
   virtual void ReceiveSwapBuffersAck(const CompositorFrameAck& ack) {}
 
-  virtual bool IsContextLost();
-
   bool visible() const { return visible_; }
   void SetVisible(bool visible);
 
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index e155be6..2d7a932 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -257,9 +257,7 @@ LayerTreeHostImpl::LayerTreeHostImpl(
       rendering_stats_instrumentation_(rendering_stats_instrumentation),
       micro_benchmark_controller_(this),
       need_to_update_visible_tiles_before_draw_(false),
-#if DCHECK_IS_ON
-      did_lose_called_(false),
-#endif
+      have_valid_output_surface_(false),
       shared_bitmap_manager_(manager),
       id_(id),
       transfer_buffer_memory_limit_(0u) {
@@ -1615,7 +1613,9 @@ void LayerTreeHostImpl::FinishAllRendering() {
 
 bool LayerTreeHostImpl::IsContextLost() {
   DCHECK(proxy_->IsImplThread());
-  return renderer_ && renderer_->IsContextLost();
+  // To avoid races, rely only on the lost-surface callback.
+  // See crbug.com/392891.
+  return !have_valid_output_surface_;
 }
 
 void LayerTreeHostImpl::SetUseGpuRasterization(bool use_gpu) {
@@ -1720,6 +1720,9 @@ float LayerTreeHostImpl::VerticalAdjust() const {
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
+  if (!have_valid_output_surface_)
+    return;
+  have_valid_output_surface_ = false;
   if (resource_provider_)
     resource_provider_->DidLoseOutputSurface();
   // TODO(jamesr): The renderer_ check is needed to make some of the
@@ -1727,9 +1730,6 @@ void LayerTreeHostImpl::DidLoseOutputSurface() {
   // important) in production. We should adjust the test to not need this.
   if (renderer_)
     client_->DidLoseOutputSurfaceOnImplThread();
-#if DCHECK_IS_ON
-  did_lose_called_ = true;
-#endif
 }
 
 bool LayerTreeHostImpl::HaveRootScrollLayer() const {
@@ -2077,9 +2077,6 @@ void LayerTreeHostImpl::EnforceZeroBudget(bool zero_budget) {
 bool LayerTreeHostImpl::InitializeRenderer(
     scoped_ptr<OutputSurface> output_surface) {
   TRACE_EVENT0("cc", "LayerTreeHostImpl::InitializeRenderer");
-#if DCHECK_IS_ON
-  DCHECK(!renderer_ || did_lose_called_);
-#endif
 
   // Since we will create a new resource provider, we cannot continue to use
   // the old resources (i.e. render_surfaces and texture IDs). Clear them
@@ -2096,6 +2093,7 @@ bool LayerTreeHostImpl::InitializeRenderer(
     return false;
 
   output_surface_ = output_surface.Pass();
+  have_valid_output_surface_ = true;
   resource_provider_ =
       ResourceProvider::Create(output_surface_.get(),
                                shared_bitmap_manager_,
diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h
index 821028f..7392d4f 100644
--- a/cc/trees/layer_tree_host_impl.h
+++ b/cc/trees/layer_tree_host_impl.h
@@ -703,9 +703,7 @@ class CC_EXPORT LayerTreeHostImpl
   MicroBenchmarkControllerImpl micro_benchmark_controller_;
 
   bool need_to_update_visible_tiles_before_draw_;
-#if DCHECK_IS_ON
-  bool did_lose_called_;
-#endif
+  bool have_valid_output_surface_;
 
   // Optional callback to notify of new tree activations.
   base::Closure tree_activation_callback_;
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 5d45d53..fe2a234 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -6815,5 +6815,36 @@ TEST_F(LayerTreeHostImplTest, DidBecomeActive) {
   EXPECT_EQ(1u, raw_replica_mask_layer->did_become_active_call_count());
 }
 
+class LayerTreeHostImplCountingLostSurfaces : public LayerTreeHostImplTest {
+ public:
+  LayerTreeHostImplCountingLostSurfaces() : num_lost_surfaces_(0) {}
+  virtual void DidLoseOutputSurfaceOnImplThread() OVERRIDE {
+    num_lost_surfaces_++;
+  }
+
+ protected:
+  int num_lost_surfaces_;
+};
+
+TEST_F(LayerTreeHostImplCountingLostSurfaces, TwiceLostSurface) {
+  // The medium term, we plan to remove LayerTreeHostImpl::IsContextLost().
+  // Until then, we need the state variable
+  // LayerTreeHostImpl::have_valid_output_surface_ and we can
+  // enforce the following behaviour, where calling DidLoseOutputSurface
+  // twice in a row only causes one subsequent
+  // call to LayerTreeHostImplClient::DidLoseOutputSurfaceOnImplThread().
+  // Really we just need at least one client notification each time
+  // we go from having a valid output surface to not having a valid output
+  // surface.
+  EXPECT_EQ(0, num_lost_surfaces_);
+  EXPECT_FALSE(host_impl_->IsContextLost());
+  host_impl_->DidLoseOutputSurface();
+  EXPECT_TRUE(host_impl_->IsContextLost());
+  EXPECT_EQ(1, num_lost_surfaces_);
+  host_impl_->DidLoseOutputSurface();
+  EXPECT_TRUE(host_impl_->IsContextLost());
+  EXPECT_EQ(1, num_lost_surfaces_);
+}
+
 }  // namespace
 }  // namespace cc
diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
index b18b93a..a379d0e 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -335,14 +335,6 @@ void ThreadProxy::UpdateRendererCapabilitiesOnImplThread() {
 void ThreadProxy::DidLoseOutputSurfaceOnImplThread() {
   TRACE_EVENT0("cc", "ThreadProxy::DidLoseOutputSurfaceOnImplThread");
   DCHECK(IsImplThread());
-  CheckOutputSurfaceStatusOnImplThread();
-}
-
-void ThreadProxy::CheckOutputSurfaceStatusOnImplThread() {
-  TRACE_EVENT0("cc", "ThreadProxy::CheckOutputSurfaceStatusOnImplThread");
-  DCHECK(IsImplThread());
-  if (!impl().layer_tree_host_impl->IsContextLost())
-    return;
   Proxy::MainThreadTaskRunner()->PostTask(
       FROM_HERE,
       base::Bind(&ThreadProxy::DidLoseOutputSurface, main_thread_weak_ptr_));
@@ -1132,9 +1124,6 @@ DrawResult ThreadProxy::DrawSwapInternal(bool forced_draw) {
         base::Bind(&ThreadProxy::DidCommitAndDrawFrame, main_thread_weak_ptr_));
   }
 
-  if (draw_frame)
-    CheckOutputSurfaceStatusOnImplThread();
-
   if (result == DRAW_SUCCESS)
     impl().timing_history.DidFinishDrawing();
 
diff --git a/cc/trees/thread_proxy.h b/cc/trees/thread_proxy.h
index ace0198..fe709112 100644
--- a/cc/trees/thread_proxy.h
+++ b/cc/trees/thread_proxy.h
@@ -272,7 +272,6 @@ class CC_EXPORT ThreadProxy : public Proxy,
   void LayerTreeHostClosedOnImplThread(CompletionEvent* completion);
   DrawResult DrawSwapInternal(bool forced_draw);
   void ForceSerializeOnSwapBuffersOnImplThread(CompletionEvent* completion);
-  void CheckOutputSurfaceStatusOnImplThread();
   void CommitPendingOnImplThreadForTesting(CommitPendingRequest* request);
   void AsValueOnImplThread(CompletionEvent* completion,
                            base::debug::TracedValue* state) const;