Merge r222220 - createMarkupInternal should protect its pointer to th…

…e Range's common ancestor https://bugs.webkit.org/show_bug.cgi?id=177033 <rdar://problem/34265390> Reviewed by Tim Horton. Source/WebCore: Adds basic safeguarding to codepaths hit while executing an outdent command. Test: editing/execCommand/outdent-with-media-query-listener-in-iframe.html * editing/IndentOutdentCommand.cpp: (WebCore::IndentOutdentCommand::outdentRegion): Avoid an infinite loop if endOfCurrentParagraph is a null position. * editing/markup.cpp: (WebCore::createMarkupInternal): Protect the raw pointer to the Range's common ancestor node. LayoutTests: Adds a test that removes the common ancestor node of a range in the middle of executing an outdent. * editing/execCommand/outdent-with-media-query-listener-in-iframe-expected.txt: Added. * editing/execCommand/outdent-with-media-query-listener-in-iframe.html: Added.
WebKit · Oct 16, 2017 · 5f520ae · 5f520ae
1 parent 7104353
commit 5f520ae
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 2 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2017-09-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        createMarkupInternal should protect its pointer to the Range's common ancestor
+        https://bugs.webkit.org/show_bug.cgi?id=177033
+        <rdar://problem/34265390>
+
+        Reviewed by Tim Horton.
+
+        Adds a test that removes the common ancestor node of a range in the middle of executing an outdent.
+
+        * editing/execCommand/outdent-with-media-query-listener-in-iframe-expected.txt: Added.
+        * editing/execCommand/outdent-with-media-query-listener-in-iframe.html: Added.
+
 2017-09-18  Emilio Cobos Álvarez  <emilio@crisal.io>
 
         Always update display: contents styles in RenderTreeUpdater.

diff --git a/LayoutTests/editing/execCommand/outdent-with-media-query-listener-in-iframe-expected.txt b/LayoutTests/editing/execCommand/outdent-with-media-query-listener-in-iframe-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/editing/execCommand/outdent-with-media-query-listener-in-iframe.html b/LayoutTests/editing/execCommand/outdent-with-media-query-listener-in-iframe.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<body>
+<blockquote>
+    <div>
+        <span id="span">
+            <i id="i1">a</i>
+            <i id="i2">b</i>
+        </span>
+    </div>
+    <div>1</div>
+</blockquote>
+</body>
+
+<script>
+let layoutCount = 0;
+
+function forceGarbageCollection() {
+    for (let i = 0; i < 100; i++)
+        new ArrayBuffer(0x100000);
+}
+
+function listener() {
+    if (layoutCount === 53)
+        document.body.insertAdjacentHTML("beforeend", "<input autofocus>");
+
+    if (layoutCount === 54) {
+        span.remove();
+        forceGarbageCollection();
+        return;
+    }
+
+    frame.contentWindow.matchMedia(`(max-width: ${layoutCount + 1}px)`).addListener(listener);
+    frame.width = layoutCount++;
+}
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+document.designMode = "on";
+document.execCommand("SelectAll");
+
+let frame = document.body.appendChild(document.createElement("iframe"));
+frame.contentWindow.matchMedia("(max-width: 100px)").addListener(listener);
+
+document.execCommand("Outdent");
+document.body.innerHTML = "<code style='color: green'>PASS</code>";
+</script>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2017-09-15  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        createMarkupInternal should protect its pointer to the Range's common ancestor
+        https://bugs.webkit.org/show_bug.cgi?id=177033
+        <rdar://problem/34265390>
+
+        Reviewed by Tim Horton.
+
+        Adds basic safeguarding to codepaths hit while executing an outdent command.
+
+        Test: editing/execCommand/outdent-with-media-query-listener-in-iframe.html
+
+        * editing/IndentOutdentCommand.cpp:
+        (WebCore::IndentOutdentCommand::outdentRegion):
+
+        Avoid an infinite loop if endOfCurrentParagraph is a null position.
+
+        * editing/markup.cpp:
+        (WebCore::createMarkupInternal):
+
+        Protect the raw pointer to the Range's common ancestor node.
+
 2017-09-18  Emilio Cobos Álvarez  <emilio@crisal.io>
 
         Always update display: contents styles in RenderTreeUpdater.

diff --git a/Source/WebCore/editing/IndentOutdentCommand.cpp b/Source/WebCore/editing/IndentOutdentCommand.cpp
@@ -225,6 +225,12 @@ void IndentOutdentCommand::outdentRegion(const VisiblePosition& startOfSelection
             endOfNextParagraph = endOfParagraph(endOfCurrentParagraph.next());
         }
         endOfCurrentParagraph = endOfNextParagraph;
+
+        if (endOfCurrentParagraph.isNull()) {
+            // If the end of the current paragraph is null, we'll end up looping infinitely, since the end of the next paragraph
+            // (and the paragraph after that, and so on) will always be null. To avoid this infinite loop, just bail.
+            break;
+        }
     }
 }
 

diff --git a/Source/WebCore/editing/markup.cpp b/Source/WebCore/editing/markup.cpp
@@ -580,13 +580,13 @@ static String createMarkupInternal(Document& document, const Range& range, Vecto
     bool collapsed = range.collapsed();
     if (collapsed)
         return emptyString();
-    Node* commonAncestor = range.commonAncestorContainer();
+    RefPtr<Node> commonAncestor = range.commonAncestorContainer();
     if (!commonAncestor)
         return emptyString();
 
     document.updateLayoutIgnorePendingStylesheets();
 
-    auto* body = enclosingElementWithTag(firstPositionInNode(commonAncestor), bodyTag);
+    auto* body = enclosingElementWithTag(firstPositionInNode(commonAncestor.get()), bodyTag);
     Element* fullySelectedRoot = nullptr;
     // FIXME: Do this for all fully selected blocks, not just the body.
     if (body && VisiblePosition(firstPositionInNode(body)) == VisiblePosition(range.startPosition())