REGRESSION: Stack overflow in RenderBlockFlow::layoutBlock after increasing the font...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Jul 2017 05:49:50 +0000 (05:49 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Jul 2017 05:49:50 +0000 (05:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174144
<rdar://problem/32781038>

Reviewed by Simon Fraser.

Source/WebCore:

We set the start/end margin on the ruby renderer to support overhanging content. The margins ensure that
adjacent boxes on the line are placed properly respecting the overhanging content.
The line breaking algorithm also takes this value into account as it affects the line's available width.
We need to reset this value before laying out the lines, otherwise we might end up using this value on the line twice;
first as the renderer's margins (as the result of the previous layout) and second as the renderer's overhanging value.
Since this is not strictly part of the renderer's layout context (i.e. we set them during the line layout and not at
RenderRubyRun::layout) we can't rely on the ruby's layout logic to reset them.

Test: fast/ruby/ruby-overhang-margin-crash.html

* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlockFlow::layoutLineBoxes):

LayoutTests:

* fast/ruby/ruby-overhang-margin-crash-expected.txt: Added.
* fast/ruby/ruby-overhang-margin-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219190 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/ruby/ruby-overhang-margin-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/ruby/ruby-overhang-margin-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlockLineLayout.cpp

index 587d733..b80e06b 100644 (file)
@@ -1,3 +1,14 @@
+2017-07-05  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION: Stack overflow in RenderBlockFlow::layoutBlock after increasing the font size to max in some RTL vertical books.
+        https://bugs.webkit.org/show_bug.cgi?id=174144
+        <rdar://problem/32781038>
+
+        Reviewed by Simon Fraser.
+
+        * fast/ruby/ruby-overhang-margin-crash-expected.txt: Added.
+        * fast/ruby/ruby-overhang-margin-crash.html: Added.
+
 2017-07-05  Jonathan Bedard  <jbedard@apple.com>
 
         Move internal iOS 11 TestExpectations to OpenSource
diff --git a/LayoutTests/fast/ruby/ruby-overhang-margin-crash-expected.txt b/LayoutTests/fast/ruby/ruby-overhang-margin-crash-expected.txt
new file mode 100644 (file)
index 0000000..99c7764
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if no crash.
+i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i凝ぎよう i i i i i i i
diff --git a/LayoutTests/fast/ruby/ruby-overhang-margin-crash.html b/LayoutTests/fast/ruby/ruby-overhang-margin-crash.html
new file mode 100644 (file)
index 0000000..a9f73b5
--- /dev/null
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8">
+<title>This tests ruby's overhanging margin.</title>
+<style>
+.col {
+    column-count: 2;
+    widows: 2;
+    orphans: 1;
+    width: 200px;
+    font-size: 10px;
+}
+</style>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+PASS if no crash.
+<div class=col><div>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i<ruby><rb>凝</rb><rt>ぎよう</rt></ruby>
+i i i i i i i</div></div>
+</body>
+</html>
index 8b488b9..66dea96 100644 (file)
@@ -1,3 +1,24 @@
+2017-07-05  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION: Stack overflow in RenderBlockFlow::layoutBlock after increasing the font size to max in some RTL vertical books.
+        https://bugs.webkit.org/show_bug.cgi?id=174144
+        <rdar://problem/32781038>
+
+        Reviewed by Simon Fraser.
+
+        We set the start/end margin on the ruby renderer to support overhanging content. The margins ensure that
+        adjacent boxes on the line are placed properly respecting the overhanging content.
+        The line breaking algorithm also takes this value into account as it affects the line's available width.
+        We need to reset this value before laying out the lines, otherwise we might end up using this value on the line twice;
+        first as the renderer's margins (as the result of the previous layout) and second as the renderer's overhanging value.
+        Since this is not strictly part of the renderer's layout context (i.e. we set them during the line layout and not at
+        RenderRubyRun::layout) we can't rely on the ruby's layout logic to reset them.
+
+        Test: fast/ruby/ruby-overhang-margin-crash.html
+
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlockFlow::layoutLineBoxes):
+
 2017-07-05  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Upgrade GCC baseline
index 8dd65a5..e2e3507 100644 (file)
@@ -1728,6 +1728,12 @@ void RenderBlockFlow::layoutLineBoxes(bool relayoutChildren, LayoutUnit& repaint
                     layoutState.floatList().append(FloatWithRect::create(box));
                 else if (isFullLayout || box.needsLayout()) {
                     // Replaced element.
+                    if (isFullLayout && is<RenderRubyRun>(box)) {
+                        // FIXME: This resets the overhanging margins that we set during line layout (see computeInlineDirectionPositionsForSegment)
+                        // Find a more suitable place for this.
+                        setMarginStartForChild(box, 0);
+                        setMarginEndForChild(box, 0);
+                    }
                     box.dirtyLineBoxes(isFullLayout);
                     if (!o.isAnonymousInlineBlock()) {
                         if (isFullLayout)