Slightly refactor StickyPositionScrollingConstraints API and add documentation

third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 36 matching lines @@
 namespace blink {
 
 namespace {
 inline bool IsOutOfFlowPositionedWithImplicitHeight(
     const LayoutBoxModelObject* child) {
   return child->IsOutOfFlowPositioned() &&
          !child->Style()->LogicalTop().IsAuto() &&
          !child->Style()->LogicalBottom().IsAuto();
 }
 
-StickyPositionScrollingConstraints* StickyConstraintsForLayoutObject(
-    const LayoutBoxModelObject* obj,
-    const PaintLayer* ancestor_overflow_layer) {
-  if (!obj)
-    return nullptr;
-
-  PaintLayerScrollableArea* scrollable_area =
-      ancestor_overflow_layer->GetScrollableArea();
-  auto it = scrollable_area->GetStickyConstraintsMap().find(obj->Layer());
-  if (it == scrollable_area->GetStickyConstraintsMap().end())
-    return nullptr;
-
-  return &it->value;
-}
-
 // Inclusive of |from|, exclusive of |to|.
-LayoutBoxModelObject* FindFirstStickyBetween(LayoutObject* from,
-                                             LayoutObject* to) {
+PaintLayer* FindFirstStickyBetween(LayoutObject* from, LayoutObject* to) {
   LayoutObject* maybe_sticky_ancestor = from;
   while (maybe_sticky_ancestor && maybe_sticky_ancestor != to) {
     if (maybe_sticky_ancestor->Style()->HasStickyConstrainedPosition()) {
-      return ToLayoutBoxModelObject(maybe_sticky_ancestor);
+      return ToLayoutBoxModelObject(maybe_sticky_ancestor)->Layer();
     }
 
     maybe_sticky_ancestor =
         maybe_sticky_ancestor->IsLayoutInline()
             ? maybe_sticky_ancestor->Container()
             : ToLayoutBox(maybe_sticky_ancestor)->LocationContainer();
   }
   return nullptr;
 }
 }  // namespace
@@ skipping 902 matching lines @@
       FloatRect(scroll_container_relative_padding_box_rect.Location() +
                     ToFloatSize(sticky_location),
                 flipped_sticky_box_rect.Size()));
 
   // To correctly compute the offsets, the constraints need to know about any
   // nested position:sticky elements between themselves and their
   // containingBlock, and between the containingBlock and their scrollAncestor.
   //
   // The respective search ranges are [container, containingBlock) and
   // [containingBlock, scrollAncestor).
-  constraints.SetNearestStickyBoxShiftingStickyBox(
+  constraints.SetNearestStickyLayerShiftingStickyBox(
       FindFirstStickyBetween(location_container, containing_block));
   // We cannot use |scrollAncestor| here as it disregards the root
   // ancestorOverflowLayer(), which we should include.
-  constraints.SetNearestStickyBoxShiftingContainingBlock(FindFirstStickyBetween(
-      containing_block, &Layer()->AncestorOverflowLayer()->GetLayoutObject()));
+  constraints.SetNearestStickyLayerShiftingContainingBlock(
+      FindFirstStickyBetween(
+          containing_block,
+          &Layer()->AncestorOverflowLayer()->GetLayoutObject()));
 
   // We skip the right or top sticky offset if there is not enough space to
   // honor both the left/right or top/bottom offsets.
   LayoutUnit horizontal_offsets =
       MinimumValueForLength(Style()->Right(),
                             LayoutUnit(constraining_size.Width())) +
       MinimumValueForLength(Style()->Left(),
                             LayoutUnit(constraining_size.Width()));
   bool skip_right = false;
   bool skip_left = false;
@@ skipping 80 matching lines @@
   return constraining_rect;
 }
 
 LayoutSize LayoutBoxModelObject::StickyPositionOffset() const {
   const PaintLayer* ancestor_overflow_layer = Layer()->AncestorOverflowLayer();
   // TODO: Force compositing input update if we ask for offset before
   // compositing inputs have been computed?
   if (!ancestor_overflow_layer)
     return LayoutSize();
 
-  StickyPositionScrollingConstraints* constraints =
-      StickyConstraintsForLayoutObject(this, ancestor_overflow_layer);
-  if (!constraints)
+  StickyConstraintsMap& constraints_map =
+      ancestor_overflow_layer->GetScrollableArea()->GetStickyConstraintsMap();
+  auto it = constraints_map.find(Layer());
+  if (it == constraints_map.end())
     return LayoutSize();
-
-  StickyPositionScrollingConstraints* shifting_sticky_box_constraints =
-      StickyConstraintsForLayoutObject(
-          constraints->NearestStickyBoxShiftingStickyBox(),
-          ancestor_overflow_layer);
-
-  StickyPositionScrollingConstraints* shifting_containing_block_constraints =
-      StickyConstraintsForLayoutObject(
-          constraints->NearestStickyBoxShiftingContainingBlock(),
-          ancestor_overflow_layer);
+  StickyPositionScrollingConstraints* constraints = &it->value;

[smcgruer, 2017/06/26 19:18:56]

Refactoring this code made me realize how dangerous this behavior is.
StickyPositionScrollingConstraints is still being treated like a const PoD,
however we need the specific object in the hashmap to be updated when
ComputeStickyOffset is called. That only happens properly afaik because HashMap
iteration happens to be implemented that way.

I'm pretty sure we should change the map to be PaintLayer* ->
std::unique_ptr<StickyPositionScrollingConstraints>, but haven't done so here.

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

I think it's okay to modify the value being pointed to by the hashmap.

 
   // The sticky offset is physical, so we can just return the delta computed in
   // absolute coords (though it may be wrong with transforms).
   FloatRect constraining_rect = ComputeStickyConstrainingRect();
-  return LayoutSize(constraints->ComputeStickyOffset(
-      constraining_rect, shifting_sticky_box_constraints,
-      shifting_containing_block_constraints));
+  return LayoutSize(
+      constraints->ComputeStickyOffset(constraining_rect, constraints_map));
 }
 
 LayoutPoint LayoutBoxModelObject::AdjustedPositionRelativeTo(
     const LayoutPoint& start_point,
     const Element* offset_parent) const {
   // If the element is the HTML body element or doesn't have a parent
   // return 0 and stop this algorithm.
   if (IsBody() || !Parent())
     return LayoutPoint();
 
@@ skipping 351 matching lines @@
   if (root_element_style->HasBackground())
     return false;
 
   if (GetNode() != GetDocument().FirstBodyElement())
     return false;
 
   return true;
 }
 
 }  // namespace blink