Refactor client visibility handling

cc/output/gl_renderer.cc

 // Copyright 2010 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/output/gl_renderer.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 400 matching lines @@
 
 GLRenderer::~GLRenderer() {
   while (!pending_async_read_pixels_.empty()) {
     PendingAsyncReadPixels* pending_read =
         pending_async_read_pixels_.back().get();
     pending_read->finished_read_pixels_callback.Cancel();
     pending_async_read_pixels_.pop_back();
   }
 
   CleanupSharedObjects();
+
+  if (context_visibility_) {
+    output_surface_->context_provider()

[danakj, 2016/08/26 23:49:08]

nit: maybe

auto* cache_controller = outsur->conpro->CacheController();
cache_controller->ClientBecameNotVisible(..)

would be less lines and easier to read formattingwise?

[ericrk, 2016/08/29 22:43:22]

Sadly, that first line is more than 80 chars... but splitting it into 3 lines is
maybe clearer?

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

Eh, 3 lines for 3 lines with more text I guess.

+        ->CacheController()
+        ->ClientBecameNotVisible(std::move(context_visibility_));
+  }
 }
 
 const RendererCapabilitiesImpl& GLRenderer::Capabilities() const {
   return capabilities_;
 }
 
 bool GLRenderer::CanPartialSwap() {
   auto* context_provider = output_surface_->context_provider();
   return context_provider->ContextCapabilities().post_sub_buffer;
 }
 
 void GLRenderer::DidChangeVisibility() {
   if (visible_) {
     output_surface_->EnsureBackbuffer();
   } else {
     TRACE_EVENT0("cc", "GLRenderer::DidChangeVisibility dropping resources");
     ReleaseRenderPassTextures();
     output_surface_->DiscardBackbuffer();
   }
 
   PrepareGeometry(NO_BINDING);
 
-  // Handle cleanup of resources on the ContextProvider. The compositor
-  // ContextProvider is not shared, so always pass 0 for the client_id.
-  // TODO(crbug.com/487471): Update this when we share compositor
-  // ContextProviders.
-  context_support_->SetClientVisible(0 /* client_id */, visible_);
-  bool aggressively_free_resources = !context_support_->AnyClientsVisible();
-  if (aggressively_free_resources)
-    output_surface_->context_provider()->DeleteCachedResources();
-  context_support_->SetAggressivelyFreeResources(aggressively_free_resources);
+  if (visible_ && !context_visibility_) {

[danakj, 2016/08/26 23:49:08]

Can you actually become visible and have a context_visibility? Sounds like a bug
if so. How about

if (visible_) {
  DCHECK(!c_v);
  ..
} else {
  DCHECK(c_v);
  ..
}
?

[ericrk, 2016/08/29 22:43:22]

Yeah, this seems reasonable.

+    context_visibility_ = output_surface_->context_provider()

[danakj, 2016/08/26 23:49:08]

similars for formatting?

[ericrk, 2016/08/29 22:43:22]

Refactored.

+                              ->CacheController()
+                              ->ClientBecameVisible();
+  } else if (!visible_ && context_visibility_) {
+    output_surface_->context_provider()
+        ->CacheController()
+        ->ClientBecameNotVisible(std::move(context_visibility_));
+  }
 }
 
 void GLRenderer::ReleaseRenderPassTextures() { render_pass_textures_.clear(); }
 
 void GLRenderer::DiscardPixels() {
   if (!use_discard_framebuffer_)
     return;
   bool using_default_framebuffer =
       !current_framebuffer_lock_ &&
       output_surface_->capabilities().uses_default_gl_framebuffer;
@@ skipping 3571 matching lines @@
 
   gl_->ScheduleCALayerSharedStateCHROMIUM(
       ca_layer_overlay->shared_state->opacity, is_clipped, clip_rect,
       sorting_context_id, gl_transform);
   gl_->ScheduleCALayerCHROMIUM(
       texture_id, contents_rect, ca_layer_overlay->background_color,
       ca_layer_overlay->edge_aa_mask, bounds_rect, filter);
 }
 
 }  // namespace cc