Use outermost containing isolate when constructing bidi runs

<http://webkit.org/b/131107>
<rdar://problem/15690021>

Reviewed by Darin Adler.

Merged from Blink (patch by jww@chromium.org):
https://src.chromium.org/viewvc/blink?revision=157268&view=revision
http://crbug.com/279277

    Update containingIsolate to go back all the way to top
    isolate from current root, rather than stopping at the first
    isolate it finds. This works because the current root is
    always updated with each isolate run.

Source/WebCore:

Tests: fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html
       fast/text/international/unicode-bidi-isolate-nested-with-removes.html

* rendering/InlineIterator.h:
(WebCore::highestContainingIsolateWithinRoot):
* rendering/RenderBlockLineLayout.cpp:
(WebCore::constructBidiRunsForSegment):

LayoutTests:

* fast/text/international/unicode-bidi-isolate-nested-with-removes-expected.txt: Updated.
* fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent-expected.txt: Added.
* fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html: Added.
* fast/text/international/unicode-bidi-isolate-nested-with-removes.html: Updated.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index c45fff8c44b736c5bda64631d2d6fc1cb50ef9c1..7f2e808b09084683ebcf2557890ab1913d63b6ef 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,25 @@
+2014-04-02  David Kilzer  <ddkilzer@apple.com>
+
+        Use outermost containing isolate when constructing bidi runs
+        <http://webkit.org/b/131107>
+        <rdar://problem/15690021>
+
+        Reviewed by Darin Adler.
+
+        Merged from Blink (patch by jww@chromium.org):
+        https://src.chromium.org/viewvc/blink?revision=157268&view=revision
+        http://crbug.com/279277
+
+            Update containingIsolate to go back all the way to top
+            isolate from current root, rather than stopping at the first
+            isolate it finds. This works because the current root is
+            always updated with each isolate run.
+
+        * fast/text/international/unicode-bidi-isolate-nested-with-removes-expected.txt: Updated.
+        * fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent-expected.txt: Added.
+        * fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html: Added.
+        * fast/text/international/unicode-bidi-isolate-nested-with-removes.html: Updated.
+
 2014-04-02  Chris Fleizach  <cfleizach@apple.com>
 
         AX: Improve ARIA live region reliability by sending notifications when live regions are created/shown and hidden/destroyed

diff --git a/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-expected.txt b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-expected.txt
index ac917ddb7ca2868e432f5f03af83ee59cfc072f5..a576ffe4aeacc3149ea25b6309b53080f444914b 100644 (file)
--- a/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-expected.txt
+++ b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-expected.txt
@@ -1,4 +1 @@
- bar                      
-
-
 PASS did not crash

diff --git a/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent-expected.txt b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent-expected.txt
new file mode 100644 (file)
index 0000000..a576ffe
--- /dev/null
+++ b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent-expected.txt
@@ -0,0 +1 @@
+PASS did not crash

diff --git a/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html
new file mode 100644 (file)
index 0000000..0002083
--- /dev/null
+++ b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html
@@ -0,0 +1,35 @@
+<!doctype html>
+<!-- This tests for regression of https://crbug.com/279277 where non-adjacent, nested isolates caused a use-after-free if the elements were later removed. -->
+<script>
+window.onload = function() {
+  document.body.offsetTop;
+  b.lastChild.parentNode.removeChild(b.lastChild);
+  document.body.offsetTop;
+  a.nextSibling.parentNode.removeChild(a.nextSibling);
+  document.body.offsetTop;
+
+  document.write("PASS did not crash");
+}
+</script>
+
+<body>
+  <div id="a">foo</div><div>baz</div><div></div>
+  <div>
+    <output>
+      <span>
+        <output>bar</output>
+        <span id="b">
+          <span>
+            <div style="display:inline-block"></div>
+            <br><br>
+          </span>
+        </span>
+      </span>
+    </output>
+  </div>
+</body>
+
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>

diff --git a/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes.html b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes.html
index 8d0213489fb7815c16790c13f397ae38c292091a..23fb022a28b1ae738517c6e24169bd22274a5c1a 100644 (file)
--- a/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes.html
+++ b/LayoutTests/fast/text/international/unicode-bidi-isolate-nested-with-removes.html
@@ -1,19 +1,15 @@
+<!doctype html>
 <!-- This tests for regression of https://crbug.com/265838 where adjacent, nested isolates caused a use-after-free if the elements were later removed. -->
 <script>
