Closed Bug 1231422 Opened 9 years ago Closed 8 years ago

Intermittent browser_async_window_flushing.js | We should not have added the window to the closed windows array - Got 1, expected 0

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 47
Tracking Flags:
Tracking Status
firefox45 --- wontfix
firefox46 --- fixed
firefox47 --- fixed
firefox-esr45 --- fixed

People

(Reporter: philor, Assigned: mconley)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

MozReview Request: Bug 1231422 - Fix busted pref observer for browser.sessionstore.debug.no_auto_updates. r?billm
58 bytes, text/x-review-board-request
billm
: review+
Details
Bugnotes
7.76 KB, application/zip
Details 
Mike, possibly related to your recent sessionstore work?
status-firefox46: --- → affected
Flags: needinfo?(mconley)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)
> Mike, possibly related to your recent sessionstore work?

Definitely, yes. I'll see if I can give this some time this weekend.
Assignee: nobody → mconley
Oh cripes, I think I've figured this one out.

The thing that's supposed to listen for the TIMEOUT_DISABLED_PREF change in content-sessionStore.js doesn't work - it's comparing the pref string to topic, when it should compare it to data:

https://dxr.mozilla.org/mozilla-central/rev/c5da92c5b4906369dee83629f81d647226ac1038/browser/components/sessionstore/content/content-sessionStore.js#713

Luckily, that pref is just used for testing code.

Anyhow, this explains why this is intermittent - usually the test works quickly enough such that the message queue doesn't get flushed by the timer in the child, but periodically things line up just right so that the timer goes off in the right pocket of time, and blam.

Patch coming up.
Comment on attachment 8711144 [details]
MozReview Request: Bug 1231422 - Fix busted pref observer for browser.sessionstore.debug.no_auto_updates. r?billm

https://reviewboard.mozilla.org/r/32003/#review29101

Oops.
Attachment #8711144 - Flags: review?(wmccloskey) → review+
Flags: needinfo?(mconley)
https://hg.mozilla.org/mozilla-central/rev/98ccd385e6fd
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Apparently that's not all of the problem.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 1245212
Pretty sure I've figured this one out for real this time - see bug 1245212, and this try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=03c1b1945fdb
And just in time, this has put on a burst of speed lately and is very close to being the top failure.
Bug 1245212 just landed on fx-team. Let's see if this goes away now.
Looks like not :(
Flags: needinfo?(mconley)
Here's the good news:

Bug 1245212 seems to have fixed the orange on mozilla-central and mozilla-inbound (and since the recent uplift, I would wager it's had an impact on mozilla-aurora for the past few days).

If you look at the time that bug 1245212 landed, and when it was merged into mozilla-inbound after having landed in fx-team, you can see an immediate drop-off in occurrences.

The problem is that bug 1245212 was never uplifted to the aurora at the time (which is currently at beta).

So we will continue to see this orange on beta, release and esr45 until bug 1245212 gets merged in.

RyanVM: According to the tree rules, I can uplift fixes without requesting approval if they're test only using a=test-only... however, the code I'm touching here is _not_ within a test. It's just code that the test uses. Should I request approval for it then? And I suppose I should also do this for esr45?
Flags: needinfo?(mconley) → needinfo?(ryanvm)
If the code is only used by tests, I think a=test-only is fine. If it's used in shipping code as well, you should probably request formal approval. Thanks!
Flags: needinfo?(ryanvm)
The patch in Bug 1245212 just landed for ESR 45 and beta, so this should be gone once and for all.
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: