Harmonize LayerTreeHost/LayerTreeHostImpl synchronization steps

cc/trees/layer_tree_host.cc

Index: cc/trees/layer_tree_host.cc
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc
index 2b51bb3dacb6345f189c701540579dbad7652ff1..9b4c14bd6e930b7b38adbeaa6e51172c8038d1a2 100644
--- a/cc/trees/layer_tree_host.cc
+++ b/cc/trees/layer_tree_host.cc
@@ -289,6 +289,8 @@ void LayerTreeHost::FinishCommitOnImplThread(
         this, [](Layer* layer) { layer->SetNeedsPushProperties(); });
   }
 
+  DCHECK(host_impl->AdvanceSyncState(BEGINNING_SYNC));
+
   LayerTreeImpl* sync_tree = host_impl->sync_tree();
 
   if (next_commit_forces_redraw_) {
@@ -305,31 +307,49 @@ void LayerTreeHost::FinishCommitOnImplThread(
   if (needs_full_tree_sync_)
     TreeSynchronizer::SynchronizeTrees(root_layer(), sync_tree);
 
-  PushPropertiesTo(sync_tree);
+  {
+    TRACE_EVENT0("cc", "LayerTreeHostInProcess::PushProperties");
 
-  sync_tree->PassSwapPromises(swap_promise_manager_.TakeSwapPromises());
+    bool property_trees_changed_on_active_tree =
+        sync_tree->IsActiveTree() && sync_tree->property_trees()->changed;
+    // Property trees may store damage status. We preserve the sync tree damage
+    // status by pushing the damage status from sync tree property trees to main
+    // thread property trees or by moving it onto the layers.
+    if (root_layer_ && property_trees_changed_on_active_tree) {
+      if (property_trees_.sequence_number ==
+          sync_tree->property_trees()->sequence_number)
+        sync_tree->property_trees()->PushChangeTrackingTo(&property_trees_);
+      else
+        sync_tree->MoveChangeTrackingToLayers();
+    }
 
-  host_impl->SetHasGpuRasterizationTrigger(has_gpu_rasterization_trigger_);
-  host_impl->SetContentIsSuitableForGpuRasterization(
-      content_is_suitable_for_gpu_rasterization_);
-  RecordGpuRasterizationHistogram(host_impl);
+    PushPropertyTreesTo(sync_tree);
+    DCHECK(host_impl->AdvanceSyncState(SYNCED_PROPERTY_TREES));
 
-  host_impl->SetViewportSize(device_viewport_size_);
-  sync_tree->SetDeviceScaleFactor(device_scale_factor_);
-  host_impl->SetDebugState(debug_state_);
+    TreeSynchronizer::PushLayerProperties(this, sync_tree);
+    DCHECK(host_impl->AdvanceSyncState(SYNCED_LAYER_PROPERTIES));
 
-  if (did_navigate_) {
-    did_navigate_ = false;
-    host_impl->ClearImageCacheOnNavigation();
-  }
+    PushPropertiesTo(sync_tree);

[chrishtr, 2017/05/12 23:13:29]

How about renaming this to PushLayerTreeHostPropertiesTo, and also
making a PushLayerTreePropertiesTo for things on the tree_impl. (sync_tree).

[pdr., 2017/05/15 18:30:24]

Done, we now do:
PushLayerTreePropertiesTo(sync_tree);
PushLayerTreeHostPropertiesTo(host_impl);

+    DCHECK(host_impl->AdvanceSyncState(SYNCED_LAYER_TREE_HOST_PROPERTIES));
 
-  sync_tree->set_ui_resource_request_queue(
-      ui_resource_manager_->TakeUIResourcesRequests());
+    sync_tree->PassSwapPromises(swap_promise_manager_.TakeSwapPromises());
 
-  {
-    TRACE_EVENT0("cc", "LayerTreeHostInProcess::PushProperties");
+    host_impl->SetHasGpuRasterizationTrigger(has_gpu_rasterization_trigger_);

[chrishtr, 2017/05/12 23:13:29]

Put this inside of PushPropertiesTo?

[pdr., 2017/05/15 18:30:24]

I put this in PushLayerTreeHostPropertiesTo. I think that's what you meant?

+    host_impl->SetContentIsSuitableForGpuRasterization(
+        content_is_suitable_for_gpu_rasterization_);
+    RecordGpuRasterizationHistogram(host_impl);
 
-    TreeSynchronizer::PushLayerProperties(this, sync_tree);
+    host_impl->SetViewportSize(device_viewport_size_);
+    sync_tree->SetDeviceScaleFactor(device_scale_factor_);
+    host_impl->SetDebugState(debug_state_);
+
+    if (did_navigate_) {
+      did_navigate_ = false;
+      host_impl->ClearImageCacheOnNavigation();
+    }
+
+    sync_tree->set_ui_resource_request_queue(
+        ui_resource_manager_->TakeUIResourcesRequests());
 
     // This must happen after synchronizing property trees and after pushing
     // properties, which updates the clobber_active_value flag.
@@ -348,6 +368,8 @@ void LayerTreeHost::FinishCommitOnImplThread(
     TRACE_EVENT0("cc", "LayerTreeHostInProcess::AnimationHost::PushProperties");
     DCHECK(host_impl->mutator_host());
     mutator_host_->PushPropertiesTo(host_impl->mutator_host());
+
+    DCHECK(host_impl->AdvanceSyncState(NOT_SYNCING));
   }
 
   // Transfer image decode requests to the impl thread.
@@ -1122,6 +1144,10 @@ void LayerTreeHost::SetPropertyTreesNeedRebuild() {
   SetNeedsUpdateLayers();
 }
 
+void LayerTreeHost::PushPropertyTreesTo(LayerTreeImpl* tree_impl) {

[chrishtr, 2017/05/12 23:13:29]

Inline this, or push change tracking down into this method?

[pdr., 2017/05/15 18:30:24]

Moved change tracking down here.

For consistency, I did the same in for the LayerTreeImpl and put the damage
tracking in LayerTreeImpl::PushPropertyTreesTo.

+  tree_impl->SetPropertyTrees(&property_trees_);
+}
+
 void LayerTreeHost::PushPropertiesTo(LayerTreeImpl* tree_impl) {
   tree_impl->set_needs_full_tree_sync(needs_full_tree_sync_);
   needs_full_tree_sync_ = false;
@@ -1160,21 +1186,6 @@ void LayerTreeHost::PushPropertiesTo(LayerTreeImpl* tree_impl) {
 
   tree_impl->RegisterSelection(selection_);
 
-  bool property_trees_changed_on_active_tree =
-      tree_impl->IsActiveTree() && tree_impl->property_trees()->changed;
-  // Property trees may store damage status. We preserve the sync tree damage
-  // status by pushing the damage status from sync tree property trees to main
-  // thread property trees or by moving it onto the layers.
-  if (root_layer_ && property_trees_changed_on_active_tree) {
-    if (property_trees_.sequence_number ==
-        tree_impl->property_trees()->sequence_number)
-      tree_impl->property_trees()->PushChangeTrackingTo(&property_trees_);
-    else
-      tree_impl->MoveChangeTrackingToLayers();
-  }
-  // Setting property trees must happen before pushing the page scale.
-  tree_impl->SetPropertyTrees(&property_trees_);
-
   tree_impl->PushPageScaleFromMainThread(
       page_scale_factor_, min_page_scale_factor_, max_page_scale_factor_);