Bug 189165

Summary: [iOS] Multiple WKWebViewAutofillTests are flaky failures
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: Tools / TestsAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, bdakin, commit-queue, jbedard, jlewis3, lforschler, megan_gardner, realdawei, thorton, tsavell, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Fix macOS builds
thorton: review+
Patch for landing (w/ typo fix) none

Ryan Haddad
Reported 2018-08-30 11:28:32 PDT
WKWebViewAutofillTests.StandalonePasswordField is a flaky failure on iOS Simulator bots TestWebKitAPI.WKWebViewAutofillTests.StandalonePasswordField LEAK: 1 WebProcessPool LEAK: 1 WebPageProxy /Volumes/Data/slave/ios-simulator-11-debug/build/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:142 Value of: [webView textInputHasAutofillContext] Actual: true Expected: false https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/6188/steps/run-api-tests/logs/stdio
Attachments
Patch (14.03 KB, patch)
2019-01-21 16:29 PST, Wenson Hsieh
no flags
Fix macOS builds (14.07 KB, patch)
2019-01-21 17:19 PST, Wenson Hsieh
thorton: review+
Patch for landing (w/ typo fix) (14.02 KB, patch)
2019-01-22 14:43 PST, Wenson Hsieh
no flags
Wenson Hsieh
Comment 1 2018-08-30 11:41:25 PDT
I suspect we need to wait for the input session to actually begin in these tests, since that can be deferred until the next post-layout EditorState arrives in the UI process.
Wenson Hsieh
Comment 2 2018-08-30 11:46:05 PDT
(In reply to Wenson Hsieh from comment #1) > I suspect we need to wait for the input session to actually begin in these > tests, since that can be deferred until the next post-layout EditorState > arrives in the UI process. Never mind, textInputHasAutofillContext already waits for the next presentation update (which should include an up-to-date EditorState). Need to keep looking...
Ryan Haddad
Comment 3 2018-09-07 17:30:54 PDT
It looks like there are more flaky autofill tests: TestWebKitAPI.WKWebViewAutofillTests.StandalonePasswordField LEAK: 1 WebProcessPool LEAK: 1 WebPageProxy /Volumes/Data/slave/ios-simulator-11-debug/build/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:142 Value of: [webView textInputHasAutofillContext] Actual: true Expected: false https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/builds/6339/steps/run-api-tests/logs/stdio TestWebKitAPI.WKWebViewAutofillTests.UsernameAndPasswordFieldSeparatedByRadioButton /Volumes/Data/slave/ios-simulator-11-release/build/Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:115 Value of: [webView textInputHasAutofillContext] Actual: true Expected: false https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/builds/7303/steps/run-api-tests/logs/stdio
Ryan Haddad
Comment 4 2019-01-09 10:27:54 PST
*** Bug 193288 has been marked as a duplicate of this bug. ***
Aakash Jain
Comment 5 2019-01-18 11:30:07 PST
Can we prioritize this? I am working on EWS for API tests, and this flaky failure is creating problem.
Aakash Jain
Comment 6 2019-01-18 11:34:14 PST
One more related flaky test: TestWebKitAPI.WKWebViewAutofillTests.UsernameAndPasswordField Noticed in: https://ews-build.webkit-uat.org/#/builders/20/builds/215 It seems like all these flaky tests are failing at similar place: EXPECT_FALSE([webView textInputHasAutofillContext]);
Wenson Hsieh
Comment 7 2019-01-18 11:35:19 PST
Sounds good! I will prioritize this.
Wenson Hsieh
Comment 8 2019-01-21 15:37:50 PST
I can reproduce this somewhat regularly by running the function body of WKWebViewAutofillTests.StandalonePasswordField 50 times in a for loop. In the implementation of the testing helper method -textInputHasAutofillContext, we first attempt to wait until the next presentation update before consulting -_autofillContext; the idea is that by bouncing to the web process and back, we allow any focused element changes (due to elements blurring or receiving focus) to finish before checking for state that depends on the UI-side input session. However, in the case where an element is being blurred, this doesn't (always) work because the IPC message when blurring an element is dispatched asynchronously on the main thread (see: WebPage::elementDidBlur). This means that we're racing the element blur IPC message dispatch against the remote layer tree commit. In most cases (at least, from what I observe on my machine), we win the race because there isn't already a layer tree flush scheduled in the web process when we try to wait for the next presentation update, and so we get a sequence of events like this: (UI process): evaluate JS ("document.activeElement.blur()") (UI process): do after next presentation update (Web process): WebPage::elementDidBlur() (Web process): RemoteLayerTreeDrawingArea::flushLayers() (Web process): WebPage::elementDidBlur() - actually send the message (Web process): BackingStoreFlusher::flush() (UI process): WebPageProxy::elementDidBlur() (UI process): call do after next presentation update completion handler ...but if we lose the race, it looks something like: (Web process): RemoteLayerTreeDrawingArea::flushLayers() . . . (UI process): evaluate JS ("document.activeElement.blur()") (UI process): do after next presentation update (Web process): WebPage::elementDidBlur() (Web process): BackingStoreFlusher::flush() (Web process): WebPage::elementDidBlur() - actually send the message (UI process): call do after next presentation update completion handler (UI process): WebPageProxy::elementDidBlur() ...and so in this case, we subsequently continue the test before the input session is actually dismissed in the UI process. The fix is probably to avoid this race by using these testing hooks to figure out when we've focused or blurred, rather than waiting for presentation updates: - (void)didStartFormControlInteraction; - (void)didEndFormControlInteraction;
Radar WebKit Bug Importer
Comment 9 2019-01-21 16:15:27 PST
Wenson Hsieh
Comment 10 2019-01-21 16:29:34 PST
Wenson Hsieh
Comment 11 2019-01-21 17:19:53 PST
Created attachment 359713 [details] Fix macOS builds
Tim Horton
Comment 12 2019-01-22 13:23:20 PST
Comment on attachment 359713 [details] Fix macOS builds View in context: https://bugs.webkit.org/attachment.cgi?id=359713&action=review > Tools/TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:83 > - [webView stringByEvaluatingJavaScript:@"user.focus()"]; > + [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"user.focus()"]; Nice!
Wenson Hsieh
Comment 13 2019-01-22 14:04:40 PST
Comment on attachment 359713 [details] Fix macOS builds View in context: https://bugs.webkit.org/attachment.cgi?id=359713&action=review Thanks for the review! > Tools/ChangeLog:38 > + Monotonically increasing identifier that's incremeneted whenever an input session is started in the UI process. Oops, "incremeneted" :|
Wenson Hsieh
Comment 14 2019-01-22 14:43:32 PST
Created attachment 359777 [details] Patch for landing (w/ typo fix)
WebKit Commit Bot
Comment 15 2019-01-22 15:21:54 PST
Comment on attachment 359777 [details] Patch for landing (w/ typo fix) Clearing flags on attachment: 359777 Committed r240301: <https://trac.webkit.org/changeset/240301>
Note You need to log in before you can comment on or make changes to this bug.