The default bug view has changed. See this FAQ.

Intermittent browser_context_menu_autocomplete_interaction.js | Autocomplete shouldn't appear - false == true - JS frame :: chrome://mochitests/content/browser/toolkit/components/passwordmgr/test/browser/browser_context_menu_autocomplete_interaction.js

ASSIGNED
Assigned to

Status

()

Toolkit
Password Manager
ASSIGNED
a month ago
4 days ago

People

(Reporter: Treeherder Bug Filer, Assigned: johannh)

Tracking

(Depends on: 1 bug, {intermittent-failure, leave-open})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [stockwell needswork])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a month ago
treeherder
Filed by: cbook [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=75469632&repo=autoland

https://queue.taskcluster.net/v1/task/JmzTD7ppQ0qUgedTkpoSYg/runs/0/artifacts/public/logs/live_backing.log

Comment 1

27 days ago
15 failures in 182 pushes (0.082 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 15

Platform breakdown:
* linux32: 9
* linux64: 6

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-02-24&endday=2017-02-24&tree=all
When someone goes looking for the reason why the frequency of this exploded, the answer is going to be that it moved to a new chunk where it isn't happy.
The test was also just updated, in bug 1337259.

:johannh - Are these failures related to your changes? Can you make this run more reliably?
Flags: needinfo?(jhofmann)

Comment 4

26 days ago
45 failures in 47 pushes (0.957 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 18
* mozilla-inbound: 16
* mozilla-central: 9
* try: 2

Platform breakdown:
* linux32: 27
* linux64: 16
* windows8-64: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-02-25&endday=2017-02-25&tree=all
(Assignee)

Comment 5

25 days ago
I'm looking into it. So far I can unfortunately not reproduce this locally, running for a long time in chaos mode. I'll try to get a loaner, I guess.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)

Comment 6

25 days ago
97 failures in 812 pushes (0.119 failures/push) were associated with this bug in the last 7 days. 

This is the #15 most frequent failure this week. 

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:
* autoland: 60
* mozilla-inbound: 21
* mozilla-central: 12
* try: 4

Platform breakdown:
* linux32: 55
* linux64: 40
* windows8-64: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-02-20&endday=2017-02-26&tree=all

Comment 7

24 days ago
75 failures in 125 pushes (0.6 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 44
* mozilla-inbound: 25
* mozilla-central: 3
* graphics: 3

Platform breakdown:
* linux32: 44
* linux64: 31

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-02-27&endday=2017-02-27&tree=all

Comment 8

23 days ago
52 failures in 87 pushes (0.598 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* mozilla-inbound: 19
* autoland: 16
* try: 5
* mozilla-central: 5
* graphics: 5
* oak: 2

Platform breakdown:
* linux64: 26
* linux32: 26

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-02-28&endday=2017-02-28&tree=all
(Assignee)

Comment 9

22 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6264cbe8792

Comment 10

22 days ago
94 failures in 181 pushes (0.519 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* mozilla-inbound: 36
* autoland: 34
* try: 13
* graphics: 5
* mozilla-central: 4
* oak: 2

Platform breakdown:
* linux32: 51
* linux64: 43

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-01&endday=2017-03-01&tree=all
(Assignee)

Comment 11

21 days ago
Quick update on this:

The intermittent failure is pretty special and should normally not be observable by users. On the loaner I could only intermittently reproduce the problem on the very first right click into an input in the program lifetime. It's not reproducible until Firefox restarts. Presuming that this has to do something with formFillController initialization I pushed the following patch to try: 

https://hg.mozilla.org/try/rev/a6264cbe879275a26080056bb6787a8693e8842d

But it did not have the desired effect. Looking into this more thoroughly, it's unlikely that the primary changes in bug 1337259 caused the sudden spike in failures, since the username field was not touched in the patch and is also affected (https://public-artifacts.taskcluster.net/NnYUlzhYQzevBzBG3v2DJQ/0/public/test_info//mozilla-test-fail-screenshot_vuXQsH.png). It was more likely triggered by:

- bug 1337259 enabling the test case on the password field, doubling the chance of this happening
- changing the execution conditions by moving it do a different chunk, since this intermittent seems to not happen if formFillController is initialized already 

I can intermittently reproduce the bug on a Linux VM in Chaos Mode. Next I will try to debug the failure, maybe trying to catch one in rr. The weirdest part is that this is happening for both input fields although these two should have pretty different code paths for hiding the popup.

If I'm unable to find the cause until next week I'd say we disable the test on Linux since it's really an edge-case in production.
(Assignee)

Comment 12

21 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3668022b02b
Thanks for the work so far on this Johann.  If you need any help with your try push or trying a few different things out, please ask for it.
Whiteboard: [stockwell needswork]
Comment hidden (mozreview-request)
(Assignee)

Comment 15

21 days ago
After applying the first patch I could not observe the only-on-first-click behavior anymore (I'm not sure if I fixed it or if I wasn't observing it correctly from the start), but the test still intermittently failed. Debugging on Linux revealed that the timeDiff value in these cases was always slightly above 250, which is the maximum threshold. In my observation, the less performant Firefox was (e.g. with rr or gdb attached), the higher the potential timeDiff (seems obvious in hindsight).

So my fix for now would be to raise the threshold to an even higher value, e.g. 500, in addition to correctly initializing the timestamp and checking for a password field before comparing timestamps. Matt, we can change that number to something lower if you like, in my testing I never saw anything above 400 and my browser was painfully slow.

The initial try run looks good and I did a bunch of retriggers to confirm. Let's wait and see.
Comment on attachment 8843002 [details]
Bug 1337772 - Use mousedown instead of contextmenu to avoid showing the password autocomplete.

https://reviewboard.mozilla.org/r/116748/#review118492

r=me with 400 and the ini change for debug/ASAN
Attachment #8843002 - Flags: review?(MattN+bmo) → review+
(In reply to Johann Hofmann [:johannh] from comment #15)
> After applying the first patch I could not observe the only-on-first-click
> behavior anymore (I'm not sure if I fixed it or if I wasn't observing it
> correctly from the start), but the test still intermittently failed.
> Debugging on Linux revealed that the timeDiff value in these cases was
> always slightly above 250, which is the maximum threshold. In my
> observation, the less performant Firefox was (e.g. with rr or gdb attached),
> the higher the potential timeDiff (seems obvious in hindsight).
> 
> So my fix for now would be to raise the threshold to an even higher value,
> e.g. 500, in addition to correctly initializing the timestamp and checking
> for a password field before comparing timestamps. Matt, we can change that
> number to something lower if you like, in my testing I never saw anything
> above 400 and my browser was painfully slow.

The timing issue makes a lot of sense since the failures are much more common on debug/asan builds. Because non-optimized builds are slower perhaps we should disable the test outside opt? How about a value of 400 plus no debug/asan?

Comment 18

21 days ago
66 failures in 149 pushes (0.443 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 19
* mozilla-inbound: 16
* graphics: 12
* try: 9
* mozilla-central: 8
* oak: 2

Platform breakdown:
* linux32: 40
* linux64: 26

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-02&endday=2017-03-02&tree=all
(Assignee)

Comment 19

20 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d16b3d8e84db
Comment hidden (mozreview-request)
(Assignee)

Comment 21

20 days ago
> The timing issue makes a lot of sense since the failures are much more common on debug/asan builds. Because non-optimized builds are slower perhaps we should disable the test outside opt? How about a value of 400 plus no debug/asan?

Let's disable only on asan or linux 32 bit debug, since that seems to be the most common platform it fails on. I would prefer not to disable on too many platforms initially since this is still an indicator of end-user experience.
(Assignee)

Comment 22

20 days ago
Let's leave this open to verify that we don't have to disable for 64 bit debug linux (and that the fix worked).
Keywords: leave-open

Comment 23

20 days ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ebf65a1af072
Fix intermittent browser_context_menu_autocomplete_interaction.js. r=MattN

Updated

20 days ago
Whiteboard: [stockwell needswork] → [stockwell fixed]

Comment 24

20 days ago
52 failures in 157 pushes (0.331 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 23
* mozilla-inbound: 20
* mozilla-central: 5
* try: 3
* graphics: 1

Platform breakdown:
* linux64: 28
* linux32: 23
* windows7-32-vm: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-03&endday=2017-03-03&tree=all

Comment 25

19 days ago
18 failures in 45 pushes (0.4 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* mozilla-inbound: 9
* mozilla-central: 4
* try: 3
* autoland: 2

Platform breakdown:
* linux64: 12
* linux32: 5
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-04&endday=2017-03-04&tree=all

Comment 26

18 days ago
391 failures in 783 pushes (0.499 failures/push) were associated with this bug in the last 7 days. 

This is the #2 most frequent failure this week. 

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:
* autoland: 143
* mozilla-inbound: 136
* try: 42
* mozilla-central: 32
* graphics: 26
* oak: 12

Platform breakdown:
* linux32: 208
* linux64: 181
* windows8-64: 1
* windows7-32-vm: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-02-27&endday=2017-03-05&tree=all

Comment 27

17 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ebf65a1af072
:johann, you landed on Autoland and after doing so I don't see failures there- I am find to say this is fixed, but it just got merged today to m-c, so another day or so and it will be clear if this is fixed.

Comment 29

17 days ago
22 failures in 113 pushes (0.195 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* mozilla-inbound: 13
* try: 5
* oak: 3
* mozilla-central: 1

Platform breakdown:
* linux64: 13
* linux32: 9

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-06&endday=2017-03-06&tree=all
(Assignee)

Comment 30

16 days ago
(In reply to Joel Maher ( :jmaher) from comment #28)
> :johann, you landed on Autoland and after doing so I don't see failures
> there- I am find to say this is fixed, but it just got merged today to m-c,
> so another day or so and it will be clear if this is fixed.

Yes, in my experience I usually have to wait a couple of days until I can clearly perceive the effect of an intermittent fix, but feel free to close this whenever you feel confident that we fixed it :)
odd, we still have a high failure rate, I suspect this didn't make it to mozilla-inbound as almost all failures are on inbound.

:tomcat, early yesterday we merged to m-c, but I see a high failure rate on m-i, I don't see this landed on inbound- is there a reason we went to m-c but not to inbound yesterday?
Flags: needinfo?(cbook)
(In reply to Joel Maher ( :jmaher) from comment #31)
> odd, we still have a high failure rate, I suspect this didn't make it to
> mozilla-inbound as almost all failures are on inbound.
> 
> :tomcat, early yesterday we merged to m-c, but I see a high failure rate on
> m-i, I don't see this landed on inbound- is there a reason we went to m-c
> but not to inbound yesterday?

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebf65a1af072 :)
Flags: needinfo?(cbook)
ok, this is on inbound, odd why we are only getting failures on inbound though, lets see what another day or two yields in terms of patterns.
This came back in the push for bug 1317322: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&fromchange=5d257241323b6f9d9cd8c514bee9cfc8e82c2ca7&bugfiler&noautoclassify&filter-tier=1&filter-searchStr=84da3d7b3f5e4ee3892ef44191abb62b8cb682a9&tochange=2e025eb770e85de558fb72825539903b3e049172&group_state=expanded
Flags: needinfo?(michael)
I have no idea how my small patches which don't affect anything other than whether or not the paste command is enabled could be causing these test failures. The code is completely unrelated.

Is there a chance that my patch is just unlucky and just caused the test to move between chunks again to a less favorable position for performance?
Flags: needinfo?(michael) → needinfo?(jhofmann)
:kwierso, thanks for the retriggers and finding the root cause.

I verified the test in question has remained in the bc1 test chunk, so we didn't see anything move around.  This looks to be the same failure (logs are the same).  The difference is this is primarily linux opt/pgo, not linux debug.

Michael, could you maybe bisect this down with your changes to see what part of your changes are the root cause?  This looks to be a perma fail, so I would recommend backing out the original changes until we have an opportunity to find the root cause.

also are there interactions between the fix for this and the new code which landed?
(In reply to Michael Layzell [:mystor] from comment #35)
> The code is completely unrelated.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac2f3894113f831e5f2828185d8ac8c93ef8053b made changes to several other context menu tests, related to the paste command. I don't see anything in browser_context_menu_autocomplete_interaction.js specifically related to paste, but the context menu connection seems more than coincidental.
I've talked with ehsan, and I think I've figured out how my changes are regressing this test.

One of the effects which my changes have is reducing the number of sync IPC messages which we make when updating command state in e10s. Before my patch we had to perform multiple sync IPC messages to determine if there is data on the clipboard to determine whether or not to enable the paste command. After my changes this is no longer necessary, and we avoid those sync IPC messages.

This most likely affects the timing of opening the context menu (which requires updating command state), and thus throws off the fragile timing of this test.
(Assignee)

Comment 39

16 days ago
I don't think it's related to performance or chunking/interaction with surrounding tests since I can easily reproduce the failure on my local VM running just this individual test with stellar performance on opt. There's something else going on here.

> This most likely affects the timing of opening the context menu (which requires updating command state), and thus throws off the fragile timing of this test.

This sounds like your change made the context menu faster, which wouldn't make the test fail since it only fails on spectacularly bad context menu opening times.

Note that we rely on the contextmenu event to arrive synchronously with the focus event (before our Runnable created in the focus event listener is run on the main thread). If your patch changed something around this it might cause the failure. It's probably best to just debug it.

It's too late in my timezone to further investigate this, but I'll try to follow up tomorrow.

Thanks!
Flags: needinfo?(jhofmann)

Comment 40

16 days ago
107 failures in 141 pushes (0.759 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* mozilla-inbound: 53
* autoland: 34
* oak: 6
* mozilla-central: 6
* graphics: 6
* try: 2

Platform breakdown:
* linux64: 85
* linux32: 21
* windows7-32-vm: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-07&endday=2017-03-07&tree=all
Michael, are you going to fix this bug?  I just want to update the assignee and get this resolved
Flags: needinfo?(michael)
(Assignee)

Comment 42

15 days ago
Since I'm also looking into this right now (Michael's patch doesn't look wrong, our test is just too brittle against these kinds of changes) it's probably fine to leave it assigned to me.

Michael, it would still be great if you could validate that this is related to reducing the IPC messages.
As Johann said, I think he's going to handle it, and I'll try to help out if I can :).
Flags: needinfo?(michael)
(Assignee)

Comment 44

15 days ago
Debugging this turned out to be a bit of a pain due to occasional crashes but logging finally gave me the answer I was looking for. Turns out that mystor's patch seems to have triggered a condition on Linux where the contextmenu event is not guaranteed to happen before we spin the event loop in the focus event handler. This causes our lastContextMenuTimeStamp variable to be set to an old value when the runnable checks it to figure out if it should show the popup.

smaug and mystor told me on IRC that we should not rely on these events happening in sync and smaug made the suggestion to instead listen for mousedown events and ignore the next focus event if the mousedown came from a right click. I'll look into that tomorrow.

Joel, I'd be up for disabling this test on all of Linux while we work on a solution. I can do that tomorrow but feel free to do it yourself if you think it's urgent.
how long do you think a fix might take?  If it could be landed by Monday, lets leave it, otherwise lets disable- I can disable tomorrow if this might take a few more days.

Comment 46

15 days ago
113 failures in 169 pushes (0.669 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 42
* mozilla-inbound: 35
* try: 16
* mozilla-central: 9
* oak: 7
* graphics: 4

Platform breakdown:
* linux64: 78
* linux32: 32
* osx-10-10: 2
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-08&endday=2017-03-08&tree=all
Created attachment 8845534 [details] [diff] [review]
disable test temporarily on linux*
Attachment #8845534 - Flags: review?(gbrown)
ni myself to follow up in 1 week- I know :johannh is working on this, I don't want to forget about it
Flags: needinfo?(jmaher)
Whiteboard: [stockwell fixed] → [stockwell disabled]

Updated

14 days ago
Attachment #8845534 - Flags: review?(gbrown) → review+

Comment 49

14 days ago
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5a1f3455ad4
Intermittent browser_context_menu_autocomplete_interaction.js. disable on linux. r=gbrown

Comment 50

14 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b5a1f3455ad4

Comment 51

14 days ago
136 failures in 137 pushes (0.993 failures/push) were associated with this bug yesterday.  
Repository breakdown:
* autoland: 38
* try: 34
* mozilla-inbound: 31
* mozilla-central: 18
* graphics: 15

Platform breakdown:
* linux64: 92
* linux32: 39
* osx-10-10: 3
* windows7-32-vm: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-09&endday=2017-03-09&tree=all

Comment 52

13 days ago
28 failures in 172 pushes (0.163 failures/push) were associated with this bug yesterday.   

Repository breakdown:
* try: 14
* graphics: 13
* mozilla-central: 1

Platform breakdown:
* linux64: 19
* linux32: 9

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-10&endday=2017-03-10&tree=all

Comment 53

11 days ago
442 failures in 790 pushes (0.559 failures/push) were associated with this bug in the last 7 days. 

This is the #2 most frequent failure this week. 

** This failure happened more than 75 times this week! Resolving this bug is a very high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 1 week, the affected test(s) may be disabled. **  

Repository breakdown:
* mozilla-inbound: 145
* autoland: 117
* try: 82
* mozilla-central: 42
* graphics: 40
* oak: 16

Platform breakdown:
* linux64: 314
* linux32: 117
* osx-10-10: 6
* windows7-32-vm: 4
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-06&endday=2017-03-12&tree=all
(Assignee)

Comment 54

10 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f443f1beb3853374163f5eca7967869ddca55af
(Assignee)

Comment 55

10 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf54ec29af01fe02d1786dc857bdc8ea41e61898
Comment hidden (mozreview-request)
(Assignee)

Comment 57

10 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=af4f4b1394ff79494bc9b10127d5f03b8c02588b
Comment hidden (mozreview-request)
(Assignee)

Comment 59

10 days ago
Comment on attachment 8843002 [details]
Bug 1337772 - Use mousedown instead of contextmenu to avoid showing the password autocomplete.

So, uh, no idea how this works with reviewboard now. It seems to have pushed on top of the existing commit. Fun.

Matt, let me know if that's ok for you to review or if I should attach a new patch.
Attachment #8843002 - Flags: review+ → review?(MattN+bmo)
(Assignee)

Comment 60

9 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cd49e1ad2ca5fab351561d2950ab114cdbc53e9
thanks for getting a fix Johann!
Flags: needinfo?(jmaher)
Whiteboard: [stockwell disabled] → [stockwell needswork]
Duplicate of this bug: 1347375
(Assignee)

Updated

9 days ago
Depends on: 1192800
Comment on attachment 8843002 [details]
Bug 1337772 - Use mousedown instead of contextmenu to avoid showing the password autocomplete.

https://reviewboard.mozilla.org/r/116748/#review122720

r=me on this version but I guess there may be follow-up fixes for test failures.

::: toolkit/components/passwordmgr/LoginManagerContent.jsm:594
(Diff revision 4)
> -      } else {
> +    } else {
> -        log("maybeOpenAutocompleteAfterFocus: FormFillController has a different focused input");
> +      log("Not opening autocomplete after focus since a context menu was opened within",
> +          timeDiff, "ms");

Please keep this before an early return and get rid of the `else` as that is m-c style.
Attachment #8843002 - Flags: review?(MattN+bmo) → review+
(Assignee)

Comment 64

7 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e56a07af9924bc3e9744ed7497fff934d70125b7
(Assignee)

Comment 65

7 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6beed03238dda582bf1b2c9665410ab91575257c
25 failures in 777 pushes (0.032 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* try: 12
* autoland: 7
* mozilla-aurora: 5
* mozilla-inbound: 1

Platform breakdown:
* linux64: 12
* osx-10-10: 5
* linux32: 5
* windows7-32-vm: 2
* windows8-64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1337772&startday=2017-03-13&endday=2017-03-19&tree=all
You need to log in before you can comment on or make changes to this bug.