[LayoutNG] Scrollbar size comparasance fix

Also added a DCHECK to make sure that we never set NeedsLayout flags while in intermediate layout. Bug: 967361 Change-Id: I078d3884025c0d659cf7a25df694e082d6dd3f1c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633031 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/master@{#664596}

[LayoutNG] Scrollbar size comparasance fix
Also added a DCHECK to make sure that we never set NeedsLayout flags while in intermediate layout. Bug: 967361 Change-Id: I078d3884025c0d659cf7a25df694e082d6dd3f1c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633031 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/master@{#664596}
6f04b5e5 · Aleks Totic · Commit Bot · d44f4549 · 6f04b5e5 · 6f04b5e5
Commit 6f04b5e5 authored May 30, 2019 by Aleks Totic Committed by Commit Bot May 30, 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
@@ -262,14 +262,17 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::Layout(
 #endif
  }

+  // Fragment geometry scrollbars are potentially size constrained, and cannot
+  // be used for comparison with their after layout size.
+  NGBoxStrut before_layout_scrollbars =
+      ComputeScrollbars(constraint_space, *this);
+
  if (!layout_result)
    layout_result = LayoutWithAlgorithm(params);

  FinishLayout(block_flow, constraint_space, break_token, layout_result);

-  NGBoxStrut after_layout_scrollbars =
-      ComputeScrollbars(constraint_space, *this);
-  if (fragment_geometry->scrollbar != after_layout_scrollbars) {
+  if (before_layout_scrollbars != ComputeScrollbars(constraint_space, *this)) {
    // If our scrollbars have changed, we need to relayout because either:
    // - Our size has changed (if shrinking to fit), or
    // - Space available to our children has changed.
@@ -281,6 +284,9 @@ scoped_refptr<const NGLayoutResult> NGBlockNode::Layout(
    // without that check.
    PaintLayerScrollableArea::FreezeScrollbarsScope freeze_scrollbars;

+    // Must not call SetNeedsLayout in intermediate layout. If we do,
+    // the NeedsLayout flag might not be cleared. crbug.com/967361
+    DCHECK(!constraint_space.IsIntermediateLayout() || box_->NeedsLayout());
    // Scrollbar changes are hard to detect. Make sure everyone gets the
    // message.
    box_->SetNeedsLayout(layout_invalidation_reason::kScrollbarChanged,

--- a/third_party/blink/web_tests/external/wpt/css/css-box/box-chrome-crash-001.html
+++ b/third_party/blink/web_tests/external/wpt/css/css-box/box-chrome-crash-001.html
+<!DOCTYPE html>
+<title>CSS Box: chrome crash</title>
+<link rel="author" href="mailto:atotic@google.com">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<link rel="help" href="https://crbug.com/967361">
+<meta name="assert" content="Chrome does not crash on narrow div with scrollbars.">
+<style>
+body  {
+  writing-mode: vertical-lr;
+}
+#target {
+  overflow-y: scroll;
+  max-height: 5px;
+  background: gray;
+}
+</style>
+<!--  -->
+<div>
+  <span>
+    <div id="container">
+      <div id="target">anon</div>
+    </div>
+  </span>
+</div>
+
+<script>
+test(() => {
+}, 'test passes if it does not crash');
+</script>