Refactor client visibility handling

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 d7d0f4c22d156bb9797bef7a43a6dd004a967c6f..5effb0fff7c87e680a9714c13fd653dc15fff1d1 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -157,35 +157,6 @@ void RecordCompositorSlowScrollMetric(InputHandler::ScrollInputType type,
   }
 }
 
-// Calls SetClientVisible on the provided |context_provider| and handles
-// additional cache cleanup.
-void UpdateVisibilityForContextProvider(int client_id,
-                                        ContextProvider* context_provider,
-                                        bool is_visible) {
-  if (!context_provider)
-    return;
-  gpu::ContextSupport* context_support = context_provider->ContextSupport();
-
-  context_support->SetClientVisible(client_id, is_visible);
-  bool aggressively_free_resources = !context_support->AnyClientsVisible();
-  if (aggressively_free_resources) {
-    context_provider->DeleteCachedResources();
-  }
-  context_support->SetAggressivelyFreeResources(aggressively_free_resources);
-}
-
-// Same as UpdateVisibilityForContextProvider, except that the
-// |context_provider| is locked before being used.
-void LockAndUpdateVisibilityForContextProvider(
-    int client_id,
-    ContextProvider* context_provider,
-    bool is_visible) {
-  if (!context_provider)
-    return;
-  ContextProvider::ScopedContextLock hold(context_provider);
-  UpdateVisibilityForContextProvider(client_id, context_provider, is_visible);
-}
-
 }  // namespace
 
 DEFINE_SCOPED_UMA_HISTOGRAM_TIMER(PendingTreeDurationHistogramTimer,
@@ -1296,19 +1267,14 @@ void LayerTreeHostImpl::UpdateTileManagerMemoryPolicy(
     // are visible. Notify the worker context here. We handle becoming
     // invisible in NotifyAllTileTasksComplete to avoid interrupting running
     // work.
-    if (output_surface_) {
-      LockAndUpdateVisibilityForContextProvider(
-          id_, output_surface_->worker_context_provider(),
-          true /* is_visible */);
-    }
+    SetWorkerContextVisibility(true);
 
     // If |global_tile_state_.hard_memory_limit_in_bytes| is greater than 0, we
     // allow the image decode controller to retain resources. We handle the
     // equal to 0 case in NotifyAllTileTasksComplete to avoid interrupting
     // running work.
     if (image_decode_controller_) {

[danakj, 2016/08/29 23:56:20]

nit: no {} needed now

[ericrk, 2016/08/30 18:18:58]

Done.

-      image_decode_controller_->SetShouldAggressivelyFreeResources(
-          false /* aggressively_free_resources */);
+      image_decode_controller_->SetShouldAggressivelyFreeResources(false);
     }
   }
 
@@ -1387,14 +1353,9 @@ void LayerTreeHostImpl::NotifyAllTileTasksCompleted() {
     // controller has released all Skia refs at the time Skia's cleanup
     // executes (within worker context's cleanup).
     if (image_decode_controller_) {

[danakj, 2016/08/29 23:56:20]

nitto

[ericrk, 2016/08/30 18:18:58]

Done.

-      image_decode_controller_->SetShouldAggressivelyFreeResources(
-          true /* aggressively_free_resources */);
-    }
-    if (output_surface_) {
-      LockAndUpdateVisibilityForContextProvider(
-          id_, output_surface_->worker_context_provider(),
-          false /* is_visible */);
+      image_decode_controller_->SetShouldAggressivelyFreeResources(true);
     }
+    SetWorkerContextVisibility(false);
   }
 }
 
@@ -2132,10 +2093,7 @@ void LayerTreeHostImpl::SetVisible(bool visible) {
   }
 
   // Update visibility for the compositor context provider.
-  if (output_surface_) {
-    UpdateVisibilityForContextProvider(id_, output_surface_->context_provider(),
-                                       visible);
-  }
+  SetMainContextVisibility(visible);
 }
 
 void LayerTreeHostImpl::SetNeedsOneBeginImplFrame() {
@@ -2350,6 +2308,12 @@ void LayerTreeHostImpl::ReleaseOutputSurface() {
   CleanUpTileManagerAndUIResources();
   resource_provider_ = nullptr;
 
+  // Release any context visibility before we destroy the OutputSurface.
+  if (main_context_visibility_)

[danakj, 2016/08/29 23:56:20]

can you use if (visible_) for the main context, and mention why the worker needs
to check the unique_ptr? (fwiw other code calls it the "compositor context")

[ericrk, 2016/08/30 18:18:58]

Done.

+    SetMainContextVisibility(false);
+  if (worker_context_visibility_)
+    SetWorkerContextVisibility(false);
+
   // Detach from the old output surface and reset |output_surface_| pointer
   // as this surface is going to be destroyed independent of if binding the
   // new output surface succeeds or not.
@@ -2385,6 +2349,12 @@ bool LayerTreeHostImpl::InitializeRenderer(OutputSurface* output_surface) {
       settings_.renderer_settings.use_gpu_memory_buffer_resources,
       settings_.renderer_settings.buffer_to_texture_target_map);
 
+  // Make sure the main context visibility is restored. Worker context
+  // visibility will be set via the memory policy update in
+  // CreateTileManagerResources below.
+  if (visible_)
+    SetMainContextVisibility(true);
+
   // Since the new context may be capable of MSAA, update status here. We don't
   // need to check the return value since we are recreating all resources
   // already.
@@ -4129,4 +4099,47 @@ bool LayerTreeHostImpl::CommitToActiveTree() const {
   return !task_runner_provider_->HasImplThread();
 }
 
+void LayerTreeHostImpl::SetMainContextVisibility(bool is_visible) {
+  if (!output_surface_)
+    return;
+
+  DCHECK_NE(is_visible, !!main_context_visibility_);
+
+  auto* main_context = output_surface_->context_provider();
+  if (!main_context)
+    return;
+
+  if (is_visible) {
+    main_context_visibility_ =
+        main_context->CacheController()->ClientBecameVisible();
+  } else {
+    main_context->CacheController()->ClientBecameNotVisible(
+        std::move(main_context_visibility_));
+  }
+}
+
+void LayerTreeHostImpl::SetWorkerContextVisibility(bool is_visible) {
+  if (!output_surface_)
+    return;
+
+  // TODO(ericrk): This check is here because worker context visibility is a
+  // bit less controlled, being settable both by memory policy changes as well
+  // as direct visibility changes. We should simplify this. crbug.com/642154
+  if (is_visible == !!worker_context_visibility_)
+    return;
+
+  auto* worker_context = output_surface_->worker_context_provider();
+  if (!worker_context)
+    return;
+
+  ContextProvider::ScopedContextLock hold(worker_context);
+  if (is_visible) {
+    worker_context_visibility_ =
+        worker_context->CacheController()->ClientBecameVisible();
+  } else {
+    worker_context->CacheController()->ClientBecameNotVisible(
+        std::move(worker_context_visibility_));
+  }
+}
+
 }  // namespace cc