LayerTreeHostImpl knows if it has an output surface.

cc/trees/layer_tree_host_impl.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/layer_tree_host_impl.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/basictypes.h"
@@ skipping 239 matching lines @@
       zero_budget_(false),
       device_scale_factor_(1.f),
       overhang_ui_resource_id_(0),
       overdraw_bottom_height_(0.f),
       resourceless_software_draw_(false),
       begin_impl_frame_interval_(BeginFrameArgs::DefaultInterval()),
       animation_registrar_(AnimationRegistrar::Create()),
       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) {
   DCHECK(proxy_->IsImplThread());
   DidVisibilityChange(this, visible_);
   animation_registrar_->set_supports_scroll_animations(
       proxy_->SupportsImplScrolling());
 
   SetDebugState(settings.initial_debug_state);
 
@@ skipping 1335 matching lines @@
   resource_provider_->MarkPendingUploadsAsNonBlocking();
 }
 
 void LayerTreeHostImpl::FinishAllRendering() {
   if (renderer_)
     renderer_->Finish();
 }
 
 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) {
   if (use_gpu == use_gpu_rasterization_)
     return;
 
   use_gpu_rasterization_ = use_gpu;
   ReleaseTreeResources();
 
   // Replace existing tile manager with another one that uses appropriate
@@ skipping 84 matching lines @@
 }
 
 float LayerTreeHostImpl::VerticalAdjust() const {
   if (!active_tree_->InnerViewportContainerLayer())
     return 0;
 
   return active_tree_->InnerViewportContainerLayer()->BoundsDelta().y();
 }
 
 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
   // 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();

[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 {
   return !!InnerViewportScrollLayer();
 }
 
 LayerImpl* LayerTreeHostImpl::RootLayer() const {
   return active_tree_->root_layer();
 }
 
@@ skipping 327 matching lines @@
          resource_provider_->use_sync_query();
 }
 
 void LayerTreeHostImpl::EnforceZeroBudget(bool zero_budget) {
   SetManagedMemoryPolicy(cached_managed_memory_policy_, 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
   // before we destroy the old resource provider.
   ReleaseTreeResources();
 
   // Note: order is important here.
   renderer_.reset();
   DestroyTileManager();
   resource_provider_.reset();
   output_surface_.reset();
 
   if (!output_surface->BindToClient(this))
     return false;
 
   output_surface_ = output_surface.Pass();
+  have_output_surface_ = true;
   resource_provider_ =
       ResourceProvider::Create(output_surface_.get(),
                                shared_bitmap_manager_,
                                settings_.highp_threshold_min,
                                settings_.use_rgba_4444_textures,
                                settings_.texture_id_allocation_chunk_size,
                                settings_.use_distance_field_text);
 
   if (output_surface_->capabilities().deferred_gl_initialization)
     EnforceZeroBudget(true);
@@ skipping 1262 matching lines @@
 }
 
 void LayerTreeHostImpl::UnregisterPictureLayerImpl(PictureLayerImpl* layer) {
   std::vector<PictureLayerImpl*>::iterator it =
       std::find(picture_layers_.begin(), picture_layers_.end(), layer);
   DCHECK(it != picture_layers_.end());
   picture_layers_.erase(it);
 }
 
 }  // namespace cc