Title: [264170] trunk
Revision
264170
Author
[email protected]
Date
2020-07-09 08:15:29 -0700 (Thu, 09 Jul 2020)

Log Message

[iOS] Occasional crash under -[UIView _setViewDelegate:] when presenting date and time pickers
https://bugs.webkit.org/show_bug.cgi?id=214120
<rdar://problem/65246918>

Reviewed by Darin Adler.

Source/WebKit:

This crash happens when attempting to present a date picker (`<input type=date>`) immediately after dismissing
it. We encounter an Objective-C exception thrown by UIKit, due to the `WKDateTimeContextMenuViewController`'s
view (the `UIDatePicker`) being presented while it is still owned by the preview view controller. We often avoid
this crash because the `WKDateTimeContextMenuViewController` is usually only owned by `WKDateTimePicker`, so
when we set `_viewController` to a new instance of `WKDateTimeContextMenuViewController`, the old view
controller is destroyed, and thus no longer owns the `UIDatePicker` view.

However, it's possible for anything (e.g. animation blocks in UIKit) to cause the old view controller to live
past the creation of the new view controller. If this happens, when we go and call `-setView:` on the new view
controller with the date picker view, the date picker view may still be the view of the old controller, and we
end up crashing.

To fix this, explicitly unload the old view controller's view before attempting to create the new view
controller.

Test: fast/forms/ios/show-and-dismiss-date-input.html

* UIProcess/ios/forms/WKDateTimeInputControl.mm:
(-[WKDateTimePicker contextMenuInteraction:configurationForMenuAtLocation:]):
(-[WKDateTimePicker contextMenuInteraction:willDisplayMenuForConfiguration:animator:]):
(-[WKDateTimePicker contextMenuInteraction:willEndForConfiguration:animator:]):

Make it possible to test date and time picker presentation and dismissal by calling into the private testing-
only subclassing hooks on `WKWebView` when we finish presenting and dismissing the date picker context menu.

(-[WKDateTimePicker removeContextMenuInteraction]):

Tools:

Implement additional support for testing date and time pickers presented using context menus on iOS 14.

* TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::isShowingContextMenu const):
* WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
* WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
(-[TestRunnerWKWebView _didShowContextMenu]):
(-[TestRunnerWKWebView _didDismissContextMenu]):
* WebKitTestRunner/ios/UIScriptControllerIOS.h:
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::setDidDismissContextMenuCallback):

Call the completion callback with `CallbackTypeDidDismissContextMenu`, rather than
`CallbackTypeDidEndFormControlInteraction`.

(WTR::UIScriptControllerIOS::isShowingContextMenu const):

Add a new script controller to ask whether we're currently presenting a context menu. This is useful in the case
where we want to wait until we're no longer showing a context menu, since we'll either immediately invoke script
completion, or stash a completion callback on `didDismissContextMenuCallback` to invoke script completion after
the context menu is finished dismissing.

LayoutTests:

Add a new layout test to verify that presenting a date picker twice does not result in a crash.

* fast/forms/ios/show-and-dismiss-date-input-expected.txt: Added.
* fast/forms/ios/show-and-dismiss-date-input.html: Added.
* resources/ui-helper.js:
(window.UIHelper.activateAndWaitForInputSessionAt.return.new.Promise.):

Teach this helper method to wait for context menus as well (for the case of date and time pickers on iOS).

(window.UIHelper.waitForInputSessionToDismiss.return.new.Promise.):
(window.UIHelper.waitForInputSessionToDismiss.return.new.Promise):
(window.UIHelper.waitForInputSessionToDismiss):

Likewise, teach this to wait for context menu dismissal.

(window.UIHelper.waitForContextMenuToHide.return.new.Promise):
(window.UIHelper.waitForContextMenuToHide):

Add a new helper to wait for context menus to hide.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (264169 => 264170)


