Simple line layout: Removing adjacent trailing whitespace runs should not crash.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Feb 2017 21:17:28 +0000 (21:17 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Feb 2017 21:17:28 +0000 (21:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167803
<rdar://problem/30337368>

Reviewed by Antti Koivisto.

Source/WebCore:

In case of adjacent collapsed whitespace fragments, the length of these fragments (TextFragmentIterator::TextFragment)
do not necessarily equal the length of the final runs (SimpleLineLayout::Run).
This patch removes the dependency on the length and switches over to using the position information instead.

Test: fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html

* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::LineState::appendFragmentAndCreateRunIfNeeded):
(WebCore::SimpleLineLayout::LineState::removeTrailingWhitespace):

LayoutTests:

* fast/text/simple-line-layout-multiple-trailingwhitespace-crash-expected.txt: Added.
* fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/text/simple-line-layout-multiple-trailingwhitespace-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/SimpleLineLayout.cpp

index 3e62d8a..5df8842 100644 (file)
@@ -1,3 +1,14 @@
+2017-02-03  Zalan Bujtas  <zalan@apple.com>
+
+        Simple line layout: Removing adjacent trailing whitespace runs should not crash.
+        https://bugs.webkit.org/show_bug.cgi?id=167803
+        <rdar://problem/30337368>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/text/simple-line-layout-multiple-trailingwhitespace-crash-expected.txt: Added.
+        * fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html: Added.
+
 2017-02-03  Chris Dumez  <cdumez@apple.com>
 
         Fix bad assertion under HTMLTreeBuilder::processStartTagForInBody()
diff --git a/LayoutTests/fast/text/simple-line-layout-multiple-trailingwhitespace-crash-expected.txt b/LayoutTests/fast/text/simple-line-layout-multiple-trailingwhitespace-crash-expected.txt
new file mode 100644 (file)
index 0000000..1e3ad87
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if no crash or assert.
+F
diff --git a/LayoutTests/fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html b/LayoutTests/fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html
new file mode 100644 (file)
index 0000000..23f24ac
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>This tests that we don't crash on multiple trailing whitespace runs</title>
+<style>
+div {
+       font-size: 0;
+}
+</style>
+</head>
+<body>
+PASS if no crash or assert.
+<div>F                 <!---->
+</div>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+</script>
+</body>
+</html>
index 6498c45..88dcb3a 100644 (file)
@@ -1,3 +1,21 @@
+2017-02-03  Zalan Bujtas  <zalan@apple.com>
+
+        Simple line layout: Removing adjacent trailing whitespace runs should not crash.
+        https://bugs.webkit.org/show_bug.cgi?id=167803
+        <rdar://problem/30337368>
+
+        Reviewed by Antti Koivisto.
+
+        In case of adjacent collapsed whitespace fragments, the length of these fragments (TextFragmentIterator::TextFragment)
+        do not necessarily equal the length of the final runs (SimpleLineLayout::Run).
+        This patch removes the dependency on the length and switches over to using the position information instead.
+
+        Test: fast/text/simple-line-layout-multiple-trailingwhitespace-crash.html
+
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::LineState::appendFragmentAndCreateRunIfNeeded):
+        (WebCore::SimpleLineLayout::LineState::removeTrailingWhitespace):
+
 2017-02-03  Brent Fulgham  <bfulgham@apple.com>
 
         Correct memory leak in MediaConstraints
index 8f3e12d..b6257a2 100644 (file)
@@ -383,20 +383,23 @@ static float computeLineLeft(ETextAlign textAlign, float availableWidth, float c
     return 0;
 }
 
-static void revertRuns(Layout::RunVector& runs, unsigned length, float width)
+static void revertRuns(Layout::RunVector& runs, unsigned positionToRevertTo, float width)
 {
-    while (length) {
-        ASSERT(runs.size());
-        Run& lastRun = runs.last();
-        unsigned lastRunLength = lastRun.end - lastRun.start;
-        if (lastRunLength > length) {
+    while (runs.size()) {
+        auto& lastRun = runs.last();
+        if (lastRun.end <= positionToRevertTo)
+            break;
+        if (lastRun.start >= positionToRevertTo) {
+            // Revert this run completely.
+            width -= (lastRun.logicalRight - lastRun.logicalLeft);
+            runs.removeLast();
+        } else {
             lastRun.logicalRight -= width;
-            lastRun.end -= length;
+            width = 0;
+            lastRun.end = positionToRevertTo;
+            // Partial removal.
             break;
         }
-        length -= lastRunLength;
-        width -= (lastRun.logicalRight - lastRun.logicalLeft);
-        runs.removeLast();
     }
 }
 
@@ -517,7 +520,7 @@ public:
         }
         ASSERT(m_lastFragment.isValid());
         m_runsWidth -= m_uncompletedWidth;
-        revertRuns(runs, endPositionForCollapsedFragment(m_lastFragment) - endPositionForCollapsedFragment(m_lastCompleteFragment), m_uncompletedWidth);
+        revertRuns(runs, endPositionForCollapsedFragment(m_lastCompleteFragment), m_uncompletedWidth);
         m_uncompletedWidth = 0;
         ASSERT(m_lastCompleteFragment.isValid());
         return m_lastCompleteFragment;
@@ -527,8 +530,7 @@ public:
     {
         if (m_lastFragment.type() != TextFragmentIterator::TextFragment::Whitespace || m_lastFragment.end() == m_lastNonWhitespaceFragment.end())
             return;
-        unsigned trailingWhitespaceLength = endPositionForCollapsedFragment(m_lastFragment) - m_lastNonWhitespaceFragment.end();
-        revertRuns(runs, trailingWhitespaceLength, m_trailingWhitespaceWidth);
+        revertRuns(runs, m_lastNonWhitespaceFragment.end(), m_trailingWhitespaceWidth);
         m_runsWidth -= m_trailingWhitespaceWidth;
         m_lastFragment = m_lastNonWhitespaceFragment;
     }