Simple line layout: Web process spins endlessly below layoutSimpleLines.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 21:32:07 +0000 (21:32 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 21:32:07 +0000 (21:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144403
rdar://problem/20742783

Reviewed by Antti Koivisto.

When a text fragment overlaps multiple renderes and it does not fit the current line,
we revert the text fragment iterator position so that the overlapping content
gets processed again for the next line.
However, TextFragmentIterator::revertToFragment() was reverting too much and
we started processing old content all over again -> infinite loop.

This patch ensures that text fragment iterator is reverted to the right position.

Source/WebCore:

Test: fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html

* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::createLineRuns):
* rendering/SimpleLineLayoutTextFragmentIterator.cpp:
(WebCore::SimpleLineLayout::TextFragmentIterator::revertToEndOfFragment):
(WebCore::SimpleLineLayout::TextFragmentIterator::revertToFragment): Deleted.
* rendering/SimpleLineLayoutTextFragmentIterator.h:

LayoutTests:

* fast/text/simple-line-layout-wrapping-multiple-renderers-hang-expected.html: Added.
* fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/text/simple-line-layout-wrapping-multiple-renderers-hang-expected.html [new file with mode: 0644]
LayoutTests/fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/SimpleLineLayout.cpp
Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp
Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.h

index 33fdad2..b7eba5e 100644 (file)
@@ -1,3 +1,22 @@
+2015-04-29  Zalan Bujtas  <zalan@apple.com>
+
+        Simple line layout: Web process spins endlessly below layoutSimpleLines.
+        https://bugs.webkit.org/show_bug.cgi?id=144403
+        rdar://problem/20742783
+
+        Reviewed by Antti Koivisto.
+
+        When a text fragment overlaps multiple renderes and it does not fit the current line,
+        we revert the text fragment iterator position so that the overlapping content
+        gets processed again for the next line.
+        However, TextFragmentIterator::revertToFragment() was reverting too much and
+        we started processing old content all over again -> infinite loop.
+
+        This patch ensures that text fragment iterator is reverted to the right position.
+
+        * fast/text/simple-line-layout-wrapping-multiple-renderers-hang-expected.html: Added.
+        * fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html: Added.
+
 2015-04-29  Antti Koivisto  <antti@apple.com>
 
         Mark newly added http/tests/cache/main-resource-304-reload.html failing on Windows.
diff --git a/LayoutTests/fast/text/simple-line-layout-wrapping-multiple-renderers-hang-expected.html b/LayoutTests/fast/text/simple-line-layout-wrapping-multiple-renderers-hang-expected.html
new file mode 100644 (file)
index 0000000..6c80888
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that simple line layout manages text fragments when they overlap multiple renderers.</title>
+<style>
+pre {
+    width: 45px; 
+    word-wrap: break-word;
+    white-space: pre-wrap;
+}
+</style>
+<script>
+    if (window.internals)
+        internals.settings.setSimpleLineLayoutEnabled(false);
+</script>
+</head>
+<body>
+The test passes if it does not hang or crash.
+<pre id=container></pre>
+<script>
+    var container = document.getElementById("container");
+    container.appendChild(document.createTextNode("A foo"));                   
+    container.appendChild(document.createTextNode("bar foobar"));                      
+    document.body.offsetWidth;                 
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html b/LayoutTests/fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html
new file mode 100644 (file)
index 0000000..15c7647
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that simple line layout manages text fragments when they overlap multiple renderers.</title>
+<style>
+pre {
+    width: 45px; 
+    word-wrap: break-word;
+    white-space: pre-wrap;
+}
+</style>
+</head>
+<body>
+The test passes if it does not hang or crash.
+<pre id=container></pre>
+<script>
+    var container = document.getElementById("container");
+    container.appendChild(document.createTextNode("A foo"));                   
+    container.appendChild(document.createTextNode("bar foobar"));                      
+    document.body.offsetWidth;                 
+</script>
+</body>
+</html>
index 04a0a71..98bb257 100644 (file)
@@ -1,3 +1,28 @@
+2015-04-29  Zalan Bujtas  <zalan@apple.com>
+
+        Simple line layout: Web process spins endlessly below layoutSimpleLines.
+        https://bugs.webkit.org/show_bug.cgi?id=144403
+        rdar://problem/20742783
+
+        Reviewed by Antti Koivisto.
+
+        When a text fragment overlaps multiple renderes and it does not fit the current line,
+        we revert the text fragment iterator position so that the overlapping content
+        gets processed again for the next line.
+        However, TextFragmentIterator::revertToFragment() was reverting too much and
+        we started processing old content all over again -> infinite loop.
+
+        This patch ensures that text fragment iterator is reverted to the right position.
+
+        Test: fast/text/simple-line-layout-wrapping-multiple-renderers-hang.html
+
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::createLineRuns):
+        * rendering/SimpleLineLayoutTextFragmentIterator.cpp:
+        (WebCore::SimpleLineLayout::TextFragmentIterator::revertToEndOfFragment):
+        (WebCore::SimpleLineLayout::TextFragmentIterator::revertToFragment): Deleted.
+        * rendering/SimpleLineLayoutTextFragmentIterator.h:
+
 2015-04-29  Filip Pizlo  <fpizlo@apple.com>
 
         JSTypeInfo should have an inline type flag to indicate of getCallData() has been overridden
index 8f6ba98..5e2ed30 100644 (file)
@@ -567,8 +567,8 @@ static bool createLineRuns(LineState& line, const LineState& previousLine, Layou
             // Non-breakable non-whitespace fragment when there's already content on the line. Push it to the next line.
             if (line.lastFragment().overlapsToNextRenderer()) {
                 // Check if this fragment is a continuation of a previous segment. In such cases, we need to remove them all.
-                const auto& currentFragment = line.revertToLastCompleteFragment(runs);
-                textFragmentIterator.revertToFragment(currentFragment);
+                const auto& lastCompleteFragment = line.revertToLastCompleteFragment(runs);
+                textFragmentIterator.revertToEndOfFragment(lastCompleteFragment);
                 break;
             }
             line.setOverflowedFragment(fragment);
index e004da7..7af785b 100644 (file)
@@ -98,15 +98,14 @@ TextFragmentIterator::TextFragment TextFragmentIterator::findNextTextFragment(fl
     return TextFragment(startPosition, endPosition, width, TextFragment::NonWhitespace, endPosition == segmentEndPosition, overlappingFragment, false, false, m_style.breakWordOnOverflow);
 }
 
-void TextFragmentIterator::revertToFragment(const TextFragment& fragment)
+void TextFragmentIterator::revertToEndOfFragment(const TextFragment& fragment)
 {
     ASSERT(m_position >= fragment.end());
-    // Revert segment first.
-    while (m_currentSegment->start > fragment.start())
+    while (m_currentSegment->start > fragment.end())
         --m_currentSegment;
     // TODO: It reverts to the last fragment on the same position, but that's ok for now as we don't need to
     // differentiate multiple renderers on the same position.
-    m_position = fragment.start();
+    m_position = fragment.end();
     m_atEndOfSegment = false;
 }
 
index a9cffb6..72cbe85 100644 (file)
@@ -95,7 +95,7 @@ public:
         bool m_isBreakable { false };
     };
     TextFragment nextTextFragment(float xPosition = 0);
-    void revertToFragment(const TextFragment&);
+    void revertToEndOfFragment(const TextFragment&);
     float textWidth(unsigned startPosition, unsigned endPosition, float xPosition) const;
 
     struct Style {