Title: [256859] trunk
Revision
256859
Author
wenson_hs...@apple.com
Date
2020-02-18 15:15:19 -0800 (Tue, 18 Feb 2020)

Log Message

[macOS] Web process may crash under ServicesOverlayController::buildPotentialHighlightsIfNeeded
https://bugs.webkit.org/show_bug.cgi?id=207899
<rdar://problem/55658207>

Reviewed by Tim Horton and Simon Fraser.

Source/WebCore:

Mitigates a null pointer crash in ServicesOverlayController::buildPotentialHighlightsIfNeeded(), wherein the
focused frame may not have a FrameView when the ServicesOverlayController's selection invalidation timer fires.
This is possible if, while being focused, the newly focused subframe is unparented and reparented, which causes
it to momentarily have a null view. During this time, if a selection change had occurred earlier in the runloop,
it will schedule the page overlay controller invalidation timer, which will fire and discover that the currently
focused frame no longer has a FrameView.

Test: editing/selection/selection-change-in-disconnected-frame-crash.html

* page/mac/ServicesOverlayController.mm:
(WebCore::ServicesOverlayController::buildSelectionHighlight):

Source/WebKit:

Add another missing null check on iOS, for the case where FrameView is null.

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformEditorState const):

Tools:

Make it possible to run tests on macOS with services controls enabled, via a new TestOptions flag.

* WebKitTestRunner/TestController.cpp:
(WTR::updateTestOptionsFromTestHeader):
* WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::hasSameInitializationOptions const):
* WebKitTestRunner/cocoa/TestControllerCocoa.mm:
(WTR::TestController::platformCreateWebView):

LayoutTests:

Add a new layout test to verify that we don't crash under this circumstance.

* editing/selection/selection-change-in-disconnected-frame-crash-expected.txt: Added.
* editing/selection/selection-change-in-disconnected-frame-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (256858 => 256859)


--- trunk/LayoutTests/ChangeLog	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/LayoutTests/ChangeLog	2020-02-18 23:15:19 UTC (rev 256859)
@@ -1,3 +1,16 @@
+2020-02-18  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [macOS] Web process may crash under ServicesOverlayController::buildPotentialHighlightsIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=207899
+        <rdar://problem/55658207>
+
+        Reviewed by Tim Horton and Simon Fraser.
+
+        Add a new layout test to verify that we don't crash under this circumstance.
+
+        * editing/selection/selection-change-in-disconnected-frame-crash-expected.txt: Added.
+        * editing/selection/selection-change-in-disconnected-frame-crash.html: Added.
+
 2020-02-18  Chris Dumez  <cdu...@apple.com>
 
         [WK1] Flaky Test: http/tests/cookies/document-cookie-during-iframe-parsing.html

Added: trunk/LayoutTests/editing/selection/selection-change-in-disconnected-frame-crash-expected.txt (0 => 256859)


--- trunk/LayoutTests/editing/selection/selection-change-in-disconnected-frame-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/selection-change-in-disconnected-frame-crash-expected.txt	2020-02-18 23:15:19 UTC (rev 256859)
@@ -0,0 +1,3 @@
+This test passes if it does not crash.
+
+ 

Added: trunk/LayoutTests/editing/selection/selection-change-in-disconnected-frame-crash.html (0 => 256859)


--- trunk/LayoutTests/editing/selection/selection-change-in-disconnected-frame-crash.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/selection-change-in-disconnected-frame-crash.html	2020-02-18 23:15:19 UTC (rev 256859)
@@ -0,0 +1,23 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ enableServiceControls=true ] -->
+<html>
+<head>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    addEventListener("load", () => {
+        const frame = document.querySelector("iframe");
+        const frameSet = document.createElement("frameset");
+        const frameDocument = frame.contentDocument;
+
+        frameDocument.getSelection().selectAllChildren(frameDocument.body);
+        frameSet._onblur_ = () => document.body.appendChild(frame);
+        frame.focus();
+    });
+</script>
+</head>
+<body>
+    <p>This test passes if it does not crash.</p>
+    <iframe srcdoc="<body>Hello</body>"></iframe>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (256858 => 256859)