--- trunk/LayoutTests/ChangeLog	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/LayoutTests/ChangeLog	2020-07-09 15:15:29 UTC (rev 264170)
@@ -1,3 +1,31 @@
+2020-07-09  Wenson Hsieh  <[email protected]>
+
+        [iOS] Occasional crash under -[UIView _setViewDelegate:] when presenting date and time pickers
+        https://bugs.webkit.org/show_bug.cgi?id=214120
+        <rdar://problem/65246918>
+
+        Reviewed by Darin Adler.
+
+        Add a new layout test to verify that presenting a date picker twice does not result in a crash.
+
+        * fast/forms/ios/show-and-dismiss-date-input-expected.txt: Added.
+        * fast/forms/ios/show-and-dismiss-date-input.html: Added.
+        * resources/ui-helper.js:
+        (window.UIHelper.activateAndWaitForInputSessionAt.return.new.Promise.):
+
+        Teach this helper method to wait for context menus as well (for the case of date and time pickers on iOS).
+
+        (window.UIHelper.waitForInputSessionToDismiss.return.new.Promise.):
+        (window.UIHelper.waitForInputSessionToDismiss.return.new.Promise):
+        (window.UIHelper.waitForInputSessionToDismiss):
+
+        Likewise, teach this to wait for context menu dismissal.
+
+        (window.UIHelper.waitForContextMenuToHide.return.new.Promise):
+        (window.UIHelper.waitForContextMenuToHide):
+
+        Add a new helper to wait for context menus to hide.
+
 2020-07-09  Zalan Bujtas  <[email protected]>
 
         [LFC][Verification] Use the table wrapper box's margin when checking the table box

Added: trunk/LayoutTests/fast/forms/ios/show-and-dismiss-date-input-expected.txt (0 => 264170)


--- trunk/LayoutTests/fast/forms/ios/show-and-dismiss-date-input-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/show-and-dismiss-date-input-expected.txt	2020-07-09 15:15:29 UTC (rev 264170)
@@ -0,0 +1,11 @@
+Tests that repeatedly showing and dismissing a date picker on iOS does not result in a crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Presented and dismissed date picker (1/2)
+PASS Presented and dismissed date picker (2/2)
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/forms/ios/show-and-dismiss-date-input.html (0 => 264170)


--- trunk/LayoutTests/fast/forms/ios/show-and-dismiss-date-input.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/show-and-dismiss-date-input.html	2020-07-09 15:15:29 UTC (rev 264170)
@@ -0,0 +1,31 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+<input type="date" value="1976-04-01">
+<script>
+jsTestIsAsync = true;
+setTimeout(finishJSTest, 5000)
+
+description("Tests that repeatedly showing and dismissing a date picker on iOS does not result in a crash.");
+addEventListener("load", async () => {
+    if (!window.testRunner)
+        return;
+
+    const input = document.querySelector("input");
+    const numberOfIterations = 2;
+    for (let i = 1; i <= numberOfIterations; ++i) {
+        await UIHelper.activateElementAndWaitForInputSession(input);
+        await UIHelper.activateAt(1, 1);
+        await UIHelper.waitForContextMenuToHide();
+        testPassed(`Presented and dismissed date picker (${i}/${numberOfIterations})`);
+    }
+    finishJSTest();
+});
+</script>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/LayoutTests/resources/ui-helper.js (264169 => 264170)


--- trunk/LayoutTests/resources/ui-helper.js	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/LayoutTests/resources/ui-helper.js	2020-07-09 15:15:29 UTC (rev 264170)
@@ -448,10 +448,12 @@
             testRunner.runUIScript(`
                 (function() {
                     function clearCallbacksAndScriptComplete() {
+                        uiController.didShowContextMenuCallback = null;
                         uiController.didShowKeyboardCallback = null;
                         uiController.willPresentPopoverCallback = null;
                         uiController.uiScriptComplete();
                     }
+                    uiController.didShowContextMenuCallback = clearCallbacksAndScriptComplete;
                     uiController.didShowKeyboardCallback = clearCallbacksAndScriptComplete;
                     uiController.willPresentPopoverCallback = clearCallbacksAndScriptComplete;
                     uiController.singleTapAtPoint(${x}, ${y}, function() { });
@@ -467,10 +469,12 @@
                     function clearCallbacksAndScriptComplete() {
                         uiController.didHideKeyboardCallback = null;
                         uiController.didDismissPopoverCallback = null;
+                        uiController.didHideContextMenuCallback = null;
                         uiController.uiScriptComplete();
                     }
                     uiController.didHideKeyboardCallback = clearCallbacksAndScriptComplete;
                     uiController.didDismissPopoverCallback = clearCallbacksAndScriptComplete;
+                    uiController.didHideContextMenuCallback = clearCallbacksAndScriptComplete;
                 })()`, resolve);
         });
     }
