LayerTreeHostImpl knows if it has an output surface.

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index e155be69517bd86761125deddecbded319767b07..f5183eb1ce01ce80be8d0b09f92e212e86eb205c 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_output_surface_(false),

[danakj, 2014/08/06 18:56:43]

nit: have_valid_output_surface_ or something along those lines. we might have
one (not null) but it's lost, so let's try make that distinction clear in this
name

[dneto, 2014/08/06 20:20:35]

Done.  Yeah, I thought about using "valid" in the name too.  I should have.

       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_output_surface_;
 }
 
 void LayerTreeHostImpl::SetUseGpuRasterization(bool use_gpu) {
@@ -1720,6 +1720,8 @@ float LayerTreeHostImpl::VerticalAdjust() const {
 }
 
 void LayerTreeHostImpl::DidLoseOutputSurface() {
+  if (!have_output_surface_)
+    return;
   if (resource_provider_)
     resource_provider_->DidLoseOutputSurface();
   // TODO(jamesr): The renderer_ check is needed to make some of the
@@ -1727,9 +1729,7 @@ void LayerTreeHostImpl::DidLoseOutputSurface() {
   // important) in production. We should adjust the test to not need this.
   if (renderer_)
     client_->DidLoseOutputSurfaceOnImplThread();

[danakj, 2014/08/06 18:56:44]

This will call over to ThreadProxy::DidLoseOutputSurfaceOnImplThread which will
call CheckOutputSurfaceStatusOnImplThread which will early out because
have_output_surface_ is still true. I'm kinda surprised that isn't breaking a
test? Can you set have_output_surface_ false before calling anything else in
this method?

[danakj, 2014/08/06 18:57:10]

Sorry ignore this I saw you removed that call.

[dneto, 2014/08/06 20:20:35]

Acknowledged.

[dneto, 2014/08/06 20:20:35]

Acknowledged.
Also, for clarity I've moved the setting of the state variable to before we
notify the resource provider and renderer.

-#if DCHECK_IS_ON
-  did_lose_called_ = true;
-#endif
+  have_output_surface_ = false;
 }
 
 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_output_surface_ = true;
   resource_provider_ =
       ResourceProvider::Create(output_surface_.get(),
                                shared_bitmap_manager_,