1073339 - Investigate autocomplete test unreliability on Linux/e10s

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Bug 1072320 ended up being quite a rabbithole of inproductivity. We have a set of tests that aren't reliable on Linux (and so have been disabled there):

browser_action_keyword.js
browser_action_searchengine.js
browser_action_searchengine_alias.js

AFAICT, this is because when the test tries to interact with the autocomplete result, nothing happens. My assumption is that this is due to focus issues around popups on Linux, which we've had troubles with in the past.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Also browser_search_favicon.js.  I can see the same problem intermittently with e10s on Windows, and see no reason to believe they are different - so I'm opening a new bug to also disable these tests with e10s keeping this bug number as the reason.

As to the reason: I'm seeing that the failure in e10s is caused due to detachController in autocomplete.xml being called during the test, causing the search to stop before we've got the results we expect.  detachController is called via:

_adjustFocusAfterTabSwitch@chrome://browser/content/tabbrowser.xml:1251:9
_finalizeTabSwitch@chrome://browser/content/tabbrowser.xml:3273:11
set_selectedIndex/<@chrome://browser/content/tabbrowser.xml:5252:15
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:865:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:7

which is tabbrowser focusing the new tab asynchronously.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch

This is based off of the patches from bug 1075450.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0002-Bug-1073339-Convert-from-CRLF-to-LF-for-browser_acti.patch

Windows -> Unix line endings.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=ce898dc49b21

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8527295 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch

Review of attachment 8527295 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for looking into this :) It's been driving me batty.

Will also need to deal with browser_autocomplete_autoselect.js, added in bug 1067903.

::: browser/base/content/test/general/browser_action_searchengine.js
@@ +8,5 @@
>  
>    Services.search.addEngineWithDetails("MozSearch", "", "", "", "GET",
>                                         "http://example.com/?q={searchTerms}");
>    let engine = Services.search.getEngineByName("MozSearch");
> +  let gOriginalEngine = Services.search.currentEngine;

Nit: Rename, so we don't accidentally assume it's a global?

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Oh, would be good to also update browser_autocomplete_a11y_label.js too, for consistency.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v2

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0002-Bug-1073339-Convert-from-CRLF-to-LF-for-autocomplete.patch, v2

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Try push with unifiedcomplete=false:

https://tbpl.mozilla.org/?tree=Try&rev=3ee73b9c370d

Try push with unifiedcomplete=true:

https://tbpl.mozilla.org/?tree=Try&rev=2d7106bda22e

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Doesn't look great with e10s and unifiedcomplete enabled. If only I could run e10s tests locally...

Comment 11

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

you can by passing --e10s to mach mochitest-browser?

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Currently broken due to bug 1103159 unfortunately.

Comment 13

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8530242 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v2

Review of attachment 8530242 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see any obvious reason why this is failing on Try :\

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0002-Bug-1073339-Convert-from-CRLF-to-LF-for-autocomplete.patch, v3

Updated due to test changes.

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v3

This is looking better:

https://tbpl.mozilla.org/?tree=Try&rev=497aac59ca3a
https://tbpl.mozilla.org/?tree=Try&rev=d98d73126544

Comment 16

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8535551 [details] [diff] [review]
0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v3

Review of attachment 8535551 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/general/browser.ini
@@ -110,5 @@
>  [browser_aboutHome.js]
>  skip-if = e10s # Bug 1093153 - no about:home support yet
>  [browser_aboutSyncProgress.js]
>  [browser_action_keyword.js]
> -skip-if = os == "linux" || e10s # Bug 1073339 - Investigate autocomplete test unreliability on Linux/e10s

Look like browser_action_keyword.js should get updated too.

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1073339-Enable-autocomplete-tests-for-Linux-and-.patch, v4

Comment 18

[Tim Taubert [:ttaubert] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=b8a1b38b172c

Comment 19

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/d68830b2ce47
https://hg.mozilla.org/integration/fx-team/rev/704ca3c0bb46

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d68830b2ce47
https://hg.mozilla.org/mozilla-central/rev/704ca3c0bb46