Make RepaintRegionAccumulator hold a WeakPtr to its root RenderView
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Mar 2017 15:11:36 +0000 (15:11 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Mar 2017 15:11:36 +0000 (15:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168480
<rdar://problem/30566976>

Reviewed by Antti Koivisto.

Source/WebCore:

Implements two mitigations to prevent the symptoms of the bug from occurring (see the bugzilla for more details).

Test: editing/execCommand/show-modal-dialog-during-execCommand.html

* editing/EditorCommand.cpp:
(WebCore::Editor::Command::execute):

Do not allow edit commands to execute if the frame's document before and after layout differ (that is, edit commands
triggered by a certain document should not run on a different document).

* rendering/RenderView.cpp:
(WebCore::RenderView::RenderView):
(WebCore::RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator):

Turns RepaintRegionAccumulator's reference to its root RenderView into a WeakPtr to gracefully handle the case
where its RenderView is destroyed before RepaintRegionAccumulator's destructor gets a chance to flush the
RenderView's repaint regions.

* rendering/RenderView.h:

LayoutTests:

Introduces a new layout test. See WebCore ChangeLog for more details.

* TestExpectations:
* editing/execCommand/show-modal-dialog-during-execCommand-expected.txt: Added.
* editing/execCommand/show-modal-dialog-during-execCommand.html: Added.
* editing/execCommand/resources/self-closing-modal-dialog.html: Added.
* platform/mac-wk1/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/editing/execCommand/resources/self-closing-modal-dialog.html [new file with mode: 0644]
LayoutTests/editing/execCommand/show-modal-dialog-during-execCommand-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/show-modal-dialog-during-execCommand.html [new file with mode: 0644]
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/editing/EditorCommand.cpp
Source/WebCore/rendering/RenderView.cpp
Source/WebCore/rendering/RenderView.h

index c2ebb7b..4a8deb4 100644 (file)
@@ -1,3 +1,19 @@
+2017-03-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Make RepaintRegionAccumulator hold a WeakPtr to its root RenderView
+        https://bugs.webkit.org/show_bug.cgi?id=168480
+        <rdar://problem/30566976>
+
+        Reviewed by Antti Koivisto.
+
+        Introduces a new layout test. See WebCore ChangeLog for more details.
+
+        * TestExpectations:
+        * editing/execCommand/show-modal-dialog-during-execCommand-expected.txt: Added.
+        * editing/execCommand/show-modal-dialog-during-execCommand.html: Added.
+        * editing/execCommand/resources/self-closing-modal-dialog.html: Added.
+        * platform/mac-wk1/TestExpectations:
+
 2017-03-13  Youenn Fablet  <youenn@apple.com>
 
         Sync web-platform-tests up to revision a5b95cb31914507088a4eda16f7674bbc6f3313f
index 92a9c9e..5c620ed 100644 (file)
@@ -35,6 +35,9 @@ media/ios [ Skip ]
 media/controls/ipad [ Skip ]
 fast/text-autosizing [ Skip ]
 
+# window.showModalDialog is only tested in DumpRenderTree on Mac.
+editing/execCommand/show-modal-dialog-during-execCommand.html [ Skip ]
+
 fast/shadow-dom/touch-event-on-text-assigned-to-slot.html [ Skip ]
 
 fast/forms/attributed-strings.html [ Skip ]
diff --git a/LayoutTests/editing/execCommand/resources/self-closing-modal-dialog.html b/LayoutTests/editing/execCommand/resources/self-closing-modal-dialog.html
new file mode 100644 (file)
index 0000000..912fece
--- /dev/null
@@ -0,0 +1,9 @@
+<script>
+setTimeout(() => {
+    window.close();
+    if (window.testRunner) {
+        testRunner.notifyDone();
+        testRunner.abortModal();
+    }
+}, 1000);
+</script>
diff --git a/LayoutTests/editing/execCommand/show-modal-dialog-during-execCommand-expected.txt b/LayoutTests/editing/execCommand/show-modal-dialog-during-execCommand-expected.txt
new file mode 100644 (file)
index 0000000..8b13789
--- /dev/null
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/editing/execCommand/show-modal-dialog-during-execCommand.html b/LayoutTests/editing/execCommand/show-modal-dialog-during-execCommand.html
new file mode 100644 (file)
index 0000000..dc71f77
--- /dev/null
@@ -0,0 +1,46 @@
+<html>
+
+<head>
+<script>
+function sleep(ms) {
+    let start = new Date();
+    while (new Date() - start < ms);
+}
+
+(() => {
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.setCanOpenWindows();
+    }
+})();
+
+window.onclick = () => {
+    window.onclick = null;
+    document.designMode = "on";
+    document.execCommand("selectAll");
+
+    let frame = document.body.appendChild(document.createElement("iframe"));
+    let mediaList = frame.contentWindow.matchMedia("(max-width: 100px)");
+    mediaList.addListener(() => {
+        let link = document.createElement("a");
+        link.href = "https://www.webkit.org";
+        link.click();
+        showModalDialog("resources/self-closing-modal-dialog.html");
+    });
+
+    document.execCommand("delete");
+}
+</script>
+</head>
+
+<body>
+<script>
+if (window.eventSender) {
+    eventSender.mouseMoveTo(innerHeight / 2, innerWidth / 2);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+}
+</script>
+</body>
+
+</html>
index 232976b..a9bc59f 100644 (file)
@@ -5,6 +5,7 @@
 # Platform-specific tests. Skipped globally, then re-enabled here.
 #//////////////////////////////////////////////////////////////////////////////////////////
 
