Update Context Client Visibility to use Scoped Pattern

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 dc170e3bf02559317e314c27d39c3ec70b2a5830..3ed96cb9bdfcd379892224103493714a4f48a400 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -84,7 +84,6 @@
 #include "cc/trees/single_thread_proxy.h"
 #include "cc/trees/tree_synchronizer.h"
 #include "gpu/GLES2/gl2extchromium.h"
-#include "gpu/command_buffer/client/context_support.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "ui/gfx/geometry/point_conversions.h"
 #include "ui/gfx/geometry/rect_conversions.h"
@@ -159,14 +158,24 @@ void RecordCompositorSlowScrollMetric(InputHandler::ScrollInputType type,
 
 // Calls SetClientVisible on the provided |context_provider| and handles

[danakj, 2016/08/18 21:39:44]

SetClientVisible is a lie

[ericrk, 2016/08/19 17:23:19]

Comment wasn't really saying much... removed.

 // additional cache cleanup.
-void UpdateVisibilityForContextProvider(int client_id,
-                                        ContextProvider* context_provider,
-                                        bool is_visible) {
+void UpdateVisibilityForContextProvider(
+    ContextProvider* context_provider,
+    std::unique_ptr<gpu::ContextSupport::ScopedVisibility>* scoped_visibility,
+    bool is_visible) {
   if (!context_provider)
     return;
+  if (*scoped_visibility && is_visible)

[danakj, 2016/08/18 21:39:44]

Combine?

[ericrk, 2016/08/19 17:23:19]

Done.

+    return;
+  if (!*scoped_visibility && !is_visible)
+    return;
+
   gpu::ContextSupport* context_support = context_provider->ContextSupport();
 
-  context_support->SetClientVisible(client_id, is_visible);
+  if (is_visible)
+    *scoped_visibility = context_support->ClientBecameVisible();
+  else
+    context_support->ClientBecameNotVisible(std::move(*scoped_visibility));
+
   bool aggressively_free_resources = !context_support->AnyClientsVisible();
   if (aggressively_free_resources) {
     context_provider->DeleteCachedResources();
@@ -177,13 +186,19 @@ void UpdateVisibilityForContextProvider(int client_id,
 // Same as UpdateVisibilityForContextProvider, except that the
 // |context_provider| is locked before being used.
 void LockAndUpdateVisibilityForContextProvider(
-    int client_id,
     ContextProvider* context_provider,
+    std::unique_ptr<gpu::ContextSupport::ScopedVisibility>* scoped_visibility,
     bool is_visible) {
   if (!context_provider)
     return;
+  if (*scoped_visibility && is_visible)

[danakj, 2016/08/18 21:39:44]

Combine?

[ericrk, 2016/08/19 17:23:19]

Done.

+    return;
+  if (!*scoped_visibility && !is_visible)
+    return;
+
   ContextProvider::ScopedContextLock hold(context_provider);
-  UpdateVisibilityForContextProvider(client_id, context_provider, is_visible);
+  UpdateVisibilityForContextProvider(context_provider, scoped_visibility,
+                                     is_visible);
 }
 
 }  // namespace
@@ -1298,8 +1313,8 @@ void LayerTreeHostImpl::UpdateTileManagerMemoryPolicy(
     // work.
     if (output_surface_) {
       LockAndUpdateVisibilityForContextProvider(
-          id_, output_surface_->worker_context_provider(),
-          true /* is_visible */);
+          output_surface_->worker_context_provider(),
+          &worker_context_client_visibility_, true /* is_visible */);
     }
 
     // If |global_tile_state_.hard_memory_limit_in_bytes| is greater than 0, we
@@ -1392,8 +1407,8 @@ void LayerTreeHostImpl::NotifyAllTileTasksCompleted() {
     }
     if (output_surface_) {
       LockAndUpdateVisibilityForContextProvider(
-          id_, output_surface_->worker_context_provider(),
-          false /* is_visible */);
+          output_surface_->worker_context_provider(),
+          &worker_context_client_visibility_, false /* is_visible */);
     }
   }
 }
@@ -2124,7 +2139,8 @@ void LayerTreeHostImpl::SetVisible(bool visible) {
 
   // Update visibility for the compositor context provider.
   if (output_surface_) {
-    UpdateVisibilityForContextProvider(id_, output_surface_->context_provider(),
+    UpdateVisibilityForContextProvider(output_surface_->context_provider(),
+                                       &main_context_client_visibility_,
                                        visible);
   }
 }
@@ -2338,10 +2354,22 @@ void LayerTreeHostImpl::ReleaseOutputSurface() {
   CleanUpTileManagerAndUIResources();
   resource_provider_ = nullptr;
 
-  // 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.
   if (output_surface_) {
+    // Ensure that any context client visibility is left in a good state.
+    if (main_context_client_visibility_) {

[danakj, 2016/08/18 21:39:44]

They already early out appropriately, can you mention why you're checking them
here too? (Cuz of Bind failing?)

[ericrk, 2016/08/19 17:23:19]

Oh yeah... no reason I guess.

+      UpdateVisibilityForContextProvider(output_surface_->context_provider(),
+                                         &main_context_client_visibility_,
+                                         false);
+    }
+    if (worker_context_client_visibility_) {
+      LockAndUpdateVisibilityForContextProvider(
+          output_surface_->worker_context_provider(),
+          &worker_context_client_visibility_, 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.
     output_surface_->DetachFromClient();
     output_surface_ = nullptr;
   }