--- trunk/Source/WebCore/ChangeLog	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Source/WebCore/ChangeLog	2020-02-18 23:15:19 UTC (rev 256859)
@@ -1,3 +1,23 @@
+2020-02-18  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [macOS] Web process may crash under ServicesOverlayController::buildPotentialHighlightsIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=207899
+        <rdar://problem/55658207>
+
+        Reviewed by Tim Horton and Simon Fraser.
+
+        Mitigates a null pointer crash in ServicesOverlayController::buildPotentialHighlightsIfNeeded(), wherein the
+        focused frame may not have a FrameView when the ServicesOverlayController's selection invalidation timer fires.
+        This is possible if, while being focused, the newly focused subframe is unparented and reparented, which causes
+        it to momentarily have a null view. During this time, if a selection change had occurred earlier in the runloop,
+        it will schedule the page overlay controller invalidation timer, which will fire and discover that the currently
+        focused frame no longer has a FrameView.
+
+        Test: editing/selection/selection-change-in-disconnected-frame-crash.html
+
+        * page/mac/ServicesOverlayController.mm:
+        (WebCore::ServicesOverlayController::buildSelectionHighlight):
+
 2020-02-18  Peng Liu  <peng.l...@apple.com>
 
         MediaSource.isTypeSupported() says "video/mp4;codecs=\"avc3.42C015\"" is not supported, but it is

Modified: trunk/Source/WebCore/page/mac/ServicesOverlayController.mm (256858 => 256859)


--- trunk/Source/WebCore/page/mac/ServicesOverlayController.mm	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Source/WebCore/page/mac/ServicesOverlayController.mm	2020-02-18 23:15:19 UTC (rev 256859)
@@ -544,7 +544,9 @@
         if (!mainFrameView)
             return;
 
