cc: split UpdateGpuRasterizationStatus() into two parts.

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 0bf40594b49d519a242d450b1859190577331892..a6f46de394a5cc220227916630481661f959ad1e 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -195,6 +195,7 @@ LayerTreeHostImpl::LayerTreeHostImpl(
       use_gpu_rasterization_(false),
       use_msaa_(false),
       gpu_rasterization_status_(GpuRasterizationStatus::OFF_DEVICE),
+      tree_resources_dirty_(false),
       input_handler_client_(NULL),
       did_lock_scrolling_layer_(false),
       should_bubble_scrolls_(false),
@@ -308,7 +309,7 @@ void LayerTreeHostImpl::BeginCommit() {
 void LayerTreeHostImpl::CommitComplete() {
   TRACE_EVENT0("cc", "LayerTreeHostImpl::CommitComplete");
 
-  UpdateGpuRasterizationStatus();
+  UpdateTreeResourcesIfNeeded();

[danakj, 2015/05/13 19:50:58]

I would UpdateGpuRasterizationStatus() here then UpdateTreeResourcesIfNeeded
right after, rather than doing it in the setters.

[Stephen White, 2015/05/13 20:15:20]

Having to remember to call UpdateGpuRasterizationStatus() seems 
error-prone. It seems safer to keep it automatically up-to-date, so 
that (e.g.) Victor can rely on gpu_rasterization_enabled_ being valid.

   sync_tree()->set_needs_update_draw_properties();
 
   if (settings_.impl_side_painting) {
@@ -1621,6 +1622,14 @@ void LayerTreeHostImpl::UpdateGpuRasterizationStatus() {
   use_gpu_rasterization_ = use_gpu;
   use_msaa_ = use_msaa;
 
+  tree_resources_dirty_ = true;
+}
+
+void LayerTreeHostImpl::UpdateTreeResourcesIfNeeded() {
+  if (!tree_resources_dirty_) {

[danakj, 2015/05/13 19:50:58]

We need to do the following stuff unconditionally when we change renderer. And
conditionally when we CommitComplete (if gpu raster changed).

[danakj, 2015/05/13 19:53:45]

Oh no I'm mistaken, the unconditional of the following happens in
InitializeRenderer, this just does it a second time (which we could actually
avoid some of this if we made the decision earlier?)

[Stephen White, 2015/05/13 20:15:20]

Yeah, it was conditional before in both cases. I've removed it from
CreateAndSetRenderer(), since it seems superfluous (see below).

+    return;
+  }
+
   // Clean up and replace existing tile manager with another one that uses
   // appropriate rasterizer.
   ReleaseTreeResources();
@@ -1634,6 +1643,8 @@ void LayerTreeHostImpl::UpdateGpuRasterizationStatus() {
   // We would not have any content to draw until the pending tree is activated.
   // Prevent the active tree from drawing until activation.
   SetRequiresHighResToDraw();
+
+  tree_resources_dirty_ = false;
 }
 
 const RendererCapabilitiesImpl&
@@ -1992,6 +2003,8 @@ void LayerTreeHostImpl::CreateAndSetRenderer() {
 
   // Since the new renderer may be capable of MSAA, update status here.
   UpdateGpuRasterizationStatus();

[danakj, 2015/05/13 19:50:58]

Move this out to InitializeRenderer?

[Stephen White, 2015/05/13 20:15:20]

Done.

+  UpdateTreeResourcesIfNeeded();

[Stephen White, 2015/05/13 20:15:20]

I see now that InitializeRenderer() is doing ReleaseTreeResoures() /
RecreateTreeResources() already, so this call seems superfluous. I've removed it
and all tests pass; let me know if I've missed something.

+
   renderer_->SetVisible(visible_);
   SetFullRootLayerDamage();