Closed Bug 1090633 Opened 10 years ago Closed 10 years ago

Intermittent browser_chatwindow.js | first window got new chat - 0 == 1 - JS frame :: browser_chatwindow.js :: testChatWindowChooser :: line 116 | first window got the chat

Categories

(Firefox Graveyard :: SocialAPI, defect)

Product:
Firefox Graveyard
Component:
SocialAPI
Platform:
x86_64
Windows 8.1
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox35 fixed, firefox36 fixed, firefox37 fixed, firefox38 fixed, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed, firefox-esr31 unaffected, firefox-esr38 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 40
Tracking Flags:
Tracking Status
firefox35 --- fixed
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- fixed

People

(Reporter: KWierso, Assigned: mixedpuppy)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

test fix
3.07 KB, patch
mixedpuppy
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review
0008-Bug-1090633-fix-some-focus-related-oranges-with-chat.patch
7.50 KB, patch
mixedpuppy
: review+
Details | Diff | Splinter Review
More likely a social api based issue.
Component: Client → SocialAPI
Product: Loop → Firefox
QA Contact: anthony.s.hughes
running a try with a possible fix here: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=149d52397a09 At least on my linux vm, I was able to 100% reproduce this intermittent, and the patch resolved it.
Attached patch test fix (obsolete) — — Splinter Review
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=149d52397a09 Looks like it's ok with this patch.
Assignee: nobody → mixedpuppy
Attachment #8534492 - Flags: review?(mhammond)
Comment on attachment 8534492 [details] [diff] [review] test fix Review of attachment 8534492 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/chat/head.js @@ +64,5 @@ > waitForFocus(deferred.resolve); > return deferred.promise; > } > > +function waitForCondition(condition, errorMsg) { TBH I'd prefer is this was in the test itself so there's less chance of it being used as the oversized hammer that it is, with a comment in the test saying we'd prefer to avoid using it using mutation observers or something. But yeah, good enough to fix this orange!
Attachment #8534492 - Flags: review?(mhammond) → review+
Attached patch test fix — — Splinter Review
moved wait code from head to test file, carry forward r+ https://hg.mozilla.org/integration/fx-team/rev/9612347414cb
Attachment #8534492 - Attachment is obsolete: true
Attachment #8534639 - Flags: review+
Comment on attachment 8534639 [details] [diff] [review] test fix Approval Request Comment [Feature/regressing bug #]: test failure [User impact if declined]: none [Describe test coverage new/current, TBPL]: [Risks and why]: none [String/UUID change made/needed]: none this fix should uplift to aurora to fix the intermittent there as well.
Attachment #8534639 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/9612347414cb
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Attachment #8534639 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/15384e0cd4d4 FYI, test-only changes can land without approval. But it doesn't hurt to ask anyway :)
status-firefox36: affected → fixed
status-firefox-esr31: --- → unaffected
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
See Also: → 1135545
Not sure who's working on SocialAPI these days, but this is happening with too great of a frequency to be ignored. Per the Test Disabling Policy [1], I intend to disable this test in a couple days if someone doesn't pick it up. [1] https://wiki.mozilla.org/Sheriffing/Test_Disabling_Policy
status-firefox38: --- → affected
status-firefox39: --- → affected
status-firefox40: --- → affected
Flags: needinfo?(mixedpuppy)
FTR, this is easy to repro for me locally - run the test with --run-until-failure and as soon as the test starts, switch focus to some other app. The test then immediately fails. I think there's a problem whereby a window isn't made the "most recent" window after focus when Fx itself doesn't have focus. In the cases when this fails, we are in a state where |Services.wm.getMostRecentWindow("navigator:browser") != window| even though |Services.focus.activeWindow == window|. Doing a waitForCondition for window to be the most-recent is better, but fails within a minute or so rather than immediately. IOW, we are managing to get into a state where |Services.focus.activeWindow != Services.wm.getMostRecentWindow("navigator:browser")|, which sounds like a bug in the window manager. We could probably "fix" this by having Chat.jsm consider the activeWindow before the most recent, but I think open that window manager bug first and see what we learn.
This is a test-only change. As we chatted about, this patch changes browser_chatwindow.js to just call findChromeWindowForChats rather than trying to ensure the chat actually opens in a window - this avoids focus changing while the test is running from breaking things. A try with that change then showed some orange in browser_focus.js, and I fixed that by not assuming the window still has focus. The browser_focus orange was only seen on Linux on earlier try runs. A linux-only try is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f5781869086, which has many retriggers and is green.
Flags: needinfo?(mixedpuppy)
Attachment #8599717 - Flags: review?(mixedpuppy)
Attachment #8599717 - Flags: review?(mixedpuppy) → review+
https://hg.mozilla.org/mozilla-central/rev/a375bdb7fb5d
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
status-firefox40: affected → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
status-firefox38.0.5: --- → affected
status-firefox-esr38: --- → affected
Target Milestone: Firefox 37 → Firefox 40
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: