cc: Move SyncedScrollOffset to scroll tree

cc/layers/layer_impl.h

 // Copyright 2011 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_LAYERS_LAYER_IMPL_H_
 #define CC_LAYERS_LAYER_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 240 matching lines @@
   const LayerImpl* replica_layer() const { return replica_layer_.get(); }
   scoped_ptr<LayerImpl> TakeReplicaLayer();
 
   bool has_mask() const { return !!mask_layer_; }
   bool has_replica() const { return !!replica_layer_; }
   bool replica_has_mask() const {
     return replica_layer_ && (mask_layer_ || replica_layer_->mask_layer_);
   }
 
   LayerTreeImpl* layer_tree_impl() const { return layer_tree_impl_; }
+  LayerListImpl* layer_list_impl() const { return layer_list_impl_; }
 
   void PopulateSharedQuadState(SharedQuadState* state) const;
   void PopulateScaledSharedQuadState(SharedQuadState* state, float scale) const;
   // WillDraw must be called before AppendQuads. If WillDraw returns false,
   // AppendQuads and DidDraw will not be called. If WillDraw returns true,
   // DidDraw is guaranteed to be called before another WillDraw or before
   // the layer is destroyed. To enforce this, any class that overrides
   // WillDraw/DidDraw must call the base class version only if WillDraw
   // returns true.
   virtual bool WillDraw(DrawMode draw_mode,
@@ skipping 185 matching lines @@
 
   void SetBounds(const gfx::Size& bounds);
   gfx::Size bounds() const;
   // Like bounds() but doesn't snap to int. Lossy on giant pages (e.g. millions
   // of pixels) due to use of single precision float.
   gfx::SizeF BoundsForScrolling() const;
   void SetBoundsDelta(const gfx::Vector2dF& bounds_delta);
   gfx::Vector2dF bounds_delta() const { return bounds_delta_; }
 
   void SetCurrentScrollOffset(const gfx::ScrollOffset& scroll_offset);
+  // must only be called by tests when updating scroll offset of a single layer,
+  // the standard process to update scroll offsets is
+  // ScrollTree::UpdateScrollOffsetMap()

[ajuma, 2016/03/01 22:54:20]

Please make this a complete sentence.

[sunxd, 2016/03/02 17:45:34]

Done.

   void PushScrollOffsetFromMainThread(const gfx::ScrollOffset& scroll_offset);

[ajuma, 2016/03/01 22:54:20]

If this is only for tests, it should be called something like
PushScrollOffsetFromMainThreadForTesting.

But are tests that call this still meaningful now? Or should they really be
calling something else that more closely matches reality (or deleted if they're
no longer relevant)?

[sunxd, 2016/03/02 17:45:34]

I think some tests call this function to set the initial value of scroll offset,
but there are still some that want to utilize the synced property feature. Can I
leave it for the next CL? Since this might also be related to the refactoring of
moving scroll offset logic to property trees and the number of callers may make
this CL inconvenient to review.

[ajuma, 2016/03/02 18:43:09]

Refactoring the tests in another CL seems fine. What about renaming this to
PushScrollOffsetFromMainThreadForTesting though? Does that seem reasonable to do
here?

[sunxd, 2016/03/02 20:52:55]

I think the function will probably go away in next CL and it is called by about
70 tests, so it makes sense to me to deal with it then to avoid duplicated
refactoring work.

-  // This method is similar to PushScrollOffsetFromMainThread but will cause the
-  // scroll offset given to clobber any scroll changes on the active tree in the
-  // time until this value is pushed to the active tree.
-  void PushScrollOffsetFromMainThreadAndClobberActiveValue(
-      const gfx::ScrollOffset& scroll_offset);
-  gfx::ScrollOffset PullDeltaForMainThread();
+
   gfx::ScrollOffset CurrentScrollOffset() const;
   gfx::ScrollOffset BaseScrollOffset() const;
   gfx::Vector2dF ScrollDelta() const;
   void SetScrollDelta(const gfx::Vector2dF& delta);
+  // This is a mirror function of Layer::scroll_offset, which is supposed to be
+  // called only by PropertyTreeBuilder
+  gfx::ScrollOffset scroll_offset() const {
+    return synced_scroll_offset()->PendingBase();

[ajuma, 2016/03/01 22:54:20]

Could PropertyTreeBuilder call CurrentScrollOffset instead (for both Layer and
LayerImpl)?

[sunxd, 2016/03/02 17:45:34]

Currently CurrentScrollOffset returns the combined offset, but here we only want
the pending base. I think we can consider this after moving the logic to
property tree in the next CL.

[ajuma, 2016/03/02 18:43:09]

These would only have different values if there are tests that are rebuilding
property trees in a state where the active/pending values are different. Could
we fix the tests instead? (It just seems weird to have LayerImpl::scroll_offset
return the pending base value even if the LayerImpl itself is in the active
tree.)

[sunxd, 2016/03/02 20:52:55]

The unit test that breaks is
https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/layer_im...:
it tries to push scroll offset to a layer_impl, then push the same offset and
verify if the layer properties changed. But the macro used here forces a rebuild
of property tree, the rebuild can take the base+delta as base if I call
CurrentScrollOffset. I'm not sure if fixing the test breaks the purpose of it.

[ajuma, 2016/03/02 22:31:08]

That test isn't really testing anything meaningful now, since it's checking the
behavior of a function we don't call outside of tests. How about changing that
so it only tests that LayerImpl::DidUpdateScrollOffset does trigger
needs_push_properties?

+  }
 
   gfx::ScrollOffset MaxScrollOffset() const;
   gfx::ScrollOffset ClampScrollOffsetToLimits(gfx::ScrollOffset offset) const;
   gfx::Vector2dF ClampScrollToMaxScrollOffset();
   void SetScrollCompensationAdjustment(const gfx::Vector2dF& scroll_offset) {
     scroll_compensation_adjustment_ = scroll_offset;
   }
   gfx::Vector2dF ScrollCompensationAdjustment() const {
     return scroll_compensation_adjustment_;
   }
@@ skipping 13 matching lines @@
   bool user_scrollable_horizontal() const {
     return user_scrollable_horizontal_;
   }
   void set_user_scrollable_vertical(bool scrollable) {
     user_scrollable_vertical_ = scrollable;
   }
   bool user_scrollable_vertical() const { return user_scrollable_vertical_; }
 
   bool user_scrollable(ScrollbarOrientation orientation) const;
 
-  void ApplySentScrollDeltasFromAbortedCommit();
-
   void set_main_thread_scrolling_reasons(
       uint32_t main_thread_scrolling_reasons);
   uint32_t main_thread_scrolling_reasons() const {
     return main_thread_scrolling_reasons_;
   }
   bool should_scroll_on_main_thread() const {
     return !!main_thread_scrolling_reasons_;
   }
 
   void SetNonFastScrollableRegion(const Region& region) {
@@ skipping 108 matching lines @@
   void Set3dSortingContextId(int id);
   int sorting_context_id() { return sorting_context_id_; }
 
   void SetFrameTimingRequests(
       const std::vector<FrameTimingRequest>& frame_timing_requests);
   const std::vector<FrameTimingRequest>& frame_timing_requests() const {
     return frame_timing_requests_;
   }
   void GatherFrameTimingRequestIds(std::vector<int64_t>* request_ids);
 
-  SyncedScrollOffset* synced_scroll_offset() { return scroll_offset_.get(); }
+  const SyncedScrollOffset* synced_scroll_offset() const;
+  SyncedScrollOffset* synced_scroll_offset();
 
   // Get the correct invalidation region instead of conservative Rect
   // for layers that provide it.
   virtual Region GetInvalidationRegion();
 
   virtual gfx::Rect GetEnclosingRectInTargetSpace() const;
 
   void set_visited(bool visited) { visited_ = visited; }
 
   bool visited() { return visited_; }
@@ skipping 29 matching lines @@
 
   bool was_ever_ready_since_last_transform_animation() const {
     return was_ever_ready_since_last_transform_animation_;
   }
 
   void set_was_ever_ready_since_last_transform_animation(bool was_ready) {
     was_ever_ready_since_last_transform_animation_ = was_ready;
   }
 
   void NoteLayerPropertyChanged();
+  void DidUpdateScrollOffset();
 
   void PushLayerPropertyChangedForSubtree();
 
  protected:
   LayerImpl(LayerTreeImpl* layer_impl,
             int id,
             scoped_refptr<SyncedScrollOffset> scroll_offset);
   LayerImpl(LayerTreeImpl* layer_impl, int id);
 
   // Get the color and size of the layer's debug border.
@@ skipping 13 matching lines @@
   void NoteLayerPropertyChangedForSubtree();
 
   // Note carefully this does not affect the current layer.
   void NoteLayerPropertyChangedForDescendants();
 
   gfx::Rect GetScaledEnclosingRectInTargetSpace(float scale) const;
 
  private:
   void ValidateQuadResourcesInternal(DrawQuad* quad) const;
 
-  void PushScrollOffset(const gfx::ScrollOffset* scroll_offset);
-  void DidUpdateScrollOffset();
   void NoteLayerPropertyChangedForDescendantsInternal();
   void PushLayerPropertyChangedForSubtreeInternal();
 
   virtual const char* LayerTypeAsString() const;
 
   // Properties internal to LayerImpl
   LayerImpl* parent_;
   OwnedLayerImplList children_;
 
   LayerImpl* scroll_parent_;
@@ skipping 10 matching lines @@
   // mask_layer_ can be temporarily stolen during tree sync, we need this ID to
   // confirm newly assigned layer is still the previous one
   int mask_layer_id_;
   scoped_ptr<LayerImpl> mask_layer_;
   int replica_layer_id_;  // ditto
   scoped_ptr<LayerImpl> replica_layer_;
   int layer_id_;
   LayerTreeImpl* layer_tree_impl_;
   LayerListImpl* layer_list_impl_;
 
-  // Properties dynamically changeable on active tree.
-  scoped_refptr<SyncedScrollOffset> scroll_offset_;
   gfx::Vector2dF bounds_delta_;
 
   // Properties synchronized from the associated Layer.
   gfx::Point3F transform_origin_;
   gfx::Size bounds_;
   int scroll_clip_layer_id_;
 
   gfx::Vector2dF offset_to_transform_parent_;
   uint32_t main_thread_scrolling_reasons_;
 
@@ skipping 114 matching lines @@
   // If true, the layer or one of its descendants has a touch handler.
   bool layer_or_descendant_has_touch_handler_;
   bool sorted_for_recursion_;
 
   DISALLOW_COPY_AND_ASSIGN(LayerImpl);
 };
 
 }  // namespace cc
 
 #endif  // CC_LAYERS_LAYER_IMPL_H_