- 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)