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

cc/trees/property_tree.h

 // 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.
 
 #ifndef CC_TREES_PROPERTY_TREE_H_
 #define CC_TREES_PROPERTY_TREE_H_
 
 #include <stddef.h>
 
 #include <memory>
@@ skipping 345 matching lines @@
 class CC_EXPORT ScrollTree final : public PropertyTree<ScrollNode> {
  public:
   ScrollTree();
   ~ScrollTree();
 
   ScrollTree& operator=(const ScrollTree& from);
   bool operator==(const ScrollTree& other) const;
 
   void clear();
 
-  typedef std::unordered_map<int, scoped_refptr<SyncedScrollOffset>>
-      ScrollOffsetMap;
   typedef std::unordered_map<int, bool> ScrollbarsEnabledMap;
 
   gfx::ScrollOffset MaxScrollOffset(int scroll_node_id) const;
   void OnScrollOffsetAnimated(int layer_id,
                               int transform_tree_index,
                               int scroll_tree_index,
                               const gfx::ScrollOffset& scroll_offset,
                               LayerTreeImpl* layer_tree_impl);
   gfx::Size scroll_clip_layer_bounds(int scroll_node_id) const;
   ScrollNode* CurrentlyScrollingNode();
   const ScrollNode* CurrentlyScrollingNode() const;
 #if DCHECK_IS_ON()
   int CurrentlyScrollingNodeId() const;
 #endif
   void set_currently_scrolling_node(int scroll_node_id);
   gfx::Transform ScreenSpaceTransform(int scroll_node_id) const;
 
+  // Returns the current scroll offset. On the main thread this would return the
+  // value from the LayerTree while on the impl thread this is the current value

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

nit: "for the LayerTree" might be clearer that we're not actually calling into
the LayerTree to get the value

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

Makes sense. Done.

+  // on the active tree.
   const gfx::ScrollOffset current_scroll_offset(int layer_id) const;
+
+  // Collects deltas for scroll changes on the impl thread that need to be
+  // reported to the main thread during the main frame. As such, should only be
+  // called on the impl thread side PropertyTrees.
   void CollectScrollDeltas(ScrollAndScaleSet* scroll_info,
                            int inner_viewport_layer_id);
-  void UpdateScrollOffsetMap(ScrollOffsetMap* new_scroll_offset_map,
-                             LayerTreeImpl* layer_tree_impl);
-  ScrollOffsetMap& scroll_offset_map();
-  const ScrollOffsetMap& scroll_offset_map() const;
+
+  // Applies deltas sent in the previous main frame onto the impl thread state.
+  // Should only be called on the impl thread side PropertyTrees.
   void ApplySentScrollDeltasFromAbortedCommit();
+
+  // Pushes scroll updates from the ScrollTree on the main thread onto the
+  // impl thread associated state.
+  void PushScrollUpdatesFromMainThread(PropertyTrees* main_property_trees,
+                                       LayerTreeImpl* sync_tree);
+
+  // Pushes scroll updates from the ScrollTree on the pending tree onto the
+  // active tree associated state.
+  void PushScrollUpdatesFromPendingTree(PropertyTrees* pending_property_trees,
+                                        LayerTreeImpl* active_tree);
+
   bool SetBaseScrollOffset(int layer_id,
                            const gfx::ScrollOffset& scroll_offset);
   bool SetScrollOffset(int layer_id, const gfx::ScrollOffset& scroll_offset);
   void SetScrollOffsetClobberActiveValue(int layer_id) {
-    synced_scroll_offset(layer_id)->set_clobber_active_value();
+    GetOrCreateSyncedScrollOffset(layer_id)->set_clobber_active_value();
   }
   bool UpdateScrollOffsetBaseForTesting(int layer_id,
                                         const gfx::ScrollOffset& offset);
   bool SetScrollOffsetDeltaForTesting(int layer_id,
                                       const gfx::Vector2dF& delta);
   const gfx::ScrollOffset GetScrollOffsetBaseForTesting(int layer_id) const;
   const gfx::ScrollOffset GetScrollOffsetDeltaForTesting(int layer_id) const;
   void CollectScrollDeltasForTesting();
 
   void DistributeScroll(ScrollNode* scroll_node, ScrollState* scroll_state);
   gfx::Vector2dF ScrollBy(ScrollNode* scroll_node,
                           const gfx::Vector2dF& scroll,
                           LayerTreeImpl* layer_tree_impl);
   gfx::ScrollOffset ClampScrollOffsetToLimits(gfx::ScrollOffset offset,
                                               ScrollNode* scroll_node) const;
 
+  const SyncedScrollOffset* synced_scroll_offset(int layer_id) const;

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

I know you're just moving this code in this CL, but it should really be
something like GetSyncedScrollOffset since it's not a trivial getter. But since
this isn't new, feel free to leave it for a follow-up CL.

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

Good point. Done.

+
+#if DCHECK_IS_ON()
+  void CopyCompleteTreeState(const ScrollTree& other);
+#endif
+
  private:
+  using ScrollOffsetMap = std::unordered_map<int, gfx::ScrollOffset>;
+  using SyncedScrollOffsetMap =
+      std::unordered_map<int, scoped_refptr<SyncedScrollOffset>>;
+
   int currently_scrolling_node_id_;
-  ScrollOffsetMap layer_id_to_scroll_offset_map_;
   ScrollbarsEnabledMap layer_id_to_scrollbars_enabled_map_;
 
-  SyncedScrollOffset* synced_scroll_offset(int layer_id);
-  const SyncedScrollOffset* synced_scroll_offset(int layer_id) const;
+  // On the main thread we store the scroll offsets directly since the main
+  // thread only needs to keep track of the current main thread state. The impl
+  // thread stores a map of SyncedProperty instances in order to track
+  // additional state necessary to synchronize scroll changes between the main
+  // and impl threads.
+  ScrollOffsetMap layer_id_to_scroll_offset_map_;
+  SyncedScrollOffsetMap layer_id_to_synced_scroll_offset_map_;
+
+  SyncedScrollOffset* GetOrCreateSyncedScrollOffset(int layer_id);
   gfx::ScrollOffset PullDeltaForMainThread(SyncedScrollOffset* scroll_offset);
-  void UpdateScrollOffsetMapEntry(int key,
-                                  ScrollOffsetMap* new_scroll_offset_map,
-                                  LayerTreeImpl* layer_tree_impl);
 };
 
 struct AnimationScaleData {
   // Variable used to invalidate cached animation scale data when transform tree
   // updates.
   int update_number;
 
   // Current animations, considering only scales at keyframes not including the
   // starting keyframe of each animation.
   float local_maximum_animation_target_scale;
@@ skipping 178 matching lines @@
   DrawTransforms& GetDrawTransforms(int transform_id, int effect_id) const;
   DrawTransformData& FetchDrawTransformsDataFromCache(int transform_id,
                                                       int effect_id) const;
 
   PropertyTreesCachedData cached_data_;
 };
 
 }  // namespace cc
 
 #endif  // CC_TREES_PROPERTY_TREE_H_