-        FrameView* viewForRange = selectionRange->ownerDocument().view();
+        RefPtr<FrameView> viewForRange = selectionRange->ownerDocument().view();
+        if (!viewForRange)
+            return;
 
         for (auto& rect : m_currentSelectionRects) {
             IntRect currentRect = snappedIntRect(rect);

Modified: trunk/Source/WebKit/ChangeLog (256858 => 256859)


--- trunk/Source/WebKit/ChangeLog	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Source/WebKit/ChangeLog	2020-02-18 23:15:19 UTC (rev 256859)
@@ -1,3 +1,16 @@
+2020-02-18  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [macOS] Web process may crash under ServicesOverlayController::buildPotentialHighlightsIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=207899
+        <rdar://problem/55658207>
+
+        Reviewed by Tim Horton and Simon Fraser.
+
+        Add another missing null check on iOS, for the case where FrameView is null.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::platformEditorState const):
+
 2020-02-18  Youenn Fablet  <you...@apple.com>
 
         NetworkDataTask should not expect its session wrapper to be always live

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (256858 => 256859)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2020-02-18 23:15:19 UTC (rev 256859)
@@ -208,6 +208,11 @@
 void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePostLayoutDataHint shouldIncludePostLayoutData) const
 {
     FrameView* view = frame.view();
+    if (!view) {
+        result.isMissingPostLayoutData = true;
+        return;
+    }
+
     if (frame.editor().hasComposition()) {
         RefPtr<Range> compositionRange = frame.editor().compositionRange();
         Vector<WebCore::SelectionRect> compositionRects;
@@ -227,7 +232,7 @@
     // to avoid the need to force a synchronous layout here to compute these entries. If we
     // have a composition or are using a hardware keyboard then we send the full editor state
     // immediately so that the UIProcess can update UI, including the position of the caret.
-    bool needsLayout = !frame.view() || frame.view()->needsLayout();
+    bool needsLayout = view->needsLayout();
     bool requiresPostLayoutData = frame.editor().hasComposition();
 #if !PLATFORM(MACCATALYST)
     requiresPostLayoutData |= m_keyboardIsAttached;

Modified: trunk/Tools/ChangeLog (256858 => 256859)


--- trunk/Tools/ChangeLog	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Tools/ChangeLog	2020-02-18 23:15:19 UTC (rev 256859)
@@ -1,3 +1,20 @@
+2020-02-18  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [macOS] Web process may crash under ServicesOverlayController::buildPotentialHighlightsIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=207899
+        <rdar://problem/55658207>
+
+        Reviewed by Tim Horton and Simon Fraser.
+
+        Make it possible to run tests on macOS with services controls enabled, via a new TestOptions flag.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::updateTestOptionsFromTestHeader):
+        * WebKitTestRunner/TestOptions.h:
+        (WTR::TestOptions::hasSameInitializationOptions const):
+        * WebKitTestRunner/cocoa/TestControllerCocoa.mm:
+        (WTR::TestController::platformCreateWebView):
+
 2020-02-18  Matt Lewis  <jlew...@apple.com>
 
         Stub repositories fail to upload some results due to missing head svn revision

Modified: trunk/Tools/WebKitTestRunner/TestController.cpp (256858 => 256859)


--- trunk/Tools/WebKitTestRunner/TestController.cpp	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Tools/WebKitTestRunner/TestController.cpp	2020-02-18 23:15:19 UTC (rev 256859)
@@ -1458,6 +1458,8 @@
             testOptions.shouldIgnoreMetaViewport = parseBooleanTestHeaderValue(value);
         else if (key == "spellCheckingDots")
             testOptions.shouldShowSpellCheckingDots = parseBooleanTestHeaderValue(value);
+        else if (key == "enableServiceControls")
+            testOptions.enableServiceControls = parseBooleanTestHeaderValue(value);
         else if (key == "enableEditableImages")
             testOptions.enableEditableImages = parseBooleanTestHeaderValue(value);
         else if (key == "editable")

Modified: trunk/Tools/WebKitTestRunner/TestOptions.h (256858 => 256859)


--- trunk/Tools/WebKitTestRunner/TestOptions.h	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Tools/WebKitTestRunner/TestOptions.h	2020-02-18 23:15:19 UTC (rev 256859)
@@ -90,6 +90,7 @@
     bool checkForWorldLeaks { false };
     bool shouldIgnoreMetaViewport { false };
     bool shouldShowSpellCheckingDots { false };
+    bool enableServiceControls { false };
     bool enableEditableImages { false };
     bool editable { false };
     bool enableUndoManagerAPI { false };
@@ -152,6 +153,7 @@
             || runSingly != options.runSingly
             || checkForWorldLeaks != options.checkForWorldLeaks
             || shouldShowSpellCheckingDots != options.shouldShowSpellCheckingDots
+            || enableServiceControls != options.enableServiceControls
             || shouldIgnoreMetaViewport != options.shouldIgnoreMetaViewport
             || enableEditableImages != options.enableEditableImages
             || editable != options.editable

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm (256858 => 256859)


--- trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm	2020-02-18 23:11:58 UTC (rev 256858)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm	2020-02-18 23:15:19 UTC (rev 256859)
@@ -136,8 +136,8 @@
         [copiedConfiguration setIgnoresViewportScaleLimits:YES];
     if (options.useCharacterSelectionGranularity)
         [copiedConfiguration setSelectionGranularity:WKSelectionGranularityCharacter];
-    if (options.useCharacterSelectionGranularity)
-        [copiedConfiguration setSelectionGranularity:WKSelectionGranularityCharacter];
+#else
+    [copiedConfiguration _setServiceControlsEnabled:options.enableServiceControls];
 #endif
 
     if (options.enableAttachmentElement)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to