Attachment 373474 Details for Bug 199503 – Fixes the bug

[patch] Fixes the bug

bug-199503-20190704155642.patch (text/plain), 15.62 KB, created by Ryosuke Niwa on 2019-07-04 15:56:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-07-04 15:56:43 PDT

Size: 15.62 KB

patch

obsolete

>Subversion Revision: 246905
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 596824f4ada88cfc9c95aff5b4ee73979fb1481e..e057d26406576ad68b8132dcab22179f6a66c1f3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-07-04  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 NOBODY (OOPS!).
>+
>+        * editing/Editor.cpp:
>+        (WebCore::Editor::compositionRange const): Added a FIXME.
>+
> 2019-06-27  Simon Fraser  <simon.fraser@apple.com>
> 
>         REGRESSION (r246869): ASSERTION FAILED: !renderer().hasRepaintLayoutRects() || renderer().repaintLayoutRects().m_repaintRect == renderer().clippedOverflowRectForRepaint(renderer().containerForRepaint())
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e9cf0a4be9dfd2fa844706739e28e02f58d774e8..c1a5217d7014f4efdc85a8803d0109f80caaf4ed 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-07-04  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 NOBODY (OOPS!).
>+
>+        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.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _ensurePositionInformationAt:completion:]):
>+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::focusedElementPositionInformation):
>+
> 2019-06-27  Sihui Liu  <sihui_liu@apple.com>
> 
>         Regression(r246526): StorageManager thread hangs
>diff --git a/Source/WebCore/editing/Editor.cpp b/Source/WebCore/editing/Editor.cpp
>index 059e1cbf91f3fd68f251a75242f0875d434dd793..79342c497753d9a1526e055549fc7797214bf6e4 100644
>--- a/Source/WebCore/editing/Editor.cpp
>+++ b/Source/WebCore/editing/Editor.cpp
>@@ -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);
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 857ef857c5daac1f3940e814c15fbe6c8249dbb4..5d6f48f6fa9c9c3f940f3d7c74ed189f15df796d 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -7246,6 +7246,17 @@ - (void)_doAfterNextPresentationUpdateWithoutWaitingForPainting:(void (^)(void))
>     [self _internalDoAfterNextPresentationUpdate:updateBlock withoutWaitingForPainting:YES withoutWaitingForAnimatedResize:NO];
> }
> 
>+- (void)_ensurePositionInformationAt:(CGPoint)location completion:(void (^)(void))updateBlock
>+{
>+#if PLATFORM(IOS_FAMILY)
>+    WebKit::InteractionInformationRequest request { WebCore::IntPoint { location } };
>+    [_contentView ensurePositionInformationIsUpToDate:request];
>+    dispatch_async(dispatch_get_main_queue(), updateBlock);
>+#else
>+    dispatch_async(dispatch_get_main_queue(), updateBlock);
>+#endif
>+}
>+
> - (void)_doAfterNextVisibleContentRectUpdate:(void (^)(void))updateBlock
> {
> #if PLATFORM(IOS_FAMILY)
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>index 9263301d59d68ad9528184aea5e5d83a62e79f72..88ad5291c87db18e19931010a5c3d10e05d9ee4f 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
>@@ -548,6 +548,7 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) {
> - (void)_doAfterNextPresentationUpdate:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(10.12), ios(10.0));
> - (void)_doAfterNextPresentationUpdateWithoutWaitingForAnimatedResizeForTesting:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(10.14), ios(12.0));
> - (void)_doAfterNextPresentationUpdateWithoutWaitingForPainting:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(10.12.3), ios(10.3));
>+- (void)_ensurePositionInformationAt:(CGPoint)location completion:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> - (void)_doAfterNextVisibleContentRectUpdate:(void (^)(void))updateBlock WK_API_AVAILABLE(macos(10.13), ios(11.0));
> 
> - (void)_disableBackForwardSnapshotVolatilityForTesting WK_API_AVAILABLE(macos(10.12.3), ios(10.3));
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 91905a3ae6239c61319c52f4af84d37aca4694f5..b626206ad2d5642c9284d601b2099277e377f5b4 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -2479,6 +2479,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())
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index e14b12394b510c1278ff483b17d556a5fbff8a48..6ce7e94170c48c76afb6ca1d2b10a57d40649c0e 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-07-04  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 NOBODY (OOPS!).
>+
>+        Added UIScriptController.ensurePositionInformationIsUpToDateAt.
>+
>+        * 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):
>+
> 2019-06-27  Beth Dakin  <bdakin@apple.com>
> 
>         Upstream use of MACCATALYST
>diff --git a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>index ba1f3eda73ee9cc3f392540464db22b0a9b7117a..0a59ea2a67811b2ee4f2d6ca4b0a771d19dc6328 100644
>--- a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>+++ b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>@@ -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);
>diff --git a/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm b/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
>index af94be57004dad16ceaac0f43d4a8451596197dc..f26d2a024d020437fbd7bf14e3b7f2b5f8a58b8c 100644
>--- a/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
>+++ b/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm
>@@ -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);
>diff --git a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>index 3b69cde12753d41303c61529cf76b8402cfab41f..7adaf40d642239635c638f6e526c65770e7e272e 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>+++ b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>@@ -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);
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>index 1bedd7eb16f7459616199885a4fd210a3eeb76d5..f354b652b33e2478df2d5d0f0069e8a44e5bca60 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>@@ -101,6 +101,10 @@ void UIScriptController::doAfterNextStablePresentationUpdate(JSValueRef)
> {
> }
> 
>+void UIScriptController::ensurePositionInformationIsUpToDateAt(long x, long y, JSValueRef)
>+{
>+}
>+
> void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef)
> {
> }
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>index 0bedc04a12ec4ac3c2340b407d77e0b0ecedb8a7..c83c8d2df9037852786dcd7aba7b947520f5c47c 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>@@ -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);
>diff --git a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>index 9933150503f489d567811a35132c5ed5c5a477c9..85d1f358f1282813994a07b72be63feda44f492a 100644
>--- a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>@@ -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 _ensurePositionInformationAt:CGPointMake(x, y) completion:^() {
>+        if (!m_context)
>+            return;
>+        m_context->asyncTaskComplete(callbackID);
>+    }];
>+}
>+
> void UIScriptController::doAfterVisibleContentRectUpdate(JSValueRef callback)
> {
>     TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index af61b1137d8ab62ba7c32f8b08f577ecb7f384be..1697de07a6598580613319e3b0abc1d39be788ff 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-07-04  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 NOBODY (OOPS!).
>+
>+        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-06-27  Russell Epstein  <russell_e@apple.com>
> 
>         Layout Test fast/parser/parser-yield-timing.html was flaky due to strict tolerance, increased tolerance.
>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
>index 0000000000000000000000000000000000000000..ad112e62d5f402b2e052cefc28372cee7bf9cc77
>--- /dev/null
>+++ b/LayoutTests/editing/input/delete-text-in-composition-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..faa822939d9d598e498f72b03c9c3b62535fe2c2
>--- /dev/null
>+++ b/LayoutTests/editing/input/delete-text-in-composition.html
>@@ -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>
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 9eb97d070eeb4d0cd4c4c89d1ed8650f4c55cebc..30693b9beb421a909d2ab40d71489d6d45653353 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -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));

Actions: View | Formatted Diff | Diff

Attachments on bug 199503: 373474 | 373531