- Revision
- 280762
- Author
- [email protected]
- Date
- 2021-08-07 15:47:09 -0700 (Sat, 07 Aug 2021)
Log Message
[macOS] Web process crashes when detaching Document with uncommitted marked text
https://bugs.webkit.org/show_bug.cgi?id=228841
rdar://79960890
Reviewed by Ryosuke Niwa.
Source/WebCore:
In the case where the document is in the process of being detached (underneath `willBeRemovedFromFrame()`), if
there is currently uncommitted marked text in the document, we will attempt to cancel the IME composition in the
process of clearing out the selection. On macOS, this calls into `Editor::cancelComposition()` which
subsequently triggers layout under various call stacks (DOM mutations, text event dispatch, and when scrolling
to reveal the selection); this triggers a security release assertion inside `Document::updateLayout()`.
To mitigate this, we avoid calling into this codepath if the Document no longer has a living render tree (i.e.,
the render tree has either been destroyed, is being destroyed, or has not been created yet).
Test: editing/inserting/remove-frame-with-marked-text.html
* editing/mac/EditorMac.mm:
(WebCore::Editor::selectionWillChange):
Source/WebKit:
Deploy a similar fix on iOS, to avoid any attempts to compute editor state due to discarding uncommitted marked
text during Document teardown. This is required in order to avoid the same security assertion when running the
new layout test on iOS.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::sendEditorStateUpdate):
Tools:
Make a small adjustment to DumpRenderTree, such that TextInputController targets the selected frame (or the main
frame, if there is no DOM selection). This behavior matches that of WebKitTestRunner, and allows layout tests
that use TextInputController to simulate setting marked text inside subframes.
* DumpRenderTree/mac/TextInputControllerMac.m:
(-[TextInputController selectedOrMainFrame]):
(-[TextInputController textInput]):
LayoutTests:
Add a layout test to exercise the crash.
* editing/inserting/remove-frame-with-marked-text-expected.txt: Added.
* editing/inserting/remove-frame-with-marked-text.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (280761 => 280762)
--- trunk/LayoutTests/ChangeLog 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/LayoutTests/ChangeLog 2021-08-07 22:47:09 UTC (rev 280762)
@@ -1,3 +1,16 @@
+2021-08-07 Wenson Hsieh <[email protected]>
+
+ [macOS] Web process crashes when detaching Document with uncommitted marked text
+ https://bugs.webkit.org/show_bug.cgi?id=228841
+ rdar://79960890
+
+ Reviewed by Ryosuke Niwa.
+
+ Add a layout test to exercise the crash.
+
+ * editing/inserting/remove-frame-with-marked-text-expected.txt: Added.
+ * editing/inserting/remove-frame-with-marked-text.html: Added.
+
2021-08-06 Arcady Goldmints-Orlov <[email protected]>
[SOUP] test imported/w3c/web-platform-tests/html/cross-origin-opener-policy/header-parsing.https.html fails
Added: trunk/LayoutTests/editing/inserting/remove-frame-with-marked-text-expected.txt (0 => 280762)
--- trunk/LayoutTests/editing/inserting/remove-frame-with-marked-text-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/inserting/remove-frame-with-marked-text-expected.txt 2021-08-07 22:47:09 UTC (rev 280762)
@@ -0,0 +1,5 @@
+PASS Removed the subframe
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Typing in the above iframe using an input method should not trigger a crash.
Added: trunk/LayoutTests/editing/inserting/remove-frame-with-marked-text.html (0 => 280762)
--- trunk/LayoutTests/editing/inserting/remove-frame-with-marked-text.html (rev 0)
+++ trunk/LayoutTests/editing/inserting/remove-frame-with-marked-text.html 2021-08-07 22:47:09 UTC (rev 280762)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script>
+jsTestIsAsync = true;
+
+let subframe;
+async function compositionDidUpdate() {
+ await new Promise(requestAnimationFrame);
+ subframe.remove();
+ testPassed("Removed the subframe");
+ finishJSTest();
+}
+
+addEventListener("load", () => {
+ subframe = document.querySelector("iframe");
+ subframe.contentDocument.body.focus();
+ if (window.textInputController)
+ textInputController.setMarkedText("hello", 0, 5);
+});
+</script>
+</head>
+<body>
+<iframe srcdoc="
+ <style>body, html { width: 100%; height: 100%; }</style>
+ <body contenteditable></body>
+ <script>document.body.addEventListener('compositionupdate', () => parent.compositionDidUpdate());</script>"></iframe>
+<p>Typing in the above <code>iframe</code> using an input method should not trigger a crash.</p>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (280761 => 280762)
--- trunk/Source/WebCore/ChangeLog 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/Source/WebCore/ChangeLog 2021-08-07 22:47:09 UTC (rev 280762)
@@ -1,3 +1,25 @@
+2021-08-07 Wenson Hsieh <[email protected]>
+
+ [macOS] Web process crashes when detaching Document with uncommitted marked text
+ https://bugs.webkit.org/show_bug.cgi?id=228841
+ rdar://79960890
+
+ Reviewed by Ryosuke Niwa.
+
+ In the case where the document is in the process of being detached (underneath `willBeRemovedFromFrame()`), if
+ there is currently uncommitted marked text in the document, we will attempt to cancel the IME composition in the
+ process of clearing out the selection. On macOS, this calls into `Editor::cancelComposition()` which
+ subsequently triggers layout under various call stacks (DOM mutations, text event dispatch, and when scrolling
+ to reveal the selection); this triggers a security release assertion inside `Document::updateLayout()`.
+
+ To mitigate this, we avoid calling into this codepath if the Document no longer has a living render tree (i.e.,
+ the render tree has either been destroyed, is being destroyed, or has not been created yet).
+
+ Test: editing/inserting/remove-frame-with-marked-text.html
+
+ * editing/mac/EditorMac.mm:
+ (WebCore::Editor::selectionWillChange):
+
2021-08-07 Myles C. Maxfield <[email protected]>
Deduplicate logging channel algorithms
Modified: trunk/Source/WebCore/editing/mac/EditorMac.mm (280761 => 280762)
--- trunk/Source/WebCore/editing/mac/EditorMac.mm 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/Source/WebCore/editing/mac/EditorMac.mm 2021-08-07 22:47:09 UTC (rev 280762)
@@ -220,7 +220,7 @@
void Editor::selectionWillChange()
{
- if (!hasComposition() || ignoreSelectionChanges() || m_document.selection().isNone())
+ if (!hasComposition() || ignoreSelectionChanges() || m_document.selection().isNone() || !m_document.hasLivingRenderTree())
return;
cancelComposition();
Modified: trunk/Source/WebKit/ChangeLog (280761 => 280762)
--- trunk/Source/WebKit/ChangeLog 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/Source/WebKit/ChangeLog 2021-08-07 22:47:09 UTC (rev 280762)
@@ -1,3 +1,18 @@
+2021-08-07 Wenson Hsieh <[email protected]>
+
+ [macOS] Web process crashes when detaching Document with uncommitted marked text
+ https://bugs.webkit.org/show_bug.cgi?id=228841
+ rdar://79960890
+
+ Reviewed by Ryosuke Niwa.
+
+ Deploy a similar fix on iOS, to avoid any attempts to compute editor state due to discarding uncommitted marked
+ text during Document teardown. This is required in order to avoid the same security assertion when running the
+ new layout test on iOS.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::sendEditorStateUpdate):
+
2021-08-07 Myles C. Maxfield <[email protected]>
Deduplicate logging channel algorithms
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (280761 => 280762)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2021-08-07 22:47:09 UTC (rev 280762)
@@ -6484,7 +6484,7 @@
void WebPage::sendEditorStateUpdate()
{
Frame& frame = m_page->focusController().focusedOrMainFrame();
- if (frame.editor().ignoreSelectionChanges())
+ if (frame.editor().ignoreSelectionChanges() || !frame.document() || !frame.document()->hasLivingRenderTree())
return;
m_pendingEditorStateUpdateStatus = PendingEditorStateUpdateStatus::NotScheduled;
Modified: trunk/Tools/ChangeLog (280761 => 280762)
--- trunk/Tools/ChangeLog 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/Tools/ChangeLog 2021-08-07 22:47:09 UTC (rev 280762)
@@ -1,3 +1,19 @@
+2021-08-07 Wenson Hsieh <[email protected]>
+
+ [macOS] Web process crashes when detaching Document with uncommitted marked text
+ https://bugs.webkit.org/show_bug.cgi?id=228841
+ rdar://79960890
+
+ Reviewed by Ryosuke Niwa.
+
+ Make a small adjustment to DumpRenderTree, such that TextInputController targets the selected frame (or the main
+ frame, if there is no DOM selection). This behavior matches that of WebKitTestRunner, and allows layout tests
+ that use TextInputController to simulate setting marked text inside subframes.
+
+ * DumpRenderTree/mac/TextInputControllerMac.m:
+ (-[TextInputController selectedOrMainFrame]):
+ (-[TextInputController textInput]):
+
2021-08-07 Aakash Jain <[email protected]>
[ews] Ensure file handle is not leaked in loadConfig.py
Modified: trunk/Tools/DumpRenderTree/mac/TextInputControllerMac.m (280761 => 280762)
--- trunk/Tools/DumpRenderTree/mac/TextInputControllerMac.m 2021-08-07 22:40:24 UTC (rev 280761)
+++ trunk/Tools/DumpRenderTree/mac/TextInputControllerMac.m 2021-08-07 22:47:09 UTC (rev 280762)
@@ -284,9 +284,14 @@
[super dealloc];
}
+- (WebFrame *)selectedOrMainFrame
+{
+ return webView.selectedFrame ?: webView.mainFrame;
+}
+
- (NSObject <NSTextInput> *)textInput
{
- NSView <NSTextInput> *view = inputMethodView ? inputMethodView : (id)[[[webView mainFrame] frameView] documentView];
+ NSView <NSTextInput> *view = inputMethodView ?: (id)self.selectedOrMainFrame.frameView.documentView;
return [view conformsToProtocol:@protocol(NSTextInput)] ? view : nil;
}
