cc: Don't create SyncedPropety instances on the main thread.

cc/trees/property_tree.cc

 // Copyright 2014 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 <stddef.h>
 
 #include <set>
 #include <vector>
 
 #include "base/logging.h"
@@ skipping 1108 matching lines @@
   // because we do not want to simply copy the map when property tree is
   // propagating from pending to active.
   // In the main to pending case, we do want to copy it, but this can be done by
   // calling UpdateScrollOffsetMap after the assignment;
   // In the other case, we want pending and active property trees to share the
   // same map.
   return *this;
 }
 
 bool ScrollTree::operator==(const ScrollTree& other) const {
-  const ScrollTree::ScrollOffsetMap& other_scroll_offset_map =
-      other.scroll_offset_map();
-  if (layer_id_to_scroll_offset_map_.size() != other_scroll_offset_map.size())
+  if (layer_id_to_scroll_offset_map_ != other.layer_id_to_scroll_offset_map_)
     return false;
-
-  for (auto map_entry : layer_id_to_scroll_offset_map_) {
-    int key = map_entry.first;
-    if (other_scroll_offset_map.find(key) == other_scroll_offset_map.end() ||
-        map_entry.second != layer_id_to_scroll_offset_map_.at(key))
-      return false;
-  }
+  if (layer_id_to_synced_scroll_offset_map_ !=
+      other.layer_id_to_synced_scroll_offset_map_)
+    return false;
 
   bool is_currently_scrolling_node_equal =
       currently_scrolling_node_id_ == other.currently_scrolling_node_id_;
 
   return PropertyTree::operator==(other) && is_currently_scrolling_node_equal;
 }
 
+#if DCHECK_IS_ON()
+void ScrollTree::CopyCompleteTreeState(const ScrollTree& other) {
+  currently_scrolling_node_id_ = other.currently_scrolling_node_id_;
+  layer_id_to_scrollbars_enabled_map_ =
+      other.layer_id_to_scrollbars_enabled_map_;
+  layer_id_to_scroll_offset_map_ = other.layer_id_to_scroll_offset_map_;
+  layer_id_to_synced_scroll_offset_map_ =
+      other.layer_id_to_synced_scroll_offset_map_;
+}
+#endif
+
 void ScrollTree::clear() {
   PropertyTree<ScrollNode>::clear();
 
   if (property_trees()->is_main_thread) {
     currently_scrolling_node_id_ = kInvalidNodeId;
     layer_id_to_scroll_offset_map_.clear();
   }
 
 #if DCHECK_IS_ON()
   ScrollTree tree;
   if (!property_trees()->is_main_thread) {
+    DCHECK(layer_id_to_scroll_offset_map_.empty());
     tree.currently_scrolling_node_id_ = currently_scrolling_node_id_;
-    tree.layer_id_to_scroll_offset_map_ = layer_id_to_scroll_offset_map_;
+    tree.layer_id_to_synced_scroll_offset_map_ =
+        layer_id_to_synced_scroll_offset_map_;
   }
   DCHECK(tree == *this);
 #endif
 }
 
 gfx::ScrollOffset ScrollTree::MaxScrollOffset(int scroll_node_id) const {
   const ScrollNode* scroll_node = Node(scroll_node_id);
   gfx::SizeF scroll_bounds =
       gfx::SizeF(scroll_node->bounds.width(), scroll_node->bounds.height());
 
@@ skipping 92 matching lines @@
   gfx::Transform screen_space_transform(
       1, 0, 0, 1, scroll_node->offset_to_transform_parent.x(),
       scroll_node->offset_to_transform_parent.y());
   screen_space_transform.ConcatTransform(
       transform_tree.ToScreen(transform_node->id));
   if (scroll_node->should_flatten)
     screen_space_transform.FlattenTo2d();
   return screen_space_transform;
 }
 