+editing/execCommand/execCommand-across-different-documents.html [ Pass ]
 fast/forms/attributed-strings.html [ Pass ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////
index dfe8109..983d654 100644 (file)
@@ -1,3 +1,31 @@
+2017-03-13  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Make RepaintRegionAccumulator hold a WeakPtr to its root RenderView
+        https://bugs.webkit.org/show_bug.cgi?id=168480
+        <rdar://problem/30566976>
+
+        Reviewed by Antti Koivisto.
+
+        Implements two mitigations to prevent the symptoms of the bug from occurring (see the bugzilla for more details).
+
+        Test: editing/execCommand/show-modal-dialog-during-execCommand.html
+
+        * editing/EditorCommand.cpp:
+        (WebCore::Editor::Command::execute):
+
+        Do not allow edit commands to execute if the frame's document before and after layout differ (that is, edit commands
+        triggered by a certain document should not run on a different document).
+
+        * rendering/RenderView.cpp:
+        (WebCore::RenderView::RenderView):
+        (WebCore::RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator):
+
+        Turns RepaintRegionAccumulator's reference to its root RenderView into a WeakPtr to gracefully handle the case
+        where its RenderView is destroyed before RepaintRegionAccumulator's destructor gets a chance to flush the
+        RenderView's repaint regions.
+
+        * rendering/RenderView.h:
+
 2017-03-14  Zan Dobersek  <zdobersek@igalia.com>
 
         [GLib] Use USE(GLIB) guards in WebCore/workers/
index 22164cc..74283b6 100644 (file)
@@ -1772,7 +1772,11 @@ bool Editor::Command::execute(const String& parameter, Event* triggeringEvent) c
         if (!allowExecutionWhenDisabled())
             return false;
     }
-    m_frame->document()->updateLayoutIgnorePendingStylesheets();
+    auto document = m_frame->document();
+    document->updateLayoutIgnorePendingStylesheets();
+    if (m_frame->document() != document)
+        return false;
+
     return m_command->execute(*m_frame, triggeringEvent, m_source, parameter);
 }
 
index 3b1070b..5846b65 100644 (file)
@@ -121,6 +121,7 @@ private:
 RenderView::RenderView(Document& document, RenderStyle&& style)
     : RenderBlockFlow(document, WTFMove(style))
     , m_frameView(*document.view())
+    , m_weakFactory(this)
     , m_lazyRepaintTimer(*this, &RenderView::lazyRepaintTimerFired)
 #if ENABLE(SERVICE_CONTROLS)
     , m_selectionRectGatherer(*this)
@@ -1423,10 +1424,15 @@ void RenderView::resumePausedImageAnimationsIfNeeded(IntRect visibleRect)
 }
 
 RenderView::RepaintRegionAccumulator::RepaintRegionAccumulator(RenderView* view)
-    : m_rootView(view ? view->document().topDocument().renderView() : nullptr)
 {
-    if (!m_rootView)
+    if (!view)
+        return;
+
+    auto* rootRenderView = view->document().topDocument().renderView();
+    if (!rootRenderView)
         return;
+
+    m_rootView = rootRenderView->createWeakPtr();
     m_wasAccumulatingRepaintRegion = !!m_rootView->m_accumulatedRepaintRegion;
     if (!m_wasAccumulatingRepaintRegion)
         m_rootView->m_accumulatedRepaintRegion = std::make_unique<Region>();
index 2e54355..3bacf7b 100644 (file)
@@ -239,10 +239,12 @@ public:
         ~RepaintRegionAccumulator();
 
     private:
-        RenderView* m_rootView;
+        WeakPtr<RenderView> m_rootView;
         bool m_wasAccumulatingRepaintRegion;
     };
 
+    WeakPtr<RenderView> createWeakPtr() { return m_weakFactory.createWeakPtr(); }
+
     void scheduleLazyRepaint(RenderBox&);
     void unscheduleLazyRepaint(RenderBox&);
 
@@ -338,6 +340,7 @@ private:
 private:
     FrameView& m_frameView;
 
+    WeakPtrFactory<RenderView> m_weakFactory;
     RenderObject* m_selectionUnsplitStart { nullptr };
     RenderObject* m_selectionUnsplitEnd { nullptr };
     std::optional<unsigned> m_selectionUnsplitStartPos;