[LayoutNG] Don't miss cache on changing table height restrictedness.

When a table-cell changes between having a restricted block-size and not
having one, that doesn't imply that we have to re-lay out all children.
Move the bit out of ConstraintSpaceFlags, so that we don't force a cache
miss on all children when this happens, but rather check if the size of
the children will actually change.

This is a follow-up on
https://chromium-review.googlesource.com/c/chromium/src/+/1621166

Bug: 964282
Change-Id: I56bca9eb820a008b54dfb7279860114939a1fe8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1624325Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662174}

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

@@ -67,10 +67,9 @@ class CORE_EXPORT NGConstraintSpace final {
     kAnonymous = 1 << 5,
     kUseFirstLineStyle = 1 << 6,
     kAncestorHasClearancePastAdjoiningFloats = 1 << 7,
-    kInRestrictedBlockSizeTableCell = 1 << 8,
 
     // Size of bitfield used to store the flags.
-    kNumberOfConstraintSpaceFlags = 9
+    kNumberOfConstraintSpaceFlags = 8
   };
 
   // To ensure that the bfc_offset_, rare_data_ union doesn't get polluted,
@@ -334,7 +333,7 @@ class CORE_EXPORT NGConstraintSpace final {
   }
 
   bool IsInRestrictedBlockSizeTableCell() const {
-    return HasFlag(kInRestrictedBlockSizeTableCell);
+    return bitfields_.is_in_restricted_block_size_table_cell;
   }
 
   NGMarginStrut MarginStrut() const {
@@ -556,6 +555,7 @@ class CORE_EXPORT NGConstraintSpace final {
           is_shrink_to_fit(false),
           is_fixed_size_inline(false),
           is_fixed_size_block(false),
+          is_in_restricted_block_size_table_cell(false),
           table_cell_child_layout_phase(static_cast<unsigned>(
               NGTableCellChildLayoutPhase::kNotTableCellChild)),
           flags(kFixedSizeBlockIsDefinite),
@@ -573,6 +573,8 @@ class CORE_EXPORT NGConstraintSpace final {
       return is_shrink_to_fit == other.is_shrink_to_fit &&
              is_fixed_size_inline == other.is_fixed_size_inline &&
              is_fixed_size_block == other.is_fixed_size_block &&
+             is_in_restricted_block_size_table_cell ==
+                 other.is_in_restricted_block_size_table_cell &&
              table_cell_child_layout_phase ==
                  other.table_cell_child_layout_phase;
     }
@@ -586,6 +588,7 @@ class CORE_EXPORT NGConstraintSpace final {
     unsigned is_shrink_to_fit : 1;
     unsigned is_fixed_size_inline : 1;
     unsigned is_fixed_size_block : 1;
+    unsigned is_in_restricted_block_size_table_cell : 1;
     unsigned table_cell_child_layout_phase : 2;  // NGTableCellChildLayoutPhase
 
     unsigned flags : kNumberOfConstraintSpaceFlags;  // ConstraintSpaceFlags

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

@@ -250,7 +250,7 @@ class CORE_EXPORT NGConstraintSpaceBuilder final {
   }
 
   NGConstraintSpaceBuilder& SetIsInRestrictedBlockSizeTableCell() {
-    SetFlag(NGConstraintSpace::kInRestrictedBlockSizeTableCell, true);
+    space_.bitfields_.is_in_restricted_block_size_table_cell = true;
     return *this;
   }
 

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

@@ -820,5 +820,75 @@ TEST_F(NGLayoutResultCachingTest, HitStandardsModePercentageBasedChild) {
   EXPECT_NE(result.get(), nullptr);
 }
 
+TEST_F(NGLayoutResultCachingTest, ChangeTableCellBlockSizeConstrainedness) {
+  ScopedLayoutNGFragmentCachingForTest layout_ng_fragment_caching(true);
+
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      .table { display: table; width: 300px; }
+      .cell { display: table-cell; }
+      .child1 { height: 100px; }
+      .child2, .child3 { overflow:auto; height:10%; }
+    </style>
+    <div class="table">
+      <div class="cell">
+        <div class="child1" id="test1"></div>
+        <div class="child2" id="test2">
+          <div style="height:30px;"></div>
+        </div>
+        <div class="child3" id="test3"></div>
+      </div>
+    </div>
+    <div class="table" style="height:300px;">
+      <div class="cell">
+        <div class="child1" id="src1"></div>
+        <div class="child2" id="src2">
+          <div style="height:30px;"></div>
+        </div>
+        <div class="child3" id="src3"></div>
+      </div>
+    </div>
+  )HTML");
+
+  auto* test1 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("test1"));
+  auto* test2 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("test2"));
+  auto* test3 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("test3"));
+  auto* src1 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("src1"));
+  auto* src2 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("src2"));
+  auto* src3 = To<LayoutBlockFlow>(GetLayoutObjectByElementId("src3"));
+
+  NGLayoutCacheStatus cache_status;
+  base::Optional<NGFragmentGeometry> fragment_geometry;
+  NGConstraintSpace space =
+      src1->GetCachedLayoutResult()->GetConstraintSpaceForCaching();
+  scoped_refptr<const NGLayoutResult> result = test1->CachedLayoutResult(
+      space, nullptr, &fragment_geometry, &cache_status);
+  // The first child has a fixed height, and shouldn't be affected by the cell
+  // height.
+  EXPECT_EQ(cache_status, NGLayoutCacheStatus::kHit);
+  EXPECT_NE(result.get(), nullptr);
+
+  fragment_geometry.reset();
+  space = src2->GetCachedLayoutResult()->GetConstraintSpaceForCaching();
+  result = test2->CachedLayoutResult(space, nullptr, &fragment_geometry,
+                                     &cache_status);
+  // The second child has overflow:auto and a percentage height, but its
+  // intrinsic height is identical to its extrinsic height (when the cell has a
+  // height). So it won't need layout, either.
+  EXPECT_EQ(cache_status, NGLayoutCacheStatus::kHit);
+  EXPECT_NE(result.get(), nullptr);
+
+  fragment_geometry.reset();
+  space = src3->GetCachedLayoutResult()->GetConstraintSpaceForCaching();
+  result = test3->CachedLayoutResult(space, nullptr, &fragment_geometry,
+                                     &cache_status);
+  // The third child has overflow:auto and a percentage height, and its
+  // intrinsic height is 0 (no children), so it matters whether the cell has a
+  // height or not. We're only going to need simplified layout, though, since no
+  // children will be affected by its height change.
+  EXPECT_EQ(cache_status, NGLayoutCacheStatus::kNeedsSimplifiedLayout);
+  EXPECT_EQ(result.get(), nullptr);
+}
+
 }  // namespace
 }  // namespace blink