-SyncedScrollOffset* ScrollTree::synced_scroll_offset(int layer_id) {
-  if (layer_id_to_scroll_offset_map_.find(layer_id) ==
-      layer_id_to_scroll_offset_map_.end()) {
-    layer_id_to_scroll_offset_map_[layer_id] = new SyncedScrollOffset;
+SyncedScrollOffset* ScrollTree::GetOrCreateSyncedScrollOffset(int layer_id) {
+  DCHECK(!property_trees()->is_main_thread);
+  if (layer_id_to_synced_scroll_offset_map_.find(layer_id) ==
+      layer_id_to_synced_scroll_offset_map_.end()) {
+    layer_id_to_synced_scroll_offset_map_[layer_id] = new SyncedScrollOffset;
   }
-  return layer_id_to_scroll_offset_map_[layer_id].get();
+  return layer_id_to_synced_scroll_offset_map_[layer_id].get();
 }
 
 const SyncedScrollOffset* ScrollTree::synced_scroll_offset(int layer_id) const {
-  if (layer_id_to_scroll_offset_map_.find(layer_id) ==
-      layer_id_to_scroll_offset_map_.end()) {
+  DCHECK(!property_trees()->is_main_thread);
+  if (layer_id_to_synced_scroll_offset_map_.find(layer_id) ==
+      layer_id_to_synced_scroll_offset_map_.end()) {
     return nullptr;
   }
-  return layer_id_to_scroll_offset_map_.at(layer_id).get();
+  return layer_id_to_synced_scroll_offset_map_.at(layer_id).get();
 }
 
 const gfx::ScrollOffset ScrollTree::current_scroll_offset(int layer_id) const {
+  if (property_trees()->is_main_thread) {
+    ScrollOffsetMap::const_iterator it =
+        layer_id_to_scroll_offset_map_.find(layer_id);
+    return it != layer_id_to_scroll_offset_map_.end() ? it->second
+                                                      : gfx::ScrollOffset();
+  }
   return synced_scroll_offset(layer_id)
              ? synced_scroll_offset(layer_id)->Current(
                    property_trees()->is_active)
              : gfx::ScrollOffset();
 }
 
 gfx::ScrollOffset ScrollTree::PullDeltaForMainThread(
     SyncedScrollOffset* scroll_offset) {
   // TODO(flackr): We should pass the fractional scroll deltas when Blink fully
   // supports fractional scrolls.
   // TODO(flackr): We should ideally round the fractional scrolls in the same
   // direction as the scroll will be snapped but for common cases this is
   // equivalent to rounding to the nearest integer offset.
   gfx::ScrollOffset current_offset =
       scroll_offset->Current(property_trees()->is_active);
   gfx::ScrollOffset rounded_offset =
       gfx::ScrollOffset(roundf(current_offset.x()), roundf(current_offset.y()));
   scroll_offset->SetCurrent(rounded_offset);
   gfx::ScrollOffset delta = scroll_offset->PullDeltaForMainThread();
   scroll_offset->SetCurrent(current_offset);
   return delta;
 }
 
 void ScrollTree::CollectScrollDeltas(ScrollAndScaleSet* scroll_info,
                                      int inner_viewport_layer_id) {
-  for (auto map_entry : layer_id_to_scroll_offset_map_) {
+  DCHECK(!property_trees()->is_main_thread);
+  for (auto map_entry : layer_id_to_synced_scroll_offset_map_) {
     gfx::ScrollOffset scroll_delta =
         PullDeltaForMainThread(map_entry.second.get());
 
     gfx::Vector2d scroll_delta_vector(scroll_delta.x(), scroll_delta.y());
     int layer_id = map_entry.first;
 
     if (!scroll_delta.IsZero()) {
       if (layer_id == inner_viewport_layer_id) {
         // Inner (visual) viewport is stored separately.
         scroll_info->inner_viewport_scroll.layer_id = layer_id;
         scroll_info->inner_viewport_scroll.scroll_delta = scroll_delta_vector;
       } else {
         LayerTreeHostCommon::ScrollUpdateInfo scroll;
         scroll.layer_id = layer_id;
         scroll.scroll_delta = scroll_delta_vector;
         scroll_info->scrolls.push_back(scroll);
       }
     }
   }
 }
 
 void ScrollTree::CollectScrollDeltasForTesting() {
-  for (auto map_entry : layer_id_to_scroll_offset_map_) {
+  for (auto map_entry : layer_id_to_synced_scroll_offset_map_) {
     PullDeltaForMainThread(map_entry.second.get());
   }
 }
 
-void ScrollTree::UpdateScrollOffsetMapEntry(
-    int key,
-    ScrollTree::ScrollOffsetMap* new_scroll_offset_map,
-    LayerTreeImpl* layer_tree_impl) {
-  bool changed = false;
-  // If we are pushing scroll offset from main to pending tree, we create a new
-  // instance of synced scroll offset; if we are pushing from pending to active,
-  // we reuse the pending tree's value in the map.
-  if (!property_trees()->is_active) {
-    changed = synced_scroll_offset(key)->PushFromMainThread(
-        new_scroll_offset_map->at(key)->PendingBase());
+void ScrollTree::PushScrollUpdatesFromMainThread(
+    PropertyTrees* main_property_trees,
+    LayerTreeImpl* sync_tree) {
+  DCHECK(!property_trees()->is_main_thread);
+  const ScrollOffsetMap& main_scroll_offset_map =
+      main_property_trees->scroll_tree.layer_id_to_scroll_offset_map_;
 
-    if (new_scroll_offset_map->at(key)->clobber_active_value()) {
-      synced_scroll_offset(key)->set_clobber_active_value();
-    }
-    if (changed) {
-      layer_tree_impl->DidUpdateScrollOffset(key);
-    }
-  } else {
-    layer_id_to_scroll_offset_map_[key] = new_scroll_offset_map->at(key);
-    changed |= synced_scroll_offset(key)->PushPendingToActive();
-    if (changed) {
-      layer_tree_impl->DidUpdateScrollOffset(key);
-    }
+  // We first want to clear SyncedProperty instances for layers which were
+  // destroyed on the main thread.

[ajuma, 2017/01/11 22:35:51]

More precisely, destroyed or became non-scrollable

[Khushal, 2017/01/11 23:04:53]

Done.

+  for (auto map_entry = layer_id_to_synced_scroll_offset_map_.begin();
+       map_entry != layer_id_to_synced_scroll_offset_map_.end();) {
+    int layer_id = map_entry->first;
+    if (main_scroll_offset_map.find(layer_id) == main_scroll_offset_map.end())
+      map_entry = layer_id_to_synced_scroll_offset_map_.erase(map_entry);
+    else
+      map_entry++;
+  }
+
+  for (auto map_entry : main_scroll_offset_map) {
+    int layer_id = map_entry.first;
+    SyncedScrollOffset* synced_scroll_offset =
+        GetOrCreateSyncedScrollOffset(layer_id);
+
+    bool changed = synced_scroll_offset->PushFromMainThread(map_entry.second);
+    // If we are committing directly to the active tree, push pending to active
+    // here.
+    if (property_trees()->is_active)
+      changed |= synced_scroll_offset->PushPendingToActive();
+
+    if (changed)
+      sync_tree->DidUpdateScrollOffset(layer_id);
   }
 }
 
-void ScrollTree::UpdateScrollOffsetMap(
-    ScrollTree::ScrollOffsetMap* new_scroll_offset_map,
-    LayerTreeImpl* layer_tree_impl) {
-  if (layer_tree_impl && !layer_tree_impl->LayerListIsEmpty()) {
-    DCHECK(!property_trees()->is_main_thread);
-    for (auto map_entry = layer_id_to_scroll_offset_map_.begin();
-         map_entry != layer_id_to_scroll_offset_map_.end();) {
-      int key = map_entry->first;
-      if (new_scroll_offset_map->find(key) != new_scroll_offset_map->end()) {
-        UpdateScrollOffsetMapEntry(key, new_scroll_offset_map, layer_tree_impl);
-        ++map_entry;
-      } else {
-        map_entry = layer_id_to_scroll_offset_map_.erase(map_entry);
-      }
-    }
+void ScrollTree::PushScrollUpdatesFromPendingTree(
+    PropertyTrees* pending_property_trees,
+    LayerTreeImpl* active_tree) {
+  DCHECK(property_trees()->is_active);
+  DCHECK(!pending_property_trees->is_main_thread);
+  DCHECK(!pending_property_trees->is_active);
 
-    for (auto& map_entry : *new_scroll_offset_map) {
-      int key = map_entry.first;
-      if (layer_id_to_scroll_offset_map_.find(key) ==
-          layer_id_to_scroll_offset_map_.end())
-        UpdateScrollOffsetMapEntry(key, new_scroll_offset_map, layer_tree_impl);
-    }
+  // When pushing to the active tree, we can simply copy over the map from the
+  // pending tree. The pending and active tree hold a reference to the same
+  // SyncedProperty instances.
+  layer_id_to_synced_scroll_offset_map_.clear();
+  for (auto map_entry : pending_property_trees->scroll_tree
+                            .layer_id_to_synced_scroll_offset_map_) {
+    layer_id_to_synced_scroll_offset_map_[map_entry.first] = map_entry.second;
+    if (map_entry.second->PushPendingToActive())
+      active_tree->DidUpdateScrollOffset(map_entry.first);
   }
 }
 
-ScrollTree::ScrollOffsetMap& ScrollTree::scroll_offset_map() {
-  return layer_id_to_scroll_offset_map_;
-}
-
-const ScrollTree::ScrollOffsetMap& ScrollTree::scroll_offset_map() const {
-  return layer_id_to_scroll_offset_map_;
-}
-
 void ScrollTree::ApplySentScrollDeltasFromAbortedCommit() {
   DCHECK(property_trees()->is_active);
-  for (auto& map_entry : layer_id_to_scroll_offset_map_)
+  for (auto& map_entry : layer_id_to_synced_scroll_offset_map_)
     map_entry.second->AbortCommit();
 }
 
 bool ScrollTree::SetBaseScrollOffset(int layer_id,
                                      const gfx::ScrollOffset& scroll_offset) {
-  return synced_scroll_offset(layer_id)->PushFromMainThread(scroll_offset);
+  if (property_trees()->is_main_thread) {
+    layer_id_to_scroll_offset_map_[layer_id] = scroll_offset;
+    return true;
+  }
+
+  // Scroll offset updates on the impl thread should only be for layers which
+  // were created on the main thread. But this method is called when we build
+  // PropertyTrees on the impl thread from LayerTreeImpl.
+  return GetOrCreateSyncedScrollOffset(layer_id)->PushFromMainThread(
+      scroll_offset);
 }
 
 bool ScrollTree::SetScrollOffset(int layer_id,
                                  const gfx::ScrollOffset& scroll_offset) {
-  if (property_trees()->is_main_thread)
-    return synced_scroll_offset(layer_id)->PushFromMainThread(scroll_offset);
-  else if (property_trees()->is_active)
-    return synced_scroll_offset(layer_id)->SetCurrent(scroll_offset);
+  if (property_trees()->is_main_thread) {
+    layer_id_to_scroll_offset_map_[layer_id] = scroll_offset;
+    return true;

[ajuma, 2017/01/11 22:35:51]

Should this first check if the value changed?

[Khushal, 2017/01/11 23:04:53]

We don't use the result on the main thread side, on impl its needed for an
update to draw properties. But it would be better to return based on the initial
value. Done.

+  }
+
+  if (property_trees()->is_active) {
+    return GetOrCreateSyncedScrollOffset(layer_id)->SetCurrent(scroll_offset);
+  }
+
   return false;
 }
 
 bool ScrollTree::UpdateScrollOffsetBaseForTesting(
     int layer_id,
     const gfx::ScrollOffset& offset) {
   DCHECK(!property_trees()->is_main_thread);
-  bool changed = synced_scroll_offset(layer_id)->PushFromMainThread(offset);
+  SyncedScrollOffset* synced_scroll_offset =
+      GetOrCreateSyncedScrollOffset(layer_id);
+  bool changed = synced_scroll_offset->PushFromMainThread(offset);
   if (property_trees()->is_active)
-    changed |= synced_scroll_offset(layer_id)->PushPendingToActive();
+    changed |= synced_scroll_offset->PushPendingToActive();
   return changed;
 }
 
 bool ScrollTree::SetScrollOffsetDeltaForTesting(int layer_id,
                                                 const gfx::Vector2dF& delta) {
-  return synced_scroll_offset(layer_id)->SetCurrent(
+  return GetOrCreateSyncedScrollOffset(layer_id)->SetCurrent(
       synced_scroll_offset(layer_id)->ActiveBase() + gfx::ScrollOffset(delta));
 }
 
 const gfx::ScrollOffset ScrollTree::GetScrollOffsetBaseForTesting(
     int layer_id) const {
   DCHECK(!property_trees()->is_main_thread);
   if (synced_scroll_offset(layer_id))
     return property_trees()->is_active
                ? synced_scroll_offset(layer_id)->ActiveBase()
                : synced_scroll_offset(layer_id)->PendingBase();
@@ skipping 149 matching lines @@
   changed = false;
   non_root_surfaces_enabled = true;
   sequence_number++;
 
 #if DCHECK_IS_ON()
   PropertyTrees tree;
   tree.transform_tree = transform_tree;
   tree.effect_tree = effect_tree;
   tree.clip_tree = clip_tree;
   tree.scroll_tree = scroll_tree;
-  // Scroll offset map and currently scrolling node id may not be copied
-  // during operator=.
-  ScrollTree::ScrollOffsetMap& map = tree.scroll_tree.scroll_offset_map();
-  map = scroll_tree.scroll_offset_map();
-  tree.scroll_tree.set_currently_scrolling_node(
-      scroll_tree.CurrentlyScrollingNodeId());
+  tree.scroll_tree.CopyCompleteTreeState(scroll_tree);
 
   tree.sequence_number = sequence_number;
   tree.is_main_thread = is_main_thread;
   tree.is_active = is_active;
   DCHECK(tree == *this);
 #endif
 }
 
 void PropertyTrees::SetInnerViewportContainerBoundsDelta(
     gfx::Vector2dF bounds_delta) {
@@ skipping 432 matching lines @@
 
   const EffectNode* effect_node = effect_tree.Node(effect_id);
 
   bool success = GetFromTarget(transform_id, effect_id, transform);
   transform->Scale(effect_node->surface_contents_scale.x(),
                    effect_node->surface_contents_scale.y());
   return success;
 }
 
 }  // namespace cc