@@ -585,6 +589,22 @@
         });
     }
 
+    static waitForContextMenuToHide()
+    {
+        if (!this.isWebKit2() || !this.isIOSFamily())
+            return Promise.resolve();
+
+        return new Promise(resolve => {
+            testRunner.runUIScript(`
+                (function() {
+                    if (uiController.isShowingContextMenu)
+                        uiController.didDismissContextMenuCallback = () => uiController.uiScriptComplete();
+                    else
+                        uiController.uiScriptComplete();
+                })()`, resolve);
+        });
+    }
+
     static waitForKeyboardToHide()
     {
         if (!this.isWebKit2() || !this.isIOSFamily())

Modified: trunk/Source/WebKit/ChangeLog (264169 => 264170)


--- trunk/Source/WebKit/ChangeLog	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Source/WebKit/ChangeLog	2020-07-09 15:15:29 UTC (rev 264170)
@@ -1,3 +1,38 @@
+2020-07-09  Wenson Hsieh  <[email protected]>
+
+        [iOS] Occasional crash under -[UIView _setViewDelegate:] when presenting date and time pickers
+        https://bugs.webkit.org/show_bug.cgi?id=214120
+        <rdar://problem/65246918>
+
+        Reviewed by Darin Adler.
+
+        This crash happens when attempting to present a date picker (`<input type=date>`) immediately after dismissing
+        it. We encounter an Objective-C exception thrown by UIKit, due to the `WKDateTimeContextMenuViewController`'s
+        view (the `UIDatePicker`) being presented while it is still owned by the preview view controller. We often avoid
+        this crash because the `WKDateTimeContextMenuViewController` is usually only owned by `WKDateTimePicker`, so
+        when we set `_viewController` to a new instance of `WKDateTimeContextMenuViewController`, the old view
+        controller is destroyed, and thus no longer owns the `UIDatePicker` view.
+
+        However, it's possible for anything (e.g. animation blocks in UIKit) to cause the old view controller to live
+        past the creation of the new view controller. If this happens, when we go and call `-setView:` on the new view
+        controller with the date picker view, the date picker view may still be the view of the old controller, and we
+        end up crashing.
+
+        To fix this, explicitly unload the old view controller's view before attempting to create the new view
+        controller.
+
+        Test: fast/forms/ios/show-and-dismiss-date-input.html
+
+        * UIProcess/ios/forms/WKDateTimeInputControl.mm:
+        (-[WKDateTimePicker contextMenuInteraction:configurationForMenuAtLocation:]):
+        (-[WKDateTimePicker contextMenuInteraction:willDisplayMenuForConfiguration:animator:]):
+        (-[WKDateTimePicker contextMenuInteraction:willEndForConfiguration:animator:]):
+
+        Make it possible to test date and time picker presentation and dismissal by calling into the private testing-
+        only subclassing hooks on `WKWebView` when we finish presenting and dismissing the date picker context menu.
+
+        (-[WKDateTimePicker removeContextMenuInteraction]):
+
 2020-07-09  Carlos Garcia Campos  <[email protected]>
 
         [GTK] Add support for shm buffers in AcceleratedBackingStoreWayland when using the WPE renderer

Modified: trunk/Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm (264169 => 264170)


--- trunk/Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm	2020-07-09 15:15:29 UTC (rev 264170)
@@ -31,6 +31,7 @@
 #import "UserInterfaceIdiom.h"
 #import "WKContentView.h"
 #import "WKContentViewInteraction.h"
+#import "WKWebViewPrivateForTesting.h"
 #import "WebPageProxy.h"
 #import <UIKit/UIBarButtonItem.h>
 #import <UIKit/UIDatePicker.h>
@@ -164,7 +165,7 @@
 - (UIContextMenuConfiguration *)contextMenuInteraction:(UIContextMenuInteraction *)interaction configurationForMenuAtLocation:(CGPoint)location
 {
     return [UIContextMenuConfiguration configurationWithIdentifier:@"_UIDatePickerCompactEditor" previewProvider:^{
-
+        [_viewController setView:nil];
         _viewController = adoptNS([[WKDateTimeContextMenuViewController alloc] init]);
         RetainPtr<UINavigationController> navigationController = adoptNS([[UINavigationController alloc] initWithRootViewController:_viewController.get()]);
         NSString *resetString = WEB_UI_STRING_KEY("Reset", "Reset Button Date/Time Context Menu", "Reset button in date input context menu");
@@ -197,13 +198,23 @@
     } actionProvider:nil];
 }
 
-- (void)contextMenuInteraction:(UIContextMenuInteraction *)interaction willEndForConfiguration:(UIContextMenuConfiguration *)configuration animator:(id<UIContextMenuInteractionAnimating>)animator
+- (void)contextMenuInteraction:(UIContextMenuInteraction *)interaction willDisplayMenuForConfiguration:(UIContextMenuConfiguration *)configuration animator:(id <UIContextMenuInteractionAnimating>)animator
 {
-    [animator addCompletion:^{
-        [_view accessoryDone];
+    [animator addCompletion:[weakSelf = WeakObjCPtr<WKDateTimePicker>(self)] {
+        auto strongSelf = weakSelf.get();
+        [strongSelf->_view.webView _didShowContextMenu];
     }];
 }
 
+- (void)contextMenuInteraction:(UIContextMenuInteraction *)interaction willEndForConfiguration:(UIContextMenuConfiguration *)configuration animator:(id <UIContextMenuInteractionAnimating>)animator
+{
+    [animator addCompletion:[weakSelf = WeakObjCPtr<WKDateTimePicker>(self)] {
+        auto strongSelf = weakSelf.get();
+        [strongSelf->_view accessoryDone];
+        [strongSelf->_view.webView _didDismissContextMenu];
+    }];
+}
+
 - (void)removeContextMenuInteraction
 {
     if (_dateTimeContextMenuInteraction) {
@@ -210,6 +221,7 @@
         [_view removeInteraction:_dateTimeContextMenuInteraction.get()];
         _dateTimeContextMenuInteraction = nil;
         [_view _removeContextMenuViewIfPossible];
+        [_view.webView _didDismissContextMenu];
     }
 }
 

Modified: trunk/Tools/ChangeLog (264169 => 264170)


--- trunk/Tools/ChangeLog	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/ChangeLog	2020-07-09 15:15:29 UTC (rev 264170)
@@ -1,3 +1,34 @@
+2020-07-09  Wenson Hsieh  <[email protected]>
+
+        [iOS] Occasional crash under -[UIView _setViewDelegate:] when presenting date and time pickers
+        https://bugs.webkit.org/show_bug.cgi?id=214120
+        <rdar://problem/65246918>
+
+        Reviewed by Darin Adler.
+
+        Implement additional support for testing date and time pickers presented using context menus on iOS 14. 
+
+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
+        (WTR::UIScriptController::isShowingContextMenu const):
+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
+        (-[TestRunnerWKWebView _didShowContextMenu]):
+        (-[TestRunnerWKWebView _didDismissContextMenu]):
+        * WebKitTestRunner/ios/UIScriptControllerIOS.h:
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptControllerIOS::setDidDismissContextMenuCallback):
+
+        Call the completion callback with `CallbackTypeDidDismissContextMenu`, rather than
+        `CallbackTypeDidEndFormControlInteraction`.
+
+        (WTR::UIScriptControllerIOS::isShowingContextMenu const):
+
+        Add a new script controller to ask whether we're currently presenting a context menu. This is useful in the case
+        where we want to wait until we're no longer showing a context menu, since we'll either immediately invoke script
+        completion, or stash a completion callback on `didDismissContextMenuCallback` to invoke script completion after
+        the context menu is finished dismissing.
+
 2020-07-09  Philippe Normand  <[email protected]>
 
         [Flatpak SDK] Unable to build without Internet connection

Modified: trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl (264169 => 264170)


--- trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl	2020-07-09 15:15:29 UTC (rev 264170)
@@ -210,6 +210,8 @@
     attribute object didShowContextMenuCallback;
     attribute object didDismissContextMenuCallback;
 
+    readonly attribute boolean isShowingContextMenu;
+
     // <select> picker
     void selectFormAccessoryPickerRow(long rowIndex);
     boolean selectFormAccessoryHasCheckedItemAtRow(long rowIndex);

Modified: trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h (264169 => 264170)


--- trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h	2020-07-09 15:15:29 UTC (rev 264170)
@@ -253,6 +253,7 @@
     virtual bool isShowingMenu() const { notImplemented(); return false; }
     virtual JSObjectRef rectForMenuAction(JSStringRef action) const { notImplemented(); return nullptr; }
     virtual JSObjectRef menuRect() const { notImplemented(); return nullptr; }
+    virtual bool isShowingContextMenu() const { notImplemented(); return false; }
 
     // Selection
 

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h (264169 => 264170)


--- trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h	2020-07-09 15:15:29 UTC (rev 264170)
@@ -62,6 +62,7 @@
 @property (nonatomic, readonly, getter=isShowingKeyboard) BOOL showingKeyboard;
 @property (nonatomic, readonly, getter=isDismissingMenu) BOOL dismissingMenu;
 @property (nonatomic, readonly, getter=isShowingPopover) BOOL showingPopover;
+@property (nonatomic, readonly, getter=isShowingContextMenu) BOOL showingContextMenu;
 @property (nonatomic, assign) BOOL usesSafariLikeRotation;
 @property (nonatomic, readonly, getter=isInteractingWithFormControl) BOOL interactingWithFormControl;
 

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm (264169 => 264170)


--- trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm	2020-07-09 15:15:29 UTC (rev 264170)
@@ -77,6 +77,7 @@
 @property (nonatomic, getter=isShowingMenu, setter=setIsShowingMenu:) BOOL showingMenu;
 @property (nonatomic, getter=isDismissingMenu, setter=setIsDismissingMenu:) BOOL dismissingMenu;
 @property (nonatomic, getter=isShowingPopover, setter=setIsShowingPopover:) BOOL showingPopover;
+@property (nonatomic, getter=isShowingContextMenu, setter=setIsShowingContextMenu:) BOOL showingContextMenu;
 
 @end
 
@@ -217,6 +218,10 @@
 
 - (void)_didShowContextMenu
 {
+    if (self.showingContextMenu)
+        return;
+
+    self.showingContextMenu = YES;
     if (self.didShowContextMenuCallback)
         self.didShowContextMenuCallback();
 }
@@ -223,6 +228,10 @@
 
 - (void)_didDismissContextMenu
 {
+    if (!self.showingContextMenu)
+        return;
+
+    self.showingContextMenu = NO;
     if (self.didDismissContextMenuCallback)
         self.didDismissContextMenuCallback();
 }

Modified: trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h (264169 => 264170)


--- trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h	2020-07-09 15:15:29 UTC (rev 264170)
@@ -145,6 +145,7 @@
     void setAllowsViewportShrinkToFit(bool) override;
     void copyText(JSStringRef) override;
     void installTapGestureOnWindow(JSValueRef) override;
+    bool isShowingContextMenu() const override;
 
     bool mayContainEditableElementsInRect(unsigned x, unsigned y, unsigned width, unsigned height) override;
 

Modified: trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm (264169 => 264170)


--- trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm	2020-07-09 14:33:50 UTC (rev 264169)
+++ trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm	2020-07-09 15:15:29 UTC (rev 264170)
@@ -944,7 +944,7 @@
     webView().didDismissContextMenuCallback = makeBlockPtr([this, strongThis = makeRef(*this)] {
         if (!m_context)
             return;
-        m_context->fireCallback(CallbackTypeDidEndFormControlInteraction);
+        m_context->fireCallback(CallbackTypeDidDismissContextMenu);
     }).get();
 }
 
@@ -1299,6 +1299,11 @@
     }).get();
 }
 
+bool UIScriptControllerIOS::isShowingContextMenu() const
+{
+    return webView().showingContextMenu;
 }
 
+}
+
 #endif // PLATFORM(IOS_FAMILY)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to