[LayoutNG] Avoid indefinite CB inline-size when entering legacy child.

The constraint space has several resolution sizes, depending on what we're to resolve. There's "available size", "percentage resolution size" (and "replaced percentage resolution size"), and even "percentage resolution inline-size for parent writing mode". The latter is used when resolving percentage based margins and padding. This works when staying within LayoutNG, but when we need to enter legacy layout for some child, we convert all those values into something that legacy layout understands, and dumb down the number of values to basically 3 (plus two optional values); see BoxLayoutExtraInput. Avoid setting an indefinite containing block inline-size, since we use this value to resolve percentage padding and margins. The test included only *crashed* without this fix (it rendered correctly if DCHECKs were bypassed), but I figured it would be useful to have a test for correct rendering in this case as well. Bug: 966795 Change-Id: Ia2a2ede55a257b41dc38236a8bb0ff689e75a310 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1631370 Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#663800}

[LayoutNG] Avoid indefinite CB inline-size when entering legacy child.
The constraint space has several resolution sizes, depending on what we're to resolve. There's "available size", "percentage resolution size" (and "replaced percentage resolution size"), and even "percentage resolution inline-size for parent writing mode". The latter is used when resolving percentage based margins and padding. This works when staying within LayoutNG, but when we need to enter legacy layout for some child, we convert all those values into something that legacy layout understands, and dumb down the number of values to basically 3 (plus two optional values); see BoxLayoutExtraInput. Avoid setting an indefinite containing block inline-size, since we use this value to resolve percentage padding and margins. The test included only *crashed* without this fix (it rendered correctly if DCHECKs were bypassed), but I figured it would be useful to have a test for correct rendering in this case as well. Bug: 966795 Change-Id: Ia2a2ede55a257b41dc38236a8bb0ff689e75a310 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1631370 Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#663800}
5215c2a0 · Morten Stenshorne · Commit Bot · 08f6c977 · 5215c2a0 · 5215c2a0
Commit 5215c2a0 authored May 28, 2019 by Morten Stenshorne Committed by Commit Bot May 28, 2019
2 changed files
--- a/third_party/blink/renderer/core/layout/ng/ng_block_node.cc
+++ b/third_party/blink/renderer/core/layout/ng/ng_block_node.cc
@@ -981,8 +981,24 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::RunLegacyLayout(
        // The sizes should be in the containing block writing mode.
        std::swap(input.containing_block_content_block_size,
                  input.containing_block_content_inline_size);
+        // We cannot lay out without a definite containing block inline-size. We
+        // end up here if we're performing a measure pass (as part of resolving
+        // the intrinsic min/max inline-size of some ancestor, for instance).
+        // Legacy layout has a tendency of clamping negative sizes to 0 anyway,
+        // but this is missing when it comes to resolving percentage-based
+        // padding, for instance.
+        if (input.containing_block_content_inline_size == kIndefiniteSize) {
+          DCHECK(constraint_space.IsIntermediateLayout());
+          input.containing_block_content_inline_size = LayoutUnit();
+        }
      }
    }
+    // We need a definite containing block inline-size, or we'd be unable to
+    // resolve percentages.
+    DCHECK_GE(input.containing_block_content_inline_size, LayoutUnit());
    input.available_inline_size = constraint_space.AvailableSize().inline_size;
    if (constraint_space.IsFixedSizeInline())

--- a/third_party/blink/web_tests/external/wpt/css/css-writing-modes/percentage-padding-in-shrink-to-fit.html
+++ b/third_party/blink/web_tests/external/wpt/css/css-writing-modes/percentage-padding-in-shrink-to-fit.html
+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-writing-modes-3/">
+<link rel="help" href="https://www.w3.org/TR/CSS22/box.html#propdef-padding-top">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=966795">
+<link rel="match" href="../reference/nothing.html">
+<meta name="assert" content="Percentage-padding is always resolved against the inline-size of the containing block, even if it's about block padding.">
+<p>There should be nothing below.</p>
+<div style="float:left;">
+  <div style="display:flex; writing-mode:vertical-rl; padding:1000%; background:red;"></div>
+</div>