[iOS] Crash in WebKit::WebPage::positionInformation via Range::startPosition
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jul 2019 21:56:58 +0000 (21:56 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jul 2019 21:56:58 +0000 (21:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199503

Reviewed by Wenson Hsieh.

Source/WebCore:

* editing/Editor.cpp:
(WebCore::Editor::compositionRange const): Added a FIXME.

Source/WebKit:

The crash was caused because focusedElementPositionInformation asssumes Editor::compositionRange is not null
whenever Editor::hasComposition returns true, which is not necessary the case when Editor::m_compositionNode
contains no text (data is of length 0).

Fixed the crash by adding an early return for when Editor::compositionRange returns nullptr.

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

Tools:

Added UIScriptController.ensurePositionInformationIsUpToDateAt using the existing WKWebView SPI:
_requestActivatedElementAtPosition

* DumpRenderTree/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
* DumpRenderTree/mac/UIScriptControllerMac.mm:
(WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
* TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* TestRunnerShared/UIScriptContext/UIScriptController.cpp:
(WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
* TestRunnerShared/UIScriptContext/UIScriptController.h:
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
* WebKitTestRunner/ios/UIScriptControllerMac.mm:
(WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):

LayoutTests:

Added a regression test for the crash.

* editing/input/delete-text-in-composition-expected.txt: Added.
* editing/input/delete-text-in-composition.html: Added.
* resources/ui-helper.js:
(window.UIHelper.ensurePositionInformationUpdateForElement): Added.

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/input/delete-text-in-composition-expected.txt [new file with mode: 0644]
LayoutTests/editing/input/delete-text-in-composition.html [new file with mode: 0644]
LayoutTests/resources/ui-helper.js
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
Tools/ChangeLog
Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm

index 45416aa..d5b8dc2 100644 (file)
@@ -1,3 +1,17 @@
+2019-07-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Crash in WebKit::WebPage::positionInformation via Range::startPosition
+        https://bugs.webkit.org/show_bug.cgi?id=199503
+
+        Reviewed by Wenson Hsieh.
+
+        Added a regression test for the crash.
+
+        * editing/input/delete-text-in-composition-expected.txt: Added.
+        * editing/input/delete-text-in-composition.html: Added.
+        * resources/ui-helper.js:
+        (window.UIHelper.ensurePositionInformationUpdateForElement): Added.
+
 2019-07-02  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [WHLSL] Standard library is too big to directly include in WebCore
diff --git a/LayoutTests/editing/input/delete-text-in-composition-expected.txt b/LayoutTests/editing/input/delete-text-in-composition-expected.txt
new file mode 100644 (file)
index 0000000..ad112e6
--- /dev/null
@@ -0,0 +1,7 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This tests deleting the composition text does not result in a crash.
+To manually test, type in the box below using input method (e.g. Japanese or Chinese). WebKit on iOS should not crash.
+
+PASS - WebKit did not crash
diff --git a/LayoutTests/editing/input/delete-text-in-composition.html b/LayoutTests/editing/input/delete-text-in-composition.html
new file mode 100644 (file)
index 0000000..faa8229
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests deleting the composition text does not result in a crash.<br>
+To manually test, type in the box below using input method (e.g. Japanese or Chinese). WebKit on iOS should not crash.</p>
+<div id="editor" style="border: solid 2px blue; padding: 5px;" contenteditable></div>
+<div id="result"></div>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/ui-helper.js"></script>
+<script>
+
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+editor.focus();
+
+async function runTest() {
+    if (window.textInputController) {
+        textInputController.setMarkedText("hello", 0, 5);
+        await UIHelper.ensurePresentationUpdate();
+        editor.firstChild.data = '';
+        await UIHelper.ensurePositionInformationUpdateForElement(editor);
+        result.textContent = 'PASS - WebKit did not crash';
+    }
+    testRunner.notifyDone();
+}
+
+window.onload = runTest;
+
+var successfullyParsed = true;
+
+</script>
+</body>
+</html>
index 96684aa..98174c3 100644 (file)
@@ -254,6 +254,25 @@ window.UIHelper = class UIHelper {
         });
     }
 
+    static ensurePositionInformationUpdateForElement(element)
+    {
+        const boundingRect = element.getBoundingClientRect();
+        const x = boundingRect.x + 5;
+        const y = boundingRect.y + 5;
+
+        if (!this.isWebKit2()) {
+            testRunner.display();
+            return Promise.resolve();
+        }
+
+        return new Promise(resolve => {
+            testRunner.runUIScript(`
+                uiController.ensurePositionInformationIsUpToDateAt(${x}, ${y}, function () {
+                    uiController.uiScriptComplete();
+                });`, resolve);
+        });
+    }
+
     static delayFor(ms)
     {
         return new Promise(resolve => setTimeout(resolve, ms));
index b08cc59..bdc0b32 100644 (file)
@@ -1,3 +1,13 @@
+2019-07-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Crash in WebKit::WebPage::positionInformation via Range::startPosition
+        https://bugs.webkit.org/show_bug.cgi?id=199503
+
+        Reviewed by Wenson Hsieh.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::compositionRange const): Added a FIXME.
+
 2019-07-02  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [WHLSL] Standard library is too big to directly include in WebCore
index 059e1cb..79342c4 100644 (file)
@@ -3077,6 +3077,7 @@ RefPtr<Range> Editor::compositionRange() const
     unsigned length = m_compositionNode->length();
     unsigned start = std::min(m_compositionStart, length);
     unsigned end = std::min(std::max(start, m_compositionEnd), length);
+    // FIXME: Why is this early return neeed?
     if (start >= end)
         return nullptr;
     return Range::create(m_compositionNode->document(), m_compositionNode.get(), start, m_compositionNode.get(), end);
index ac2eced..c65bf2c 100644 (file)
@@ -1,3 +1,19 @@
+2019-07-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Crash in WebKit::WebPage::positionInformation via Range::startPosition
+        https://bugs.webkit.org/show_bug.cgi?id=199503
+
+        Reviewed by Wenson Hsieh.
+
+        The crash was caused because focusedElementPositionInformation asssumes Editor::compositionRange is not null
+        whenever Editor::hasComposition returns true, which is not necessary the case when Editor::m_compositionNode
+        contains no text (data is of length 0).
+
+        Fixed the crash by adding an early return for when Editor::compositionRange returns nullptr.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::focusedElementPositionInformation):
+
 2019-07-05  Zalan Bujtas  <zalan@apple.com>
 
         [ContentChangeObserver] REGRESSION (r247015): facebook photo/video upload button is unresponsive to user interaction.
index 1da5fdc..a4999bd 100644 (file)
@@ -2508,6 +2508,9 @@ static void focusedElementPositionInformation(WebPage& page, Element& focusedEle
     VisiblePosition position = frame.visiblePositionForPoint(constrainedPoint);
 
     RefPtr<Range> compositionRange = frame.editor().compositionRange();
+    if (!compositionRange)
+        return;
+
     if (position < compositionRange->startPosition())
         position = compositionRange->startPosition();
     else if (position > compositionRange->endPosition())
index be2c803..7e06dc2 100644 (file)
@@ -1,3 +1,26 @@
+2019-07-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Crash in WebKit::WebPage::positionInformation via Range::startPosition
+        https://bugs.webkit.org/show_bug.cgi?id=199503
+
+        Reviewed by Wenson Hsieh.
+
+        Added UIScriptController.ensurePositionInformationIsUpToDateAt using the existing WKWebView SPI:
+        _requestActivatedElementAtPosition
+
+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
+        * DumpRenderTree/mac/UIScriptControllerMac.mm:
+        (WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
+        (WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
+        * WebKitTestRunner/ios/UIScriptControllerMac.mm:
+        (WTR::UIScriptController::ensurePositionInformationIsUpToDateAt):
+
 2019-07-05  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r247115.
index d7ea466..c6f55a3 100644 (file)
@@ -63,6 +63,11 @@ void UIScriptController::doAfterNextStablePresentationUpdate(JSValueRef callback
     doAsyncTask(callback);
 }
 
+void UIScriptController::ensurePositionInformationIsUpToDateAt(long x, long y, JSValueRef callback)
+{
+    return doAsyncTask(callback);
+}
+
 void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef callback)
 {
     doAsyncTask(callback);
index af94be5..f26d2a0 100644 (file)
@@ -61,6 +61,11 @@ void UIScriptController::doAfterNextStablePresentationUpdate(JSValueRef callback
     doAsyncTask(callback);
 }
 
+void UIScriptController::ensurePositionInformationIsUpToDateAt(long x, long y, JSValueRef callback)
+{
+    doAsyncTask(callback);
+}
+
 void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef callback)
 {
     doAsyncTask(callback);
index 9e868ce..2144cf7 100644 (file)
@@ -45,7 +45,7 @@ interface UIScriptController {
 
     void doAfterPresentationUpdate(object callback); // Call the callback after sending a message to the WebProcess and receiving a subsequent update.
     void doAfterNextStablePresentationUpdate(object callback);
-
+    void ensurePositionInformationIsUpToDateAt(long x, long y, object callback);
     void doAfterVisibleContentRectUpdate(object callback);
 
     void simulateAccessibilitySettingsChangeNotification(object callback);
index 0f8db13..85c926e 100644 (file)
@@ -101,6 +101,10 @@ void UIScriptController::doAfterNextStablePresentationUpdate(JSValueRef)
 {
 }
 
+void UIScriptController::ensurePositionInformationIsUpToDateAt(long x, long y, JSValueRef)
+{
+}
+
 void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef)
 {
 }
index 18e18ab..fc380fa 100644 (file)
@@ -67,6 +67,7 @@ public:
     void doAsyncTask(JSValueRef callback);
     void doAfterPresentationUpdate(JSValueRef callback);
     void doAfterNextStablePresentationUpdate(JSValueRef callback);
+    void ensurePositionInformationIsUpToDateAt(long x, long y, JSValueRef callback);
     void doAfterVisibleContentRectUpdate(JSValueRef callback);
 
     void zoomToScale(double scale, JSValueRef callback);
index 955f654..e67a23d 100644 (file)
@@ -157,6 +157,18 @@ void UIScriptController::doAfterNextStablePresentationUpdate(JSValueRef callback
     }];
 }
 
+void UIScriptController::ensurePositionInformationIsUpToDateAt(long x, long y, JSValueRef callback)
+{
+    TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
+
+    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
+    [webView _requestActivatedElementAtPosition:CGPointMake(x, y) completionBlock:^(_WKActivatedElementInfo *) {
+        if (!m_context)
+            return;
+        m_context->asyncTaskComplete(callbackID);
+    }];
+}
+
 void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef callback)
 {
     TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
index 56fa83f..acb26c2 100644 (file)
@@ -59,6 +59,11 @@ void UIScriptController::doAfterNextStablePresentationUpdate(JSValueRef callback
     doAsyncTask(callback);
 }
 
+void UIScriptController::ensurePositionInformationIsUpToDateAt(long, long, JSValueRef callback)
+{
+    doAsyncTask(callback);
+}
+
 void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef callback)
 {
     doAsyncTask(callback);