| Summary: | [iOS] Multiple WKWebViewAutofillTests are flaky failures | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||
| Component: | Tools / Tests | Assignee: | 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: |
|
||||||||||
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. (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... 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
*** Bug 193288 has been marked as a duplicate of this bug. *** Can we prioritize this? I am working on EWS for API tests, and this flaky failure is creating problem. 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]); Sounds good! I will prioritize this. 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;
Created attachment 359711 [details]
Patch
Created attachment 359713 [details]
Fix macOS builds
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! 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" :| Created attachment 359777 [details]
Patch for landing (w/ typo fix)
Comment on attachment 359777 [details] Patch for landing (w/ typo fix) Clearing flags on attachment: 359777 Committed r240301: <https://trac.webkit.org/changeset/240301> |
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