Chromium: Hang parsing bidi control chars on Mac OS X 10.6
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Feb 2013 00:51:18 +0000 (00:51 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Feb 2013 00:51:18 +0000 (00:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108877

This was broken a while ago by:
    https://bugs.webkit.org/show_bug.cgi?id=83045

On 10.6, CoreText will not produce any runs covering the
Unicode BiDi RTL mark control char, which causes an infinite
loop in ComplexTextController::indexOfCurrentRun() due to no
run covering the character at offset 0.

This patch fixes that issue by finding the earliest run
explicitly via the minimum stringBegin() index instead of
relying on a run existing that covers offset 0.

Fixes hang on many BiDi wikipedia pages on Chromium/Mac10.6.
Chromium bug: http://crbug.com/167844

Source/WebCore:

New test in the same style as the harfbuzz-buffer-overrun.html
test (in the same folder).

Patch by Alexei Svitkine <asvitkine@chromium.org> on 2013-02-07
Reviewed by Eric Seidel.

Test: fast/text/international/rtl-mark.html

* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::indexOfCurrentRun):

LayoutTests:

New test in the same style as harfbuzz-buffer-overrun.html
in the same folder.

Patch by Alexei Svitkine <asvitkine@chromium.org> on 2013-02-07
Reviewed by Eric Seidel.

* fast/text/international/rtl-mark-expected.txt: Added.
* fast/text/international/rtl-mark.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/text/international/rtl-mark-expected.txt [new file with mode: 0644]
LayoutTests/fast/text/international/rtl-mark.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/mac/ComplexTextController.cpp

index 576d9a529d9a521ac176a13d30682d37875eb08e..594f094c4d0c2905bdc5c3e48b30e26d5f43e251 100644 (file)
@@ -1,3 +1,31 @@
+2013-02-07  Alexei Svitkine  <asvitkine@chromium.org>
+
+        Chromium: Hang parsing bidi control chars on Mac OS X 10.6
+        https://bugs.webkit.org/show_bug.cgi?id=108877
+
+        This was broken a while ago by:
+            https://bugs.webkit.org/show_bug.cgi?id=83045
+
+        On 10.6, CoreText will not produce any runs covering the
+        Unicode BiDi RTL mark control char, which causes an infinite
+        loop in ComplexTextController::indexOfCurrentRun() due to no
+        run covering the character at offset 0.
+
+        This patch fixes that issue by finding the earliest run
+        explicitly via the minimum stringBegin() index instead of
+        relying on a run existing that covers offset 0.
+
+        Fixes hang on many BiDi wikipedia pages on Chromium/Mac10.6.
+        Chromium bug: http://crbug.com/167844
+
+        New test in the same style as harfbuzz-buffer-overrun.html
+        in the same folder.
+
+        Reviewed by Eric Seidel.
+
+        * fast/text/international/rtl-mark-expected.txt: Added.
+        * fast/text/international/rtl-mark.html: Added.
+
 2013-02-07  Kentaro Hara  <haraken@chromium.org>
 
         Implement FocusEvent constructor
diff --git a/LayoutTests/fast/text/international/rtl-mark-expected.txt b/LayoutTests/fast/text/international/rtl-mark-expected.txt
new file mode 100644 (file)
index 0000000..4b7936c
--- /dev/null
@@ -0,0 +1 @@
+PASS: does not hang
diff --git a/LayoutTests/fast/text/international/rtl-mark.html b/LayoutTests/fast/text/international/rtl-mark.html
new file mode 100644 (file)
index 0000000..ac5ea04
--- /dev/null
@@ -0,0 +1,14 @@
+<html>
+<body>
+<p>&#x200F;&#x628;&#x62D;&#x631;&#x6CC;&#x646;&#xA;</p>
+<script>
+// Force layout.
+document.body.offsetTop;
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+document.body.innerHTML = "PASS: does not hang";
+</script>
+</body>
+</html>
index eff2bbea9594913943f881362d02394f8c86bf61..8e1a09377226f546cea5b9a5cfbaab5859477948 100644 (file)
@@ -1,3 +1,33 @@
+2013-02-07  Alexei Svitkine  <asvitkine@chromium.org>
+
+        Chromium: Hang parsing bidi control chars on Mac OS X 10.6
+        https://bugs.webkit.org/show_bug.cgi?id=108877
+
+        This was broken a while ago by:
+            https://bugs.webkit.org/show_bug.cgi?id=83045
+
+        On 10.6, CoreText will not produce any runs covering the
+        Unicode BiDi RTL mark control char, which causes an infinite
+        loop in ComplexTextController::indexOfCurrentRun() due to no
+        run covering the character at offset 0.
+
+        This patch fixes that issue by finding the earliest run
+        explicitly via the minimum stringBegin() index instead of
+        relying on a run existing that covers offset 0.
+
+        Fixes hang on many BiDi wikipedia pages on Chromium/Mac10.6.
+        Chromium bug: http://crbug.com/167844
+
+        New test in the same style as the harfbuzz-buffer-overrun.html
+        test (in the same folder).
+
+        Reviewed by Eric Seidel.
+
+        Test: fast/text/international/rtl-mark.html
+
+        * platform/graphics/mac/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::indexOfCurrentRun):
+
 2013-02-07  Kentaro Hara  <haraken@chromium.org>
 
         Implement FocusEvent constructor
index 87bd52dc07b44c835d9ecc9e9566a824e747ad4d..00f4e624a2e1ab5174786838bc346628dccd0955 100644 (file)
@@ -423,8 +423,21 @@ unsigned ComplexTextController::indexOfCurrentRun(unsigned& leftmostGlyph)
         return m_currentRun;
     }
 
+    if (m_runIndices.isEmpty()) {
+        unsigned firstRun = 0;
+        unsigned firstRunOffset = stringBegin(*m_complexTextRuns[0]);
+        for (unsigned i = 1; i < runCount; ++i) {
+            unsigned offset = stringBegin(*m_complexTextRuns[i]);
+            if (offset < firstRunOffset) {
+                firstRun = i;
+                firstRunOffset = offset;
+            }
+        }
+        m_runIndices.uncheckedAppend(firstRun);
+    }
+
     while (m_runIndices.size() <= m_currentRun) {
-        unsigned offset = m_runIndices.isEmpty() ? 0 : stringEnd(*m_complexTextRuns[m_runIndices.last()]);
+        unsigned offset = stringEnd(*m_complexTextRuns[m_runIndices.last()]);
 
         for (unsigned i = 0; i < runCount; ++i) {
             if (offset == stringBegin(*m_complexTextRuns[i])) {