[FlexNG] Fix %-relpos for a grandchild of a flexbox.

When we resolve %-based offsets for a relative position child, if its parent is a flex-item we'll read the constraint-space used to generate this fragment, and resolve the %-based offset using that block-size. I.e. https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/layout/layout_box.cc;l=4521;drc=3f50460dcc4c426eb876cd0a1d2838eb35472e18?originalUrl=https:%2F%2Fcs.chromium.org%2F However flex can have two passes, one without a fixed block-size, and one with. If we can determine ahead of time that these would result in the same fragment we'll hit the cache, but the constraint space would be the one without the fixed block-size. In other words, if a flex-item remained the same size between the measure, and layout passes, a relative position grandchild would resolve its %-based offsets incorrectly. This patch introduces *some* logic from another patch current WIP: https://chromium-review.googlesource.com/c/chromium/src/+/2214608 which got reverted for unrelated reasons. Effectively if we have a %-based grandchild like this, we'll miss the cache, meaning that when we later read the constraint space fixed block-size, it'll resolve against the correct size. Bug: 1106074 Change-Id: I5003e4c0903bda408c45f8fb608ad4106c8f22b0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302231 Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Cr-Commit-Position: refs/heads/master@{#789058}

[FlexNG] Fix %-relpos for a grandchild of a flexbox.
When we resolve %-based offsets for a relative position child, if its parent is a flex-item we'll read the constraint-space used to generate this fragment, and resolve the %-based offset using that block-size. I.e. https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/renderer/core/layout/layout_box.cc;l=4521;drc=3f50460dcc4c426eb876cd0a1d2838eb35472e18?originalUrl=https:%2F%2Fcs.chromium.org%2F However flex can have two passes, one without a fixed block-size, and one with. If we can determine ahead of time that these would result in the same fragment we'll hit the cache, but the constraint space would be the one without the fixed block-size. In other words, if a flex-item remained the same size between the measure, and layout passes, a relative position grandchild would resolve its %-based offsets incorrectly. This patch introduces *some* logic from another patch current WIP: https://chromium-review.googlesource.com/c/chromium/src/+/2214608 which got reverted for unrelated reasons. Effectively if we have a %-based grandchild like this, we'll miss the cache, meaning that when we later read the constraint space fixed block-size, it'll resolve against the correct size. Bug: 1106074 Change-Id: I5003e4c0903bda408c45f8fb608ad4106c8f22b0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302231 Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Reviewed-by: David Grogan <dgrogan@chromium.org> Cr-Commit-Position: refs/heads/master@{#789058}
882a1102 · Ian Kilpatrick · Commit Bot · 7a63a3c7 · 882a1102 · 882a1102
Commit 882a1102 authored Jul 16, 2020 by Ian Kilpatrick Committed by Commit Bot Jul 16, 2020
2 changed files
--- a/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_container_fragment_builder.cc
@@ -128,8 +128,28 @@ void NGContainerFragmentBuilder::PropagateChildData(
  // We only need to report if inflow or floating elements depend on the
  // percentage resolution block-size. OOF-positioned children resolve their
  // percentages against the "final" size of their parent.
-  if (child.DependsOnPercentageBlockSize() && !child.IsOutOfFlowPositioned())
-    has_descendant_that_depends_on_percentage_block_size_ = true;
+  if (!has_descendant_that_depends_on_percentage_block_size_) {
+    if (child.DependsOnPercentageBlockSize() && !child.IsOutOfFlowPositioned())
+      has_descendant_that_depends_on_percentage_block_size_ = true;
+
+    // We may have a child which has the following style:
+    // <div style="position: relative; top: 50%;"></div>
+    // We need to mark this as depending on our %-block-size for the its offset
+    // to be correctly calculated. This is *slightly* too broad as it only
+    // depends on the available block-size, rather than the %-block-size.
+    const auto& child_style = child.Style();
+    if (child.IsCSSBox() && child_style.GetPosition() == EPosition::kRelative) {
+      if (IsHorizontalWritingMode(Style().GetWritingMode())) {
+        if (child_style.Top().IsPercentOrCalc() ||
+            child_style.Bottom().IsPercentOrCalc())
+          has_descendant_that_depends_on_percentage_block_size_ = true;
+      } else {
+        if (child_style.Left().IsPercentOrCalc() ||
+            child_style.Right().IsPercentOrCalc())
+          has_descendant_that_depends_on_percentage_block_size_ = true;
+      }
+    }
+  }

  // The |may_have_descendant_above_block_start_| flag is used to determine if
  // a fragment can be re-used when preceding floats are present. This is

--- a/third_party/blink/web_tests/external/wpt/css/css-flexbox/position-relative-percentage-top-002.html
+++ b/third_party/blink/web_tests/external/wpt/css/css-flexbox/position-relative-percentage-top-002.html
+<!DOCTYPE html>
+<meta name="assert" content="This ensures that a relative-positioned grandchild (of a flexbox) correctly resolves percentages against its stretched parent.">
+<link rel="help" href="https://crbug.com/1106074">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="width: 100px; height: 100px; background: red;"></div>
+<div style="display: flex; width: 100px;">
+  <div style="height: 100px;"></div>
+  <div style="width: 100px;">
+    <div style="background: green; height: 100px; position: relative; top: -100%;"></div>
+  </div>
+</div>