-function remove(node)
-{
-    node.parentNode.removeChild(node);
-}
-
 window.onload = function()
 {
     document.body.offsetTop;
-    remove(b.lastChild);
+    b.lastChild.parentNode.removeChild(b.lastChild);
     document.body.offsetTop;
-    remove(a.firstChild);
+    a.firstChild.parentNode.removeChild(a.firstChild);
     document.body.offsetTop;
 
-    document.body.appendChild(document.createTextNode("PASS did not crash"));
+    document.write("PASS did not crash");
 }
 </script>
 

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 745e6f202ac995b463d9caf692a8a49a7a213145..6999d62a1b827a1a3b4eeca6bc8d76ae5deb021c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2014-04-02  David Kilzer  <ddkilzer@apple.com>
+
+        Use outermost containing isolate when constructing bidi runs
+        <http://webkit.org/b/131107>
+        <rdar://problem/15690021>
+
+        Reviewed by Darin Adler.
+
+        Merged from Blink (patch by jww@chromium.org):
+        https://src.chromium.org/viewvc/blink?revision=157268&view=revision
+        http://crbug.com/279277
+
+            Update containingIsolate to go back all the way to top
+            isolate from current root, rather than stopping at the first
+            isolate it finds. This works because the current root is
+            always updated with each isolate run.
+
+        Tests: fast/text/international/unicode-bidi-isolate-nested-with-removes-not-adjacent.html
+               fast/text/international/unicode-bidi-isolate-nested-with-removes.html
+
+        * rendering/InlineIterator.h:
+        (WebCore::highestContainingIsolateWithinRoot):
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::constructBidiRunsForSegment):
+
 2014-04-02  Chris Fleizach  <cfleizach@apple.com>
 
         AX: Improve ARIA live region reliability by sending notifications when live regions are created/shown and hidden/destroyed

diff --git a/Source/WebCore/rendering/InlineIterator.h b/Source/WebCore/rendering/InlineIterator.h
index 772029ad802a0197dc4e9f4e8c9c98731d36bbc8..07ea7963231686425a36b5875aab3dc07b26f683 100644 (file)
--- a/Source/WebCore/rendering/InlineIterator.h
+++ b/Source/WebCore/rendering/InlineIterator.h
@@ -448,14 +448,11 @@ static inline bool isIsolatedInline(RenderObject* object)
     return object->isRenderInline() && isIsolated(object->style().unicodeBidi());
 }
 
-static inline RenderObject* containingIsolate(RenderObject* object, RenderObject* root)
+static inline RenderObject* highestContainingIsolateWithinRoot(RenderObject* object, RenderObject* root)
 {
     ASSERT(object);
     RenderObject* containingIsolateObject = 0;
     while (object && object != root) {
-        if (containingIsolateObject && !isIsolatedInline(object))
-            break;
-
         if (isIsolatedInline(object))
             containingIsolateObject = object;
 

diff --git a/Source/WebCore/rendering/RenderBlockLineLayout.cpp b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
index e1af81fd4108a705a07d6448d4977ba2c9d1df30..86e7e62e1af1efc7d5934cafd2e253a9a1583ba1 100644 (file)
--- a/Source/WebCore/rendering/RenderBlockLineLayout.cpp
+++ b/Source/WebCore/rendering/RenderBlockLineLayout.cpp
@@ -876,7 +876,9 @@ static inline void constructBidiRunsForSegment(InlineBidiResolver& topResolver,
         // tree to see which parent inline is the isolate. We could change enterIsolate
         // to take a RenderObject and do this logic there, but that would be a layering
         // violation for BidiResolver (which knows nothing about RenderObject).
-        RenderInline* isolatedInline = toRenderInline(containingIsolate(&startObj, currentRoot));
+        RenderInline* isolatedInline = toRenderInline(highestContainingIsolateWithinRoot(&startObj, currentRoot));
+        ASSERT(isolatedInline);
+
         InlineBidiResolver isolatedResolver;
         EUnicodeBidi unicodeBidi = isolatedInline->style().unicodeBidi();
         TextDirection direction;