1738980 - Race condition in autocomplete and from autofill

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, defect, P2)

Description

[[:owlish] 🦉 PST]

Was discovered while tracing an intermittent mSearchesOngoing > 0 && mSearches.Contains(aSearch) assertion failure in nsAutoCompleteController.

Failure was not reproducible locally, and only happened in Web Render builds in automation. It reproduced always in the same test, in the same spot, here: https://searchfox.org/mozilla-central/source/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AutocompleteTest.kt#1032-1033 . There we first set value and then focus on the field. If those two statements are switched, the bug will be reproduced. My current understanding of it is this:

• Focus triggers the autocomplete machinery recognize the fields.
• Calling setValue on the input caused “change” DOM event fire for the input field.
• On that event, FormAutofillContent actor calls nsFormFillController.showPopup(), if the popup is pending.
• Since the input is non-zero length, that causes this series of calls: nsFormFillController.showPopup() => nsAutoCompleteController::HandleText => nsAutoCompleteController::StartSearches(). This last one mutates mSearches - it adds the input to that list. It also sets the timer for the search.
• ( The timer is cleared when closing popup - when selecting something from the popup or when the popup is invalidated by user deleting what she typed or typing something new; and also when nsAutoCompleteController::StopSearch() is called.)
• When the timer expires, the timer framework calls nsAutoCompleteController::BeforeSearches(), which sets mSearchesOngoing to the length of mSearches - 1 at this point.
• Meanwhile, the test focuses on the next input element, and the focus event triggers nsAutoCompleteController::ResetInternalState calling StopSearch() and thus clearing mSearches and the timer. If we are lucky, this event happens before the timer is up and before nsAutoCompleteController::onSearchResult is called, if we are not - nsAutoCompleteController::onSearchResult happens with the illegal state.

Adding if (!this.forceStop) here https://searchfox.org/mozilla-central/rev/80fddabd6773cd028ec69dd4f5a2a34fcd6b4387/toolkit/components/formautofill/FormAutofillContent.jsm#277 fixes the intermittent. (See https://treeherder.mozilla.org/jobs?repo=try&revision=e57bb3b6ba36f8b2dc70ada59a869c6ddfebdcb1 )

Comment 1

[Dimi Lee [:dimi][:dlee]]

Attachment: Bug 1738980 - Do not notify autocomplete search result when the search operation is canceled. r=sfoster,tgiles

Before this patch, when the search operation is canceled, we return null
in _getRecords instead of returning AutoCompleteResult, See
https://searchfox.org/mozilla-central/rev/80fddabd6773cd028ec69dd4f5a2a34fcd6b4387/toolkit/components/formautofill/FormAutofillContent.jsm#255-271

However, a null result does stop us from calling onSearchResult callback, which triggers
an assertion in nsAutoCompleteController.
https://searchfox.org/mozilla-central/rev/80fddabd6773cd028ec69dd4f5a2a34fcd6b4387/toolkit/components/formautofill/FormAutofillContent.jsm#276-291
https://searchfox.org/mozilla-central/rev/1e7f7235cf822e79cd79ba9e200329ede3d37925/toolkit/components/autocomplete/nsAutoCompleteController.cpp#813

This patches check whether the search operation is canceled before calling onSearchResult.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9985a5aacee
Do not notify autocomplete search result when the search  operation is canceled. r=sfoster,tgiles

Comment 3

[Norisz Fay [:noriszfay]]

Backed out for causing mochitest failures in browser_anti_clickjacking.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/88d84316a810249f76ee5a0c32a4c23615dfed26
Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=Edydfd4jRJy-zCoek_iRWw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=b9985a5aacee94ade0dd1f60c414b695d2172fc1
Failure log: https://treeherder.mozilla.org/logviewer?job_id=356993153&repo=autoland // https://treeherder.mozilla.org/logviewer?job_id=356994003&repo=autoland // https://treeherder.mozilla.org/logviewer?job_id=356993535&repo=autoland

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a3510e5b766
Do not notify autocomplete search result when the search  operation is canceled. r=sfoster,tgiles

Comment 5

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/5a3510e5b766