[LayoutNG] Disable first-tier OOF-cache when borders/scrollbars are present.

The first-tier OOF-cache attempts to skip all the OOF sizing and positioning
work required, as the other cache simply compares the constraint spaces
which can be (relatively) expensive to determine.

This first-tier cache works by comparing the available-size given to the
OOF-positioned descendant. If equal we can skip the OOF-positioned sizing
and positioning steps.

This mostly worked, however had an issue where the containing-block may have
added/removed a borders/scrollbars, but the available-size given remained
the same.

This resulted in an incorrectly positioned OOF-positioned node.

This fix disallows the first-tier cache when borders/scrollbars are present.
This is sufficiently rare that this cache still has a relatively high
hit-rate and is still relative cheap to calculate.

Bug: 1013866
Change-Id: I71d41d2c4b47bcff6412f395db2deabf2f6f3a8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1891768
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714394}

third_party/blink/renderer/core/layout/ng/ng_block_node.cc

@@ -439,6 +439,11 @@ NGBlockNode::CachedLayoutResultForOutOfFlowPositioned(
   if (!cached_layout_result)
     return nullptr;
 
+  // The containing-block may have borders/scrollbars which might change
+  // between passes affecting the final position.
+  if (!cached_layout_result->CanUseOutOfFlowPositionedFirstTierCache())
+    return nullptr;
+
   // TODO(layout-dev): There are potentially more cases where we can reuse this
   // layout result.
   // E.g. when we have a fixed-length top position constraint (top: 5px), we

third_party/blink/renderer/core/layout/ng/ng_block_node.h

@@ -53,7 +53,11 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode {
   //
   // As OOF-positioned objects have their position, and size computed
   // pre-layout, we need a way to quickly determine if we need to perform this
-  // work. This method compares the containing-block size to determine this.
+  // work.
+  //
+  // We have this "first-tier" cache explicitly for this purpose.
+  // This method compares the containing-block size to determine if we can skip
+  // the position, and size calculation.
   //
   // If the containing-block size hasn't changed, and we are layout-clean we
   // can reuse the previous layout result.

third_party/blink/renderer/core/layout/ng/ng_layout_result.h

@@ -68,6 +68,12 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
                          : oof_positioned_offset_;
   }
 
+  // Returns if we can use the first-tier OOF-positioned cache.
+  bool CanUseOutOfFlowPositionedFirstTierCache() const {
+    DCHECK(physical_fragment_->IsOutOfFlowPositioned());
+    return bitfields_.can_use_out_of_flow_positioned_first_tier_cache;
+  }
+
   const NGUnpositionedListMarker UnpositionedListMarker() const {
     return HasRareData() ? rare_data_->unpositioned_list_marker
                          : NGUnpositionedListMarker();
@@ -243,13 +249,18 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
    protected:
     friend class NGOutOfFlowLayoutPart;
 
-    void SetOutOfFlowPositionedOffset(const LogicalOffset& offset) {
+    void SetOutOfFlowPositionedOffset(
+        const LogicalOffset& offset,
+        bool can_use_out_of_flow_positioned_first_tier_cache) {
       // OOF-positioned nodes *must* always have an initial BFC-offset.
       DCHECK(layout_result_->physical_fragment_->IsOutOfFlowPositioned());
       DCHECK_EQ(layout_result_->BfcLineOffset(), LayoutUnit());
       DCHECK_EQ(layout_result_->BfcBlockOffset().value_or(LayoutUnit()),
                 LayoutUnit());
 
+      layout_result_->bitfields_
+          .can_use_out_of_flow_positioned_first_tier_cache =
+          can_use_out_of_flow_positioned_first_tier_cache;
       layout_result_->bitfields_.has_oof_positioned_offset = true;
       if (layout_result_->HasRareData())
         layout_result_->rare_data_->oof_positioned_offset = offset;
@@ -376,6 +387,7 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
         : has_rare_data(false),
           has_rare_data_exclusion_space(false),
           has_oof_positioned_offset(false),
+          can_use_out_of_flow_positioned_first_tier_cache(false),
           is_bfc_block_offset_nullopt(false),
           has_forced_break(false),
           is_self_collapsing(is_self_collapsing),
@@ -392,6 +404,7 @@ class CORE_EXPORT NGLayoutResult : public RefCounted<NGLayoutResult> {
     unsigned has_rare_data : 1;
     unsigned has_rare_data_exclusion_space : 1;
     unsigned has_oof_positioned_offset : 1;
+    unsigned can_use_out_of_flow_positioned_first_tier_cache : 1;
     unsigned is_bfc_block_offset_nullopt : 1;
 
     unsigned has_forced_break : 1;

third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc

@@ -90,7 +90,8 @@ NGOutOfFlowLayoutPart::NGOutOfFlowLayoutPart(
       container_builder_(container_builder),
       writing_mode_(container_style.GetWritingMode()),
       is_absolute_container_(is_absolute_container),
-      is_fixed_container_(is_fixed_container) {
+      is_fixed_container_(is_fixed_container),
+      allow_first_tier_oof_cache_(border_scrollbar.IsEmpty()) {
   if (!container_builder->HasOutOfFlowPositionedCandidates() &&
       !To<LayoutBlock>(container_builder_->GetLayoutObject())
            ->HasPositionedObjects())
@@ -460,10 +461,13 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::LayoutCandidate(
   // Determine if we need to actually run the full OOF-positioned sizing, and
   // positioning algorithm.
   //
-  // When this candidate has an inline container, the container may move without
-  // setting |NeedsLayout()| to the candidate and that there are cases where the
-  // cache validity cannot be determined.
-  if (!candidate.inline_container) {
+  // The first-tier cache compares the given available-size. However we can't
+  // reuse the result if the |ContainingBlockInfo::container_offset| may change.
+  // This can occur when:
+  //  - The default containing-block has borders and/or scrollbars.
+  //  - The candidate has an inline container (instead of the default
+  //    containing-block).
+  if (allow_first_tier_oof_cache_ && !candidate.inline_container) {
     LogicalSize container_content_size_in_candidate_writing_mode =
         container_physical_content_size.ConvertToLogical(
             candidate_writing_mode);
@@ -732,7 +736,8 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
       offset.inline_offset = *y;
   }
 
-  layout_result->GetMutableForOutOfFlow().SetOutOfFlowPositionedOffset(offset);
+  layout_result->GetMutableForOutOfFlow().SetOutOfFlowPositionedOffset(
+      offset, allow_first_tier_oof_cache_);
   return layout_result;
 }
 
@@ -740,8 +745,8 @@ bool NGOutOfFlowLayoutPart::IsContainingBlockForCandidate(
     const NGLogicalOutOfFlowPositionedNode& candidate) {
   EPosition position = candidate.node.Style().GetPosition();
 
-  // Candidates whose containing block is inline are always positioned
-  // inside closest parent block flow.
+  // Candidates whose containing block is inline are always positioned inside
+  // closest parent block flow.
   if (candidate.inline_container) {
     DCHECK(
         candidate.node.Style().GetPosition() == EPosition::kAbsolute &&
@@ -765,15 +770,13 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::GenerateFragment(
     const LogicalSize& container_content_size_in_candidate_writing_mode,
     const base::Optional<LayoutUnit>& block_estimate,
     const NGLogicalOutOfFlowPosition& node_position) {
-  // As the |block_estimate| is always in the node's writing mode, we
-  // build the constraint space in the node's writing mode.
+  // As the |block_estimate| is always in the node's writing mode, we build the
+  // constraint space in the node's writing mode.
   WritingMode writing_mode = node.Style().GetWritingMode();
 
   LayoutUnit inline_size = node_position.size.inline_size;
-  LayoutUnit block_size =
-      block_estimate
-          ? *block_estimate
-          : container_content_size_in_candidate_writing_mode.block_size;
+  LayoutUnit block_size = block_estimate.value_or(
+      container_content_size_in_candidate_writing_mode.block_size);
 
   LogicalSize available_size(inline_size, block_size);
 

third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.h

@@ -131,6 +131,7 @@ class CORE_EXPORT NGOutOfFlowLayoutPart {
   const WritingMode writing_mode_;
   bool is_absolute_container_;
   bool is_fixed_container_;
+  bool allow_first_tier_oof_cache_;
 };
 
 }  // namespace blink

third_party/blink/web_tests/external/wpt/css/css-position/position-absolute-dynamic-relayout-002.html

+<!DOCTYPE html>
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://www.w3.org/TR/css-position-3/" />
+<meta name="assert" content="This test checks that a dynamic change padding-box location, but no change in available-size positions correctly an absolute child correctly."/>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div id="target" style="position: relative; background: red; margin: 25px; width: 50px; height: 50px;">
+  <div style="position: absolute; top: 0; left: 0; width: 50px; height: 50px; background: green;"></div>
+</div>
+<script>
+  document.body.offsetTop;
+  const target = document.getElementById('target');
+  target.style.border = '25px solid green';
+  target.style.margin = '0';
+</script>