Changeset 240301 in webkit


Ignore:
Timestamp:
Jan 22, 2019 3:21:45 PM (5 years ago)
Author:
Wenson Hsieh
Message:

[iOS] Multiple WKWebViewAutofillTests are flaky failures
​https://bugs.webkit.org/show_bug.cgi?id=189165
<rdar://problem/47433765>

Reviewed by Tim Horton.

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 incremented 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.

Location:
trunk/Tools
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r240299 r240301  
     12019-01-22  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Multiple WKWebViewAutofillTests are flaky failures
     4        https://bugs.webkit.org/show_bug.cgi?id=189165
     5        <rdar://problem/47433765>
     6
     7        Reviewed by Tim Horton.
     8
     9        These tests are currently flaky because they expect an invocation of "Element.blur()" in the web process to
     10        immediately dispatch an IPC message to notify the UI process that the element has been blurred. In particular,
     11        the -textInputHasAutofillContext helper assumes that waiting for the next remote layer tree commit in the UI
     12        process in sufficient to ensure that any previous action that blurred the focused element in the web process
     13        would make its way to the UI process by the time the layer tree commit is finished.
     14
     15        However, WebPage::elementDidBlur sends its IPC message to the UI process asynchronously, using callOnMainThread.
     16        This means that if a layer tree flush was already scheduled in the web process before the element was blurred,
     17        the element blur IPC message to the UI process will lose the race against the layer tree commit, and the test
     18        will fail because it asks for -_autofillContext too early.
     19
     20        To fix this, we tweak these tests to actually wait until the intended input session change triggered by script
     21        is handled in the UI process.
     22
     23        * TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:
     24
     25        Tweak some of these tests to wait for input session changes before checking for the presence of an autofill
     26        context. The only exception is an existing test that doesn't allow programmatic focus to begin input sessions
     27        by default; to fix this test, we simply wait for _WKInputDelegate to be invoked, instead of waiting for a new
     28        input session.
     29
     30        (-[AutofillTestView textInputHasAutofillContext]):
     31
     32        Remove the incorrect presentation update here. This helper now assumes that the UI process is up to date.
     33
     34        * TestWebKitAPI/cocoa/TestWKWebView.h:
     35        * TestWebKitAPI/cocoa/TestWKWebView.mm:
     36        (nextInputSessionChangeCount):
     37
     38        Monotonically increasing identifier that's incremented whenever an input session is started in the UI process.
     39        This includes changing the focused element from one to another.
     40
     41        (-[TestWKWebView initWithFrame:configuration:addToWindow:]):
     42        (-[TestWKWebView didStartFormControlInteraction]):
     43        (-[TestWKWebView didEndFormControlInteraction]):
     44        (-[TestWKWebView evaluateJavaScriptAndWaitForInputSessionToChange:]):
     45
     46        Add a helper to evaluate JavaScript and wait for this script to cause some change in the input session. This
     47        handles three cases: (1) changing focus from an element that doesn't require an input session to one that does,
     48        (2) changing focus between elements that require input sessions, and (3) changing focus from an input session
     49        that doesn't require an input session to one that doesn't.
     50
    1512019-01-22  David Kilzer  <ddkilzer@apple.com>
    252

  • trunk/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm

    r237266 r240301  
    6565- (BOOL)textInputHasAutofillContext
    6666{
    67     [self waitForNextPresentationUpdate];
    6867    NSURL *url = [self._autofillInputView._autofillContext objectForKey:@"_WebViewURL"];
    6968    if (![url isKindOfClass:[NSURL class]])
    … …  
    8281    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    8382    [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input id='password' type='password'>"];
    84     [webView stringByEvaluatingJavaScript:@"user.focus()"];
     83    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"];
    8584    EXPECT_TRUE([webView textInputHasAutofillContext]);
    8685
    87     [webView stringByEvaluatingJavaScript:@"password.focus()"];
     86    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
    8887    EXPECT_TRUE([webView textInputHasAutofillContext]);
    8988
    … …  
    9392    EXPECT_WK_STREQ("famos", [webView stringByEvaluatingJavaScript:@"password.value"]);
    9493
    95     [webView stringByEvaluatingJavaScript:@"document.activeElement.blur()"];
     94    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.activeElement.blur()"];
    9695    EXPECT_FALSE([webView textInputHasAutofillContext]);
    9796}
    … …  
    101100    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    102101    [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input type='radio' name='radio_button' value='radio'><input id='password' type='password'>"];
    103     [webView stringByEvaluatingJavaScript:@"user.focus()"];
     102    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"];
    104103    EXPECT_TRUE([webView textInputHasAutofillContext]);
    105104
    106     [webView stringByEvaluatingJavaScript:@"password.focus()"];
     105    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
    107106    EXPECT_TRUE([webView textInputHasAutofillContext]);
    108107
    … …  
    113112    EXPECT_WK_STREQ("famos", [webView stringByEvaluatingJavaScript:@"password.value"]);
    114113
    115     [webView stringByEvaluatingJavaScript:@"document.activeElement.blur()"];
     114    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.activeElement.blur()"];
    116115    EXPECT_FALSE([webView textInputHasAutofillContext]);
    117116}
    … …  
    121120    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    122121    [webView synchronouslyLoadHTMLString:@"<input id='text1' type='email'><input id='text2' type='text'>"];
    123     [webView stringByEvaluatingJavaScript:@"text1.focus()"];
     122    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"text1.focus()"];
    124123    EXPECT_FALSE([webView textInputHasAutofillContext]);
    125124
    126     [webView stringByEvaluatingJavaScript:@"text2.focus()"];
     125    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"text2.focus()"];
    127126    EXPECT_FALSE([webView textInputHasAutofillContext]);
    128127}
    … …  
    132131    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    133132    [webView synchronouslyLoadHTMLString:@"<input id='password' type='password'>"];
    134     [webView stringByEvaluatingJavaScript:@"password.focus()"];
     133    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
    135134    EXPECT_TRUE([webView textInputHasAutofillContext]);
    136135
    … …  
    140139    EXPECT_WK_STREQ("famos", [webView stringByEvaluatingJavaScript:@"password.value"]);
    141140
    142     [webView stringByEvaluatingJavaScript:@"document.activeElement.blur()"];
     141    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.activeElement.blur()"];
    143142    EXPECT_FALSE([webView textInputHasAutofillContext]);
    144143}
    … …  
    148147    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    149148    [webView synchronouslyLoadHTMLString:@"<input id='textfield' type='text'>"];
    150     [webView stringByEvaluatingJavaScript:@"textfield.focus()"];
     149    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"textfield.focus()"];
    151150    EXPECT_FALSE([webView textInputHasAutofillContext]);
    152151}
    … …  
    156155    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    157156    [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input id='password' type='password'><input id='confirm_password' type='password'>"];
    158     [webView stringByEvaluatingJavaScript:@"user.focus()"];
     157    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"];
    159158    EXPECT_FALSE([webView textInputHasAutofillContext]);
    160159
    161     [webView stringByEvaluatingJavaScript:@"password.focus()"];
     160    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"password.focus()"];
    162161    EXPECT_FALSE([webView textInputHasAutofillContext]);
    163162
    164     [webView stringByEvaluatingJavaScript:@"confirm_password.focus()"];
     163    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"confirm_password.focus()"];
    165164    EXPECT_FALSE([webView textInputHasAutofillContext]);
    166165}
    … …  
    175174    ClassMethodSwizzler swizzler([UIKeyboard class], @selector(isInHardwareKeyboardMode), reinterpret_cast<IMP>(overrideIsInHardwareKeyboardMode));
    176175
     176    bool done = false;
    177177    auto webView = adoptNS([[AutofillTestView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
    178     [(TestInputDelegate *)[webView _inputDelegate] setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
     178    [(TestInputDelegate *)[webView _inputDelegate] setFocusStartsInputSessionPolicyHandler:[&done] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
     179        done = true;
    179180        return _WKFocusStartsInputSessionPolicyAuto;
    180181    }];
    181182    [webView synchronouslyLoadHTMLString:@"<input id='user' type='email'><input id='password' type='password'>"];
    182183    [webView stringByEvaluatingJavaScript:@"user.focus()"];
     184    Util::run(&done);
    183185
    184186    EXPECT_FALSE([webView textInputHasAutofillContext]);

  • trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.h

    r238471 r240301  
    8888@property (nonatomic, readonly) NSArray<NSValue *> *selectionViewRectsInContentCoordinates;
    8989- (_WKActivatedElementInfo *)activatedElementAtPosition:(CGPoint)position;
     90- (void)evaluateJavaScriptAndWaitForInputSessionToChange:(NSString *)script;
    9091@end
    9192#endif

  • trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

    r238921 r240301  
    250250@end
    251251
     252#if PLATFORM(IOS_FAMILY)
     253
     254using InputSessionChangeCount = NSUInteger;
     255static InputSessionChangeCount nextInputSessionChangeCount()
     256{
     257    static InputSessionChangeCount gInputSessionChangeCount = 0;
     258    return ++gInputSessionChangeCount;
     259}
     260
     261#endif
     262
    252263@implementation TestWKWebView {
    253264    RetainPtr<TestWKWebViewHostWindow> _hostWindow;
    … …  
    255266#if PLATFORM(IOS_FAMILY)
    256267    std::unique_ptr<ClassMethodSwizzler> _sharedCalloutBarSwizzler;
     268    InputSessionChangeCount _inputSessionChangeCount;
    257269#endif
    258270}
    … …  
    290302    // FIXME: Remove this workaround once <https://webkit.org/b/175204> is fixed.
    291303    _sharedCalloutBarSwizzler = std::make_unique<ClassMethodSwizzler>([UICalloutBar class], @selector(sharedCalloutBar), reinterpret_cast<IMP>(suppressUICalloutBar));
     304    _inputSessionChangeCount = 0;
    292305#endif
    293306
    … …  
    410423}
    411424
     425#if PLATFORM(IOS_FAMILY)
     426
     427- (void)didStartFormControlInteraction
     428{
     429    _inputSessionChangeCount = nextInputSessionChangeCount();
     430}
     431
     432- (void)didEndFormControlInteraction
     433{
     434    _inputSessionChangeCount = 0;
     435}
     436
     437#endif // PLATFORM(IOS_FAMILY)
     438
    412439@end
    413440
    … …  
    415442
    416443@implementation TestWKWebView (IOSOnly)
     444
     445- (void)evaluateJavaScriptAndWaitForInputSessionToChange:(NSString *)script
     446{
     447    auto initialChangeCount = _inputSessionChangeCount;
     448    BOOL hasEmittedWarning = NO;
     449    NSTimeInterval secondsToWaitUntilWarning = 2;
     450    NSTimeInterval startTime = [NSDate timeIntervalSinceReferenceDate];
     451
     452    [self objectByEvaluatingJavaScript:script];
     453    while ([[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]]) {
     454        if (_inputSessionChangeCount != initialChangeCount)
     455            break;
     456
     457        if (hasEmittedWarning || startTime + secondsToWaitUntilWarning >= [NSDate timeIntervalSinceReferenceDate])
     458            continue;
     459
     460        NSLog(@"Warning: expecting input session change count to differ from %tu", initialChangeCount);
     461        hasEmittedWarning = YES;
     462    }
     463}
    417464
    418465- (UIView <UITextInputPrivate, UITextInputMultiDocument> *)textInputContentView
Note: See TracChangeset for help on using the changeset viewer.