Slightly refactor StickyPositionScrollingConstraints API and add documentation

third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h

Index: third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h
diff --git a/third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h b/third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h
index 428531757cec98cfd1175c4089b07cf026a6d1cb..3d79ceec38e4fcc1987ce3a7666afb103bfb1909 100644
--- a/third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h
+++ b/third_party/WebKit/Source/core/page/scrolling/StickyPositionScrollingConstraints.h
@@ -11,15 +11,31 @@
 
 namespace blink {
 
-class LayoutBoxModelObject;
 class PaintLayer;
 class StickyPositionScrollingConstraints;
 
 typedef WTF::HashMap<PaintLayer*, StickyPositionScrollingConstraints>
     StickyConstraintsMap;
 
-// TODO(smcgruer): Add detailed comment explaining how
-// StickyPositionScrollingConstraints works.
+// Encapsulates the constraint information for a position: sticky element and
+// provides calculation of the sticky offset for a given viewport rectangle.

[flackr, 2017/06/29 15:24:10]

I think viewport just refers to the frame scroller, mention overflow clip
rectangle for the overflow scrollers.

[smcgruer, 2017/06/29 18:51:47]

Done.

+//
+// To avoid slowing down scrolling we cannot make the offset calculation a
+// layout-inducing event. Instead constraint information is cached during layout
+// and used as the scroll position changes to determine the current offset. In
+// most cases the only information that is needed is the sticky element
+// rectangle, it's containing block rectangle, and the set of sticky edge
+// constraints (i.e. the distance from each edge the element should stick).

[flackr, 2017/06/29 15:24:10]

Mention that all of these are rectangles with respect to the nearest ancestor
scroller, which the sticky elements stick to.

[smcgruer, 2017/06/29 18:51:47]

Done.

+//
+// For a given (non-cached) viewport rectangle, calculating the current offset
+// in most cases just requires sliding the (cached) sticky element rectangle
+// until it satisfies the (cached) sticky edge constraints for the viewport
+// rectangle, whilst not letting the sticky element rectangle escape it's
+// (cached) containing block rect.
+//
+// Unfortunately this approach breaks down in the presence of nested sticky
+// elements. For those we apply more complicated logic, which is explained in
+// the implementation of |ComputeStickyOffset|.
 class StickyPositionScrollingConstraints final {
  public:
   enum AnchorEdgeFlags {
@@ -36,8 +52,8 @@ class StickyPositionScrollingConstraints final {
         right_offset_(0),
         top_offset_(0),
         bottom_offset_(0),
-        nearest_sticky_box_shifting_sticky_box_(nullptr),
-        nearest_sticky_box_shifting_containing_block_(nullptr) {}
+        nearest_sticky_layer_shifting_sticky_box_(nullptr),
+        nearest_sticky_layer_shifting_containing_block_(nullptr) {}
 
   StickyPositionScrollingConstraints(
       const StickyPositionScrollingConstraints& other)
@@ -50,23 +66,31 @@ class StickyPositionScrollingConstraints final {
             other.scroll_container_relative_containing_block_rect_),
         scroll_container_relative_sticky_box_rect_(
             other.scroll_container_relative_sticky_box_rect_),
-        nearest_sticky_box_shifting_sticky_box_(
-            other.nearest_sticky_box_shifting_sticky_box_),
-        nearest_sticky_box_shifting_containing_block_(
-            other.nearest_sticky_box_shifting_containing_block_),
+        nearest_sticky_layer_shifting_sticky_box_(
+            other.nearest_sticky_layer_shifting_sticky_box_),
+        nearest_sticky_layer_shifting_containing_block_(
+            other.nearest_sticky_layer_shifting_containing_block_),
         total_sticky_box_sticky_offset_(other.total_sticky_box_sticky_offset_),
         total_containing_block_sticky_offset_(
             other.total_containing_block_sticky_offset_) {}
 
-  FloatSize ComputeStickyOffset(
-      const FloatRect& viewport_rect,
-      const StickyPositionScrollingConstraints* ancestor_sticky_box_constraints,
-      const StickyPositionScrollingConstraints*
-          ancestor_containing_block_constraints);
+  // Computes the sticky offset for a given viewport rect.
+  //
+  // This method is non-const as we cache internal state for performance; see
+  // documentation in the implementation for details.
+  FloatSize ComputeStickyOffset(const FloatRect& viewport_rect,
+                                const StickyConstraintsMap&);
+
+  // Returns the last-computed offset of the sticky box from its original
+  // position before scroll.
+  //
+  // This method exists for performance (to avoid recomputing the sticky offset)
+  // and is only safe to call if ComputeStickyOffset has already been invoked.

[flackr, 2017/06/29 15:24:10]

Maybe it's better to relate this to when we expect ComputeStickyOffset must have
been invoked by, i.e. when compositing inputs (or prepaint after spv2 iiuc) are
clean for the sticky element.

[smcgruer, 2017/06/29 18:51:47]

Done.

+  FloatSize GetOffsetForStickyPosition(const StickyConstraintsMap&) const;
 
   bool HasAncestorStickyElement() const {
-    return nearest_sticky_box_shifting_sticky_box_ ||
-           nearest_sticky_box_shifting_containing_block_;
+    return nearest_sticky_layer_shifting_sticky_box_ ||
+           nearest_sticky_layer_shifting_containing_block_;
   }
 
   AnchorEdges GetAnchorEdges() const { return anchor_edges_; }
@@ -74,7 +98,6 @@ class StickyPositionScrollingConstraints final {
     return anchor_edges_ & flag;
   }
   void AddAnchorEdge(AnchorEdgeFlags edge_flag) { anchor_edges_ |= edge_flag; }
-  void SetAnchorEdges(AnchorEdges edges) { anchor_edges_ = edges; }
 
   float LeftOffset() const { return left_offset_; }
   float RightOffset() const { return right_offset_; }
@@ -100,45 +123,18 @@ class StickyPositionScrollingConstraints final {
     return scroll_container_relative_sticky_box_rect_;
   }
 
-  void SetNearestStickyBoxShiftingStickyBox(LayoutBoxModelObject* layer) {
-    nearest_sticky_box_shifting_sticky_box_ = layer;
+  void SetNearestStickyLayerShiftingStickyBox(PaintLayer* layer) {
+    nearest_sticky_layer_shifting_sticky_box_ = layer;
   }
-  LayoutBoxModelObject* NearestStickyBoxShiftingStickyBox() const {
-    return nearest_sticky_box_shifting_sticky_box_;
+  PaintLayer* NearestStickyLayerShiftingStickyBox() const {
+    return nearest_sticky_layer_shifting_sticky_box_;
   }
 
-  void SetNearestStickyBoxShiftingContainingBlock(LayoutBoxModelObject* layer) {
-    nearest_sticky_box_shifting_containing_block_ = layer;
+  void SetNearestStickyLayerShiftingContainingBlock(PaintLayer* layer) {
+    nearest_sticky_layer_shifting_containing_block_ = layer;
   }
-  LayoutBoxModelObject* NearestStickyBoxShiftingContainingBlock() const {
-    return nearest_sticky_box_shifting_containing_block_;
-  }
-
-  const FloatSize& GetTotalStickyBoxStickyOffset() const {
-    return total_sticky_box_sticky_offset_;
-  }
-  const FloatSize& GetTotalContainingBlockStickyOffset() const {
-    return total_containing_block_sticky_offset_;
-  }
-
-  // Returns the relative position of the sticky box and its original position
-  // before scroll. This method is only safe to call if ComputeStickyOffset has
-  // been invoked.
-  FloatSize GetOffsetForStickyPosition(const StickyConstraintsMap&) const;
-
-  const LayoutBoxModelObject* NearestStickyAncestor() const {
-    // If we have one or more sticky ancestor elements between ourselves and our
-    // containing block, |m_nearestStickyBoxShiftingStickyBox| points to the
-    // closest. Otherwise, |m_nearestStickyBoxShiftingContainingBlock| points
-    // to the the first sticky ancestor between our containing block (inclusive)
-    // and our scroll ancestor (exclusive). Therefore our nearest sticky
-    // ancestor is the former if it exists, or the latter otherwise.
-    //
-    // If both are null, then we have no sticky ancestors before our scroll
-    // ancestor, so the correct action is to return null.
-    return nearest_sticky_box_shifting_sticky_box_
-               ? nearest_sticky_box_shifting_sticky_box_
-               : nearest_sticky_box_shifting_containing_block_;
+  PaintLayer* NearestStickyLayerShiftingContainingBlock() const {
+    return nearest_sticky_layer_shifting_containing_block_;
   }
 
   bool operator==(const StickyPositionScrollingConstraints& other) const {
@@ -150,10 +146,10 @@ class StickyPositionScrollingConstraints final {
                other.scroll_container_relative_containing_block_rect_ &&
            scroll_container_relative_sticky_box_rect_ ==
                other.scroll_container_relative_sticky_box_rect_ &&
-           nearest_sticky_box_shifting_sticky_box_ ==
-               other.nearest_sticky_box_shifting_sticky_box_ &&
-           nearest_sticky_box_shifting_containing_block_ ==
-               other.nearest_sticky_box_shifting_containing_block_ &&
+           nearest_sticky_layer_shifting_sticky_box_ ==
+               other.nearest_sticky_layer_shifting_sticky_box_ &&
+           nearest_sticky_layer_shifting_containing_block_ ==
+               other.nearest_sticky_layer_shifting_containing_block_ &&
            total_sticky_box_sticky_offset_ ==
                other.total_sticky_box_sticky_offset_ &&
            total_containing_block_sticky_offset_ ==
@@ -165,21 +161,35 @@ class StickyPositionScrollingConstraints final {
   }
 
  private:
+  FloatSize AncestorStickyBoxOffset(const StickyConstraintsMap&);
+  FloatSize AncestorContainingBlockOffset(const StickyConstraintsMap&);
+
   AnchorEdges anchor_edges_;
   float left_offset_;
   float right_offset_;
   float top_offset_;
   float bottom_offset_;
+
+  // The layout position of the sticky element's containing block rectangle,
+  // relative to the scroll container. When calculating the sticky offset it is
+  // used to ensure the sticky element stays bounded by it's containing block.
   FloatRect scroll_container_relative_containing_block_rect_;
+
+  // The layout position of the sticky element. When calculating the sticky
+  // offset it is used to determine how large the offset needs to be to satisfy
+  // the sticky constraints.
   FloatRect scroll_container_relative_sticky_box_rect_;
 
-  // In order to properly compute the stickyOffset, we need to know if we have
-  // any sticky ancestors both between ourselves and our containing block and
-  // between our containing block and the viewport. These ancestors are needed
-  // to properly shift our constraining rects with regards to the containing
-  // block and viewport.
-  LayoutBoxModelObject* nearest_sticky_box_shifting_sticky_box_;
-  LayoutBoxModelObject* nearest_sticky_box_shifting_containing_block_;
+  // In the case of nested sticky elements the layout position of the sticky
+  // element and it's containing block are not accurate (as they are affected by
+  // ancestor sticky offsets). To ensure a correct sticky offset calculation in
+  // that case we must track any sticky ancestors between the sticky element and
+  // it's containing block, and between it's containing block and the viewport.
+  //
+  // See the implementation of |ComputeStickyOffset| for documentation on how
+  // these ancestors are used to correct the offset calculation.
+  PaintLayer* nearest_sticky_layer_shifting_sticky_box_;
+  PaintLayer* nearest_sticky_layer_shifting_containing_block_;
 
   // For performance we cache our accumulated sticky offset to allow descendant
   // sticky elements to offset their constraint rects. Because we can either