Attachment 359711 Details for Bug 189165 – Patch

[patch] Patch

bug-189165-20190121162933.patch (text/plain), 14.03 KB, created by Wenson Hsieh on 2019-01-21 16:29:34 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-01-21 16:29:34 PST

Size: 14.03 KB

patch

obsolete

>Subversion Revision: 240198
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 6296fefde2b9acfba3a7e1be488ac3c0809f5390..bdcebd7233cdeb8766660143a64ba774a1adb72b 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,53 @@
>+2019-01-21  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Multiple WKWebViewAutofillTests are flaky failures
>+        https://bugs.webkit.org/show_bug.cgi?id=189165
>+        <rdar://problem/47433765>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        These tests are currently flaky because they expect an invocation of "Element.blur()" in the web process to
>+        immediately dispatch an IPC message to notify the UI process that the element has been blurred. In particular,
>+        the -textInputHasAutofillContext helper assumes that waiting for the next remote layer tree commit in the UI
>+        process in sufficient to ensure that any previous action that blurred the focused element in the web process
>+        would make its way to the UI process by the time the layer tree commit is finished.
>+
>+        However, WebPage::elementDidBlur sends its IPC message to the UI process asynchronously, using callOnMainThread.
>+        This means that if a layer tree flush was already scheduled in the web process before the element was blurred,
>+        the element blur IPC message to the UI process will lose the race against the layer tree commit, and the test
>+        will fail because it asks for -_autofillContext too early.
>+
>+        To fix this, we tweak these tests to actually wait until the intended input session change triggered by script
>+        is handled in the UI process.
>+
>+        * TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:
>+
>+        Tweak some of these tests to wait for input session changes before checking for the presence of an autofill
>+        context. The only exception is an existing test that doesn't allow programmatic focus to begin input sessions
>+        by default; to fix this test, we simply wait for _WKInputDelegate to be invoked, instead of waiting for a new
>+        input session.
>+
>+        (-[AutofillTestView textInputHasAutofillContext]):
>+
>+        Remove the incorrect presentation update here. This helper now assumes that the UI process is up to date.
>+
>+        * TestWebKitAPI/cocoa/TestWKWebView.h:
>+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
>+        (nextInputSessionChangeCount):
>+
>+        Monotonically increasing identifier that's incremeneted whenever an input session is started in the UI process.
>+        This includes changing the focused element from one to another.
>+
>+        (-[TestWKWebView initWithFrame:configuration:addToWindow:]):
>+        (-[TestWKWebView didStartFormControlInteraction]):
>+        (-[TestWKWebView didEndFormControlInteraction]):
>+        (-[TestWKWebView evaluateJavaScriptAndWaitForInputSessionToChange:]):
>+
>+        Add a helper to evaluate JavaScript and wait for this script to cause some change in the input session. This
>+        handles three cases: (1) changing focus from an element that doesn't require an input session to one that does,
>+        (2) changing focus between elements that require input sessions, and (3) changing focus from an input session
>+        that doesn't require an input session to one that doesn't.
>+
> 2019-01-19  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [Cocoa] Allow the page to add to the platform undo stack via UndoManager.addItem()
>diff --git a/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm b/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm
>index 9f8d304c21cce49aea81b86b8cc60b4c1c74e04c..fdde8090866d07dd03943dafa4038f7239fe5a86 100644
>--- a/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm
>+++ b/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm
>@@ -64,7 +64,6 @@ - (AutofillInputView *)_autofillInputView
> 
> - (BOOL)textInputHasAutofillContext
> {
>-    [self waitForNextPresentationUpdate];
>     NSURL *url = [self._autofillInputView._autofillContext objectForKey:@"_WebViewURL"];
>     if (![url isKindOfClass:[NSURL class]])
>         return NO;
>@@ -81,10 +80,10 @@ TEST(WKWebViewAutofillTests, UsernameAndPasswordField)
> {
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input id='password' type='password'>"];
>-    [webView stringByEvaluatingJavaScript:@"user.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"];
>     EXPECT_TRUE([webView textInputHasAutofillContext]);
> 
>-    [webView stringByEvaluatingJavaScript:@"password.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
>     EXPECT_TRUE([webView textInputHasAutofillContext]);
> 
>     auto credentialSuggestion = [UITextAutofillSuggestion autofillSuggestionWithUsername:@"frederik" password:@"famos"];
>@@ -92,7 +91,7 @@ TEST(WKWebViewAutofillTests, UsernameAndPasswordField)
> 
>     EXPECT_WK_STREQ("famos", [webView stringByEvaluatingJavaScript:@"password.value"]);
> 
>-    [webView stringByEvaluatingJavaScript:@"document.activeElement.blur()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.activeElement.blur()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
> 
>@@ -100,10 +99,10 @@ TEST(WKWebViewAutofillTests, UsernameAndPasswordFieldSeparatedByRadioButton)
> {
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input type='radio' name='radio_button' value='radio'><input id='password' type='password'>"];
>-    [webView stringByEvaluatingJavaScript:@"user.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"];
>     EXPECT_TRUE([webView textInputHasAutofillContext]);
> 
>-    [webView stringByEvaluatingJavaScript:@"password.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
>     EXPECT_TRUE([webView textInputHasAutofillContext]);
> 
>     auto credentialSuggestion = [UITextAutofillSuggestion autofillSuggestionWithUsername:@"frederik" password:@"famos"];
>@@ -112,7 +111,7 @@ TEST(WKWebViewAutofillTests, UsernameAndPasswordFieldSeparatedByRadioButton)
>     EXPECT_WK_STREQ("frederik", [webView stringByEvaluatingJavaScript:@"user.value"]);
>     EXPECT_WK_STREQ("famos", [webView stringByEvaluatingJavaScript:@"password.value"]);
> 
>-    [webView stringByEvaluatingJavaScript:@"document.activeElement.blur()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.activeElement.blur()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
> 
>@@ -120,10 +119,10 @@ TEST(WKWebViewAutofillTests, TwoTextFields)
> {
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView synchronouslyLoadHTMLString:@"<input id='text1' type='email'><input id='text2' type='text'>"];
>-    [webView stringByEvaluatingJavaScript:@"text1.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"text1.focus()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> 
>-    [webView stringByEvaluatingJavaScript:@"text2.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"text2.focus()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
> 
>@@ -131,7 +130,7 @@ TEST(WKWebViewAutofillTests, StandalonePasswordField)
> {
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView synchronouslyLoadHTMLString:@"<input id='password' type='password'>"];
>-    [webView stringByEvaluatingJavaScript:@"password.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
>     EXPECT_TRUE([webView textInputHasAutofillContext]);
> 
>     auto credentialSuggestion = [UITextAutofillSuggestion autofillSuggestionWithUsername:@"frederik" password:@"famos"];
>@@ -139,7 +138,7 @@ TEST(WKWebViewAutofillTests, StandalonePasswordField)
> 
>     EXPECT_WK_STREQ("famos", [webView stringByEvaluatingJavaScript:@"password.value"]);
> 
>-    [webView stringByEvaluatingJavaScript:@"document.activeElement.blur()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.activeElement.blur()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
> 
>@@ -147,7 +146,7 @@ TEST(WKWebViewAutofillTests, StandaloneTextField)
> {
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView synchronouslyLoadHTMLString:@"<input id='textfield' type='text'>"];
>-    [webView stringByEvaluatingJavaScript:@"textfield.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"textfield.focus()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
> 
>@@ -155,13 +154,13 @@ TEST(WKWebViewAutofillTests, AccountCreationPage)
> {
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input id='password' type='password'><input id='confirm_password' type='password'>"];
>-    [webView stringByEvaluatingJavaScript:@"user.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> 
>-    [webView stringByEvaluatingJavaScript:@"password.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> 
>-    [webView stringByEvaluatingJavaScript:@"confirm_password.focus()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"confirm_password.focus()"];
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
> 
>@@ -174,12 +173,15 @@ TEST(WKWebViewAutofillTests, AutofillRequiresInputSession)
> {
>     ClassMethodSwizzler swizzler([UIKeyboard class], @selector(isInHardwareKeyboardMode), reinterpret_cast<IMP>(overrideIsInHardwareKeyboardMode));
> 
>+    bool done = false;
>     auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>-    [(TestInputDelegate *)[webView _inputDelegate] setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
>+    [(TestInputDelegate *)[webView _inputDelegate] setFocusStartsInputSessionPolicyHandler:[&done] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
>+        done = true;
>         return _WKFocusStartsInputSessionPolicyAuto;
>     }];
>     [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input id='password' type='password'>"];
>     [webView stringByEvaluatingJavaScript:@"user.focus()"];
>+    Util::run(&done);
> 
>     EXPECT_FALSE([webView textInputHasAutofillContext]);
> }
>diff --git a/Tools/TestWebKitAPI/cocoa/TestWKWebView.h b/Tools/TestWebKitAPI/cocoa/TestWKWebView.h
>index 883abd1caca9d75e95e45019cae0bd42ef505de0..b6f099290c0b60d2276dbe665f7bb3fe1ad9703b 100644
>--- a/Tools/TestWebKitAPI/cocoa/TestWKWebView.h
>+++ b/Tools/TestWebKitAPI/cocoa/TestWKWebView.h
>@@ -87,6 +87,7 @@
> @property (nonatomic, readonly) CGRect caretViewRectInContentCoordinates;
> @property (nonatomic, readonly) NSArray<NSValue *> *selectionViewRectsInContentCoordinates;
> - (_WKActivatedElementInfo *)activatedElementAtPosition:(CGPoint)position;
>+- (void)evaluateJavaScriptAndWaitForInputSessionToChange:(NSString *)script;
> @end
> #endif
> 
>diff --git a/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm b/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
>index 54416bfa8197593b6224c5be66a89e52194d0153..d21116f10bb47bffc97a4be3820759bab3e6df37 100644
>--- a/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
>+++ b/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm
>@@ -249,11 +249,19 @@ - (void)resignKeyWindow
> 
> @end
> 
>+using InputSessionChangeCount = NSUInteger;
>+static InputSessionChangeCount nextInputSessionChangeCount()
>+{
>+    static InputSessionChangeCount gInputSessionChangeCount = 0;
>+    return ++gInputSessionChangeCount;
>+}
>+
> @implementation TestWKWebView {
>     RetainPtr<TestWKWebViewHostWindow> _hostWindow;
>     RetainPtr<TestMessageHandler> _testHandler;
> #if PLATFORM(IOS_FAMILY)
>     std::unique_ptr<ClassMethodSwizzler> _sharedCalloutBarSwizzler;
>+    InputSessionChangeCount _inputSessionChangeCount;
> #endif
> }
> 
>@@ -289,6 +297,7 @@ - (instancetype)initWithFrame:(CGRect)frame configuration:(WKWebViewConfiguratio
> #if PLATFORM(IOS_FAMILY)
>     // FIXME: Remove this workaround once <https://webkit.org/b/175204> is fixed.
>     _sharedCalloutBarSwizzler = std::make_unique<ClassMethodSwizzler>([UICalloutBar class], @selector(sharedCalloutBar), reinterpret_cast<IMP>(suppressUICalloutBar));
>+    _inputSessionChangeCount = 0;
> #endif
> 
>     return self;
>@@ -409,12 +418,46 @@ - (void)collapseToEnd
>     [self evaluateJavaScript:@"getSelection().collapseToEnd()" completionHandler:nil];
> }
> 
>+#if PLATFORM(IOS_FAMILY)
>+
>+- (void)didStartFormControlInteraction
>+{
>+    _inputSessionChangeCount = nextInputSessionChangeCount();
>+}
>+
>+- (void)didEndFormControlInteraction
>+{
>+    _inputSessionChangeCount = 0;
>+}
>+
>+#endif // PLATFORM(IOS_FAMILY)
>+
> @end
> 
> #if PLATFORM(IOS_FAMILY)
> 
> @implementation TestWKWebView (IOSOnly)
> 
>+- (void)evaluateJavaScriptAndWaitForInputSessionToChange:(NSString *)script
>+{
>+    auto initialChangeCount = _inputSessionChangeCount;
>+    BOOL hasEmittedWarning = NO;
>+    NSTimeInterval secondsToWaitUntilWarning = 2;
>+    NSTimeInterval startTime = [NSDate timeIntervalSinceReferenceDate];
>+
>+    [self objectByEvaluatingJavaScript:script];
>+    while ([[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]]) {
>+        if (_inputSessionChangeCount != initialChangeCount)
>+            break;
>+
>+        if (hasEmittedWarning || startTime + secondsToWaitUntilWarning >= [NSDate timeIntervalSinceReferenceDate])
>+            continue;
>+
>+        NSLog(@"Warning: expecting input session change count to differ from %tu", initialChangeCount);
>+        hasEmittedWarning = YES;
>+    }
>+}
>+
> - (UIView <UITextInputPrivate, UITextInputMultiDocument> *)textInputContentView
> {
>     return (UIView <UITextInputPrivate, UITextInputMultiDocument> *)[self valueForKey:@"_currentContentView"];

Actions: View | Formatted Diff | Diff

Attachments on bug 189165: 359711 | 359713 | 359777