[LayoutNG] Force relayout when constraint spaces change.

This patch reuses a lot of the logic we currently have to skip layout
in LayoutNGMixin::CachedLayoutResult.
It also moves this logic from ng_length_utils.cc to ng_layout_utils.cc

We also make sure that if the constraint spaces don't match we force
a layout of the child.

Previously we were only performing a LayoutIfNeeded in this case, and
as a result children not respecting the available size, etc.

Bug: 635619, 928672
Change-Id: I36e248f1a7413f5becd2b39bdd2db25ca2843559
Reviewed-on: https://chromium-review.googlesource.com/c/1476263
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633839}

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

@@ -282,11 +282,6 @@ scoped_refptr<const NGLayoutResult> LayoutNGMixin<Base>::CachedLayoutResult(
   if (!cached_layout_result)
     return nullptr;
 
-  const NGConstraintSpace& old_space =
-      cached_layout_result->GetConstraintSpaceForCaching();
-  if (!new_space.MaySkipLayout(old_space))
-    return nullptr;
-
   // If we have an orthogonal flow root descendant, we don't attempt to cache
   // our layout result. This is because the initial containing block size may
   // have changed, having a high likelihood of changing the size of the
@@ -294,18 +289,11 @@ scoped_refptr<const NGLayoutResult> LayoutNGMixin<Base>::CachedLayoutResult(
   if (cached_layout_result->HasOrthogonalFlowRoots())
     return nullptr;
 
-  if (!new_space.AreSizesEqual(old_space)) {
-    // We need to descend all the way down into BODY if we're in quirks mode,
-    // since it magically follows the viewport size.
-    if (NGBlockNode(this).IsQuirkyAndFillsViewport())
-      return nullptr;
+  if (!MaySkipLayout(NGBlockNode(this), *cached_layout_result, new_space))
+    return nullptr;
 
-    // If the available / percentage sizes have changed in a way that may affect
-    // layout, we cannot re-use the previous result.
-    if (SizeMayChange(Base::StyleRef(), new_space, old_space,
-                      *cached_layout_result))
-      return nullptr;
-  }
+  const NGConstraintSpace& old_space =
+      cached_layout_result->GetConstraintSpaceForCaching();
 
   // Check BFC block offset. Even if they don't match, there're some cases we
   // can still reuse the fragment.

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

@@ -238,9 +238,8 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::Layout(
     const NGConstraintSpace& constraint_space,
     const NGBreakToken* break_token) {
   // Use the old layout code and synthesize a fragment.
-  if (!CanUseNewLayout()) {
+  if (!CanUseNewLayout())
     return RunOldLayout(constraint_space);
-  }
 
   LayoutBlockFlow* block_flow =
       box_->IsLayoutNGMixin() ? ToLayoutBlockFlow(box_) : nullptr;
@@ -898,8 +897,12 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::RunOldLayout(
   scoped_refptr<const NGLayoutResult> layout_result =
       box_->GetCachedLayoutResult();
 
-  if (box_->NeedsLayout() || !layout_result ||
-      layout_result->GetConstraintSpaceForCaching() != constraint_space) {
+  // We need to force a layout on the child if the constraint space given will
+  // change the layout.
+  bool needs_force_relayout =
+      layout_result && !MaySkipLayout(*this, *layout_result, constraint_space);
+
+  if (box_->NeedsLayout() || !layout_result || needs_force_relayout) {
     BoxLayoutExtraInput input(*box_);
     input.containing_block_content_inline_size =
         CalculateAvailableInlineSizeForLegacy(*box_, constraint_space);
@@ -926,7 +929,7 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::RunOldLayout(
     // Using |LayoutObject::LayoutIfNeeded| save us a little bit of overhead,
     // compared to |LayoutObject::ForceChildLayout|.
     DCHECK(!box_->IsLayoutNGMixin());
-    if (box_->NeedsLayout())
+    if (box_->NeedsLayout() && !needs_force_relayout)
       box_->LayoutIfNeeded();
     else
       box_->ForceChildLayout();

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

@@ -6,9 +6,146 @@
 
 #include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_length_utils.h"
 
 namespace blink {
 
+namespace {
+
+inline bool InlineLengthMayChange(const Length& length,
+                                  LengthResolveType type,
+                                  const NGConstraintSpace& new_space,
+                                  const NGConstraintSpace& old_space) {
+  // Percentage inline margins will affect the size if the size is unspecified
+  // (auto and similar). So we need to check both available size and the
+  // percentage resolution size in that case.
+  bool is_unspecified =
+      (length.IsAuto() && type != LengthResolveType::kMinSize) ||
+      length.IsFitContent() || length.IsFillAvailable();
+  if (is_unspecified) {
+    if (new_space.AvailableSize().inline_size !=
+        old_space.AvailableSize().inline_size)
+      return true;
+  }
+  if (is_unspecified || length.IsPercentOrCalc()) {
+    if (new_space.PercentageResolutionInlineSize() !=
+        old_space.PercentageResolutionInlineSize())
+      return true;
+  }
+  return false;
+}
+
+inline bool BlockLengthMayChange(const Length& length,
+                                 const NGConstraintSpace& new_space,
+                                 const NGConstraintSpace& old_space) {
+  if (length.IsFillAvailable()) {
+    if (new_space.AvailableSize().block_size !=
+        old_space.AvailableSize().block_size)
+      return true;
+  }
+  return false;
+}
+
+// Return true if it's possible (but not necessarily guaranteed) that the new
+// constraint space will give a different size compared to the old one, when
+// computed style and child content remain unchanged.
+bool SizeMayChange(const ComputedStyle& style,
+                   const NGConstraintSpace& new_space,
+                   const NGConstraintSpace& old_space,
+                   const NGLayoutResult& layout_result) {
+  DCHECK_EQ(new_space.IsFixedSizeInline(), old_space.IsFixedSizeInline());
+  DCHECK_EQ(new_space.IsFixedSizeBlock(), old_space.IsFixedSizeBlock());
+
+  // Go through all length properties, and, depending on length type
+  // (percentages, auto, etc.), check whether the constraint spaces differ in
+  // such a way that the resulting size *may* change. There are currently many
+  // possible false-positive situations here, as we don't rule out length
+  // changes that won't have any effect on the final size (e.g. if inline-size
+  // is 100px, max-inline-size is 50%, and percentage resolution inline size
+  // changes from 1000px to 500px). If the constraint space has "fixed" size in
+  // a dimension, we can skip checking properties in that dimension and just
+  // look for available size changes, since that's how a "fixed" constraint
+  // space works.
+  if (new_space.IsFixedSizeInline()) {
+    if (new_space.AvailableSize().inline_size !=
+        old_space.AvailableSize().inline_size)
+      return true;
+  } else {
+    if (InlineLengthMayChange(style.LogicalWidth(),
+                              LengthResolveType::kContentSize, new_space,
+                              old_space) ||
+        InlineLengthMayChange(style.LogicalMinWidth(),
+                              LengthResolveType::kMinSize, new_space,
+                              old_space) ||
+        InlineLengthMayChange(style.LogicalMaxWidth(),
+                              LengthResolveType::kMaxSize, new_space,
+                              old_space))
+      return true;
+  }
+
+  if (new_space.IsFixedSizeBlock()) {
+    if (new_space.AvailableSize().block_size !=
+        old_space.AvailableSize().block_size)
+      return true;
+  } else {
+    if (BlockLengthMayChange(style.LogicalHeight(), new_space, old_space) ||
+        BlockLengthMayChange(style.LogicalMinHeight(), new_space, old_space) ||
+        BlockLengthMayChange(style.LogicalMaxHeight(), new_space, old_space))
+      return true;
+    // We only need to check if the PercentageResolutionBlockSizes match if the
+    // layout result has explicitly marked itself as dependent.
+    if (layout_result.DependsOnPercentageBlockSize()) {
+      if (new_space.PercentageResolutionBlockSize() !=
+          old_space.PercentageResolutionBlockSize())
+        return true;
+      if (new_space.ReplacedPercentageResolutionBlockSize() !=
+          old_space.ReplacedPercentageResolutionBlockSize())
+        return true;
+    }
+  }
+
+  if (new_space.PercentageResolutionInlineSize() !=
+      old_space.PercentageResolutionInlineSize()) {
+    // Percentage-based padding is resolved against the inline content box size
+    // of the containing block.
+    if (style.PaddingTop().IsPercentOrCalc() ||
+        style.PaddingRight().IsPercentOrCalc() ||
+        style.PaddingBottom().IsPercentOrCalc() ||
+        style.PaddingLeft().IsPercentOrCalc())
+      return true;
+  }
+
+  return false;
+}
+
+}  // namespace
+
+bool MaySkipLayout(const NGBlockNode node,
+                   const NGLayoutResult& cached_layout_result,
+                   const NGConstraintSpace& new_space) {
+  DCHECK_EQ(cached_layout_result.Status(), NGLayoutResult::kSuccess);
+  DCHECK(cached_layout_result.HasValidConstraintSpaceForCaching());
+
+  const NGConstraintSpace& old_space =
+      cached_layout_result.GetConstraintSpaceForCaching();
+  if (!new_space.MaySkipLayout(old_space))
+    return false;
+
+  if (!new_space.AreSizesEqual(old_space)) {
+    // We need to descend all the way down into BODY if we're in quirks mode,
+    // since it magically follows the viewport size.
+    if (node.IsQuirkyAndFillsViewport())
+      return false;
+
+    // If the available / percentage sizes have changed in a way that may affect
+    // layout, we cannot re-use the previous result.
+    if (SizeMayChange(node.Style(), new_space, old_space, cached_layout_result))
+      return false;
+  }
+
+  return true;
+}
+
 bool IsBlockLayoutComplete(const NGConstraintSpace& space,
                            const NGLayoutResult& result) {
   if (result.Status() != NGLayoutResult::kSuccess)

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

@@ -5,15 +5,24 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_NG_LAYOUT_UTILS_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_NG_LAYOUT_UTILS_H_
 
+#include "third_party/blink/renderer/core/layout/ng/ng_block_node.h"
+
 namespace blink {
 
 class NGConstraintSpace;
 class NGLayoutResult;
 
+// Returns true if for a given |new_space|, the |node| will provide the same
+// |NGLayoutResult| as |cached_layout_result|, and therefore might be able to
+// skip layout.
+bool MaySkipLayout(const NGBlockNode node,
+                   const NGLayoutResult& cached_layout_result,
+                   const NGConstraintSpace& new_space);
+
 // Return true if layout is considered complete. In some cases we require more
 // than one layout pass.
-// This function never considers intermediate layouts
-// (space,IsIntermediateLayout()) to be complete.
+// This function never considers intermediate layouts with
+// |NGConstraintSpace::IsIntermediateLayout| to be complete.
 bool IsBlockLayoutComplete(const NGConstraintSpace&, const NGLayoutResult&);
 
 }  // namespace blink

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

@@ -11,7 +11,6 @@
 #include "third_party/blink/renderer/core/layout/ng/ng_block_node.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h"
-#include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_space_utils.h"
 #include "third_party/blink/renderer/core/style/computed_style.h"
 #include "third_party/blink/renderer/platform/geometry/layout_unit.h"
@@ -52,40 +51,6 @@ inline EBlockAlignment BlockAlignment(const ComputedStyle& style,
   }
 }
 
-inline bool InlineLengthMayChange(const Length& length,
-                                  LengthResolveType type,
-                                  const NGConstraintSpace& new_space,
-                                  const NGConstraintSpace& old_space) {
-  // Percentage inline margins will affect the size if the size is unspecified
-  // (auto and similar). So we need to check both available size and the
-  // percentage resolution size in that case.
-  bool is_unspecified =
-      (length.IsAuto() && type != LengthResolveType::kMinSize) ||
-      length.IsFitContent() || length.IsFillAvailable();
-  if (is_unspecified) {
-    if (new_space.AvailableSize().inline_size !=
-        old_space.AvailableSize().inline_size)
-      return true;
-  }
-  if (is_unspecified || length.IsPercentOrCalc()) {
-    if (new_space.PercentageResolutionInlineSize() !=
-        old_space.PercentageResolutionInlineSize())
-      return true;
-  }
-  return false;
-}
-
-inline bool BlockLengthMayChange(const Length& length,
-                                 const NGConstraintSpace& new_space,
-                                 const NGConstraintSpace& old_space) {
-  if (length.IsFillAvailable()) {
-    if (new_space.AvailableSize().block_size !=
-        old_space.AvailableSize().block_size)
-      return true;
-  }
-  return false;
-}
-
 }  // anonymous namespace
 
 bool NeedMinMaxSizeForContentContribution(WritingMode mode,
@@ -750,75 +715,6 @@ NGLogicalSize ComputeReplacedSize(
   return NGLogicalSize(*replaced_inline, *replaced_block);
 }
 
-bool SizeMayChange(const ComputedStyle& style,
-                   const NGConstraintSpace& new_space,
-                   const NGConstraintSpace& old_space,
-                   const NGLayoutResult& layout_result) {
-  DCHECK_EQ(new_space.IsFixedSizeInline(), old_space.IsFixedSizeInline());
-  DCHECK_EQ(new_space.IsFixedSizeBlock(), old_space.IsFixedSizeBlock());
-
-  // Go through all length properties, and, depending on length type
-  // (percentages, auto, etc.), check whether the constraint spaces differ in
-  // such a way that the resulting size *may* change. There are currently many
-  // possible false-positive situations here, as we don't rule out length
-  // changes that won't have any effect on the final size (e.g. if inline-size
-  // is 100px, max-inline-size is 50%, and percentage resolution inline size
-  // changes from 1000px to 500px). If the constraint space has "fixed" size in
-  // a dimension, we can skip checking properties in that dimension and just
-  // look for available size changes, since that's how a "fixed" constraint
-  // space works.
-  if (new_space.IsFixedSizeInline()) {
-    if (new_space.AvailableSize().inline_size !=
-        old_space.AvailableSize().inline_size)
-      return true;
-  } else {
-    if (InlineLengthMayChange(style.LogicalWidth(),
-                              LengthResolveType::kContentSize, new_space,
-                              old_space) ||
-        InlineLengthMayChange(style.LogicalMinWidth(),
-                              LengthResolveType::kMinSize, new_space,
-                              old_space) ||
-        InlineLengthMayChange(style.LogicalMaxWidth(),
-                              LengthResolveType::kMaxSize, new_space,
-                              old_space))
-      return true;
-  }
-
-  if (new_space.IsFixedSizeBlock()) {
-    if (new_space.AvailableSize().block_size !=
-        old_space.AvailableSize().block_size)
-      return true;
-  } else {
-    if (BlockLengthMayChange(style.LogicalHeight(), new_space, old_space) ||
-        BlockLengthMayChange(style.LogicalMinHeight(), new_space, old_space) ||
-        BlockLengthMayChange(style.LogicalMaxHeight(), new_space, old_space))
-      return true;
-    // We only need to check if the PercentageResolutionBlockSizes match if the
-    // layout result has explicitly marked itself as dependent.
-    if (layout_result.DependsOnPercentageBlockSize()) {
-      if (new_space.PercentageResolutionBlockSize() !=
-          old_space.PercentageResolutionBlockSize())
-        return true;
-      if (new_space.ReplacedPercentageResolutionBlockSize() !=
-          old_space.ReplacedPercentageResolutionBlockSize())
-        return true;
-    }
-  }
-
-  if (new_space.PercentageResolutionInlineSize() !=
-      old_space.PercentageResolutionInlineSize()) {
-    // Percentage-based padding is resolved against the inline content box size
-    // of the containing block.
-    if (style.PaddingTop().IsPercentOrCalc() ||
-        style.PaddingRight().IsPercentOrCalc() ||
-        style.PaddingBottom().IsPercentOrCalc() ||
-        style.PaddingLeft().IsPercentOrCalc())
-      return true;
-  }
-
-  return false;
-}
-
 int ResolveUsedColumnCount(int computed_count,
                            LayoutUnit computed_size,
                            LayoutUnit used_gap,

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

@@ -24,7 +24,6 @@ struct MinMaxSizeInput;
 class NGConstraintSpace;
 class NGBlockNode;
 class NGLayoutInputNode;
-class NGLayoutResult;
 
 // LengthResolvePhase indicates what type of layout pass we are currently in.
 // This changes how lengths are resolved. kIntrinsic must be used during the
@@ -159,14 +158,6 @@ ComputeReplacedSize(const NGLayoutInputNode&,
                     const NGConstraintSpace&,
                     const base::Optional<MinMaxSize>&);
 
-// Return true if it's possible (but not necessarily guaranteed) that the new
-// constraint space will give a different size compared to the old one, when
-// computed style and child content remain unchanged.
-bool SizeMayChange(const ComputedStyle&,
-                   const NGConstraintSpace& new_space,
-                   const NGConstraintSpace& old_space,
-                   const NGLayoutResult& layout_result);
-
 // Based on available inline size, CSS computed column-width, CSS computed
 // column-count and CSS used column-gap, return CSS used column-count.
 CORE_EXPORT int ResolveUsedColumnCount(int computed_count,

third_party/blink/web_tests/external/wpt/css/css-sizing/dynamic-available-size-iframe.html

+<!DOCTYPE html>
+<html class=reftest-wait>
+<meta name="assert" content="Checks that an absolute positioned element is positioned correctly, when the available size of its parent changes due to document resize." />
+<link rel="help" href="https://crbug.com/928672">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<script src="/common/reftest-wait.js"></script>
+<p>Test passes if there is a filled green square.</p>
+<iframe id="target" height="100" width="200" src="support/dynamic-available-size-iframe.html" style="border: none;"></iframe>
+<script>
+onload = () => {
+  requestAnimationFrame(() => requestAnimationFrame(() => {
+    target.width = '100';
+    takeScreenshot();
+  }));
+};
+</script>

third_party/blink/web_tests/external/wpt/css/css-sizing/support/dynamic-available-size-iframe.html

+<!DOCTYPE html>
+<style>
+body { margin: 0; }
+.parent {
+  position: relative;
+  display: flex;
+  width: 100%;
+  height: 100px;
+  background: red;
+}
+
+.content {
+  position: absolute;
+  top: 0;
+  left: 0;
+  right: 0;
+  bottom: 0;
+  margin: auto;
+  width: 100px;
+  height: 100px;
+  background: green;
+}
+
+svg {
+  width: 50px;
+  height: 50px;
+}
+</style>
+<div class="parent">
+  <div class="content">
+    <svg xmlns="http://www.w3.org/2000/svg"></svg>
+  </div>
+</div>