createMarkupInternal should protect its pointer to the Range's common ancestor
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Sep 2017 20:25:47 +0000 (20:25 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Sep 2017 20:25:47 +0000 (20:25 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/outdent-with-media-query-listener-in-iframe-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/outdent-with-media-query-listener-in-iframe.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/IndentOutdentCommand.cpp
Source/WebCore/editing/markup.cpp

index 30b049a..ab71d8d 100644 (file)
@@ -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-19  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r222217 and r222214.
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
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..39f40d1
--- /dev/null
@@ -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>
index 6d2f0e9..590d1f1 100644 (file)
@@ -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-19  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r222217 and r222214.
index a45d96a..394208a 100644 (file)
@@ -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;
+        }
     }
 }
 
index bcc1ccb..a5bde9f 100644 (file)
@@ -584,13 +584,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())