Closed
Bug 897095
Opened 8 years ago
Closed 8 years ago
"System JS : ERROR resource:///modules/sessionstore/SessionStore.jsm:4307 \n TypeError: this._windows[aWindow.__SSi] is undefined" on green debug test runs
Categories
(Firefox :: Session Restore, defect)
Tracking
()
RESOLVED
FIXED
Firefox 27
Tracking | Status | |
---|---|---|
firefox25 | --- | unaffected |
firefox26 | --- | fixed |
firefox27 | --- | fixed |
firefox-esr24 | --- | unaffected |
People
(Reporter: emorley, Assigned: ttaubert)
References
(Blocks 1 open bug)
Details
(Whiteboard: [qa-])
Attachments
(1 file)
2.28 KB,
patch
|
smacleod
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
A soon to be deployed TBPL parser improvement has found a whole bunch of log spam in green debug mochitest-browser-chrome runs. a) Should these be making the test run fail? b) We should either null check or fix the root cause if this is unexpected, so we don't spam the annotated failure summary once this TBPL patchset is rolled out. eg: https://tbpl-dev.allizom.org/php/getParsedLog.php?id=25604620&tree=Mozilla-Central#error2 Rev4 MacOSX Lion 10.7 mozilla-central debug test mochitest-browser-chrome on 2013-07-23 02:32:38 PDT for push fb4bf993a58a slave: talos-r4-lion-044 { 02:58:05 INFO - INFO TEST-END | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_491577.js | finished in 1315ms 02:58:05 INFO - TEST-INFO | checking window state 02:58:05 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_491577.js | must wait for focus 02:58:05 INFO - WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file ../../../../content/events/src/nsContentEventHandler.cpp, line 93 02:58:06 INFO - TEST-START | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_495495.js 02:58:06 INFO - ++DOCSHELL 0x1624c8f30 == 88 [id = 1945] 02:58:06 INFO - ++DOMWINDOW == 210 (0x15ee1f1c8) [serial = 5230] [outer = 0x0] 02:58:06 INFO - ++DOMWINDOW == 211 (0x164726f48) [serial = 5231] [outer = 0x15ee1f1c8] 02:58:06 INFO - WARNING: NS_ENSURE_TRUE(mTextInputHandler) failed: file ../../../widget/cocoa/nsChildView.mm, line 5113 02:58:06 INFO - System JS : ERROR resource:///modules/sessionstore/SessionStore.jsm:4307 02:58:06 INFO - TypeError: this._windows[aWindow.__SSi] is undefined } http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/src/SessionStore.jsm#4370 4362 /** 4363 * Set the given window's busy state 4364 * @param aWindow the window 4365 * @param aValue the window's busy state 4366 */ 4367 _setWindowStateBusyValue: 4368 function ssi_changeWindowStateBusyValue(aWindow, aValue) { 4369 4370 this._windows[aWindow.__SSi].busy = aValue; 4371 4372 // Keep the to-be-restored state in sync because that is returned by 4373 // getWindowState() as long as the window isn't loaded, yet. 4374 if (!this._isWindowLoaded(aWindow)) { 4375 let stateToRestore = this._statesToRestore[aWindow.__SS_restoreID].windows[0]; 4376 stateToRestore.busy = aValue; 4377 } 4378 },
Flags: needinfo?(dteller)
I am not familiar with that bit of the code, but yes, it looks like these should cause failures.
Flags: needinfo?(dteller)
Assignee | ||
Comment 2•8 years ago
|
||
I assume those are caused by _setWindowStateReady() calls done off a timeout after the window has been closed. We should definitely fix that.
Comment 3•8 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #2) > I assume those are caused by _setWindowStateReady() calls done off a timeout > after the window has been closed. We should definitely fix that. I'm going to assume you're OK with being assigned to this then :)
Assignee: nobody → ttaubert
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #2) > I assume those are caused by _setWindowStateReady() calls done off a timeout > after the window has been closed. We should definitely fix that. :-)
Flags: needinfo?(ttaubert)
Assignee | ||
Comment 5•8 years ago
|
||
Sorry I don't have time to investigate this right now. I took a stab at it a couple of days ago but it wasn't as easy as I assumed in comment #2. Should not be too hard to fix once we identified the actual cause.
Assignee: ttaubert → nobody
Flags: needinfo?(ttaubert)
Updated•8 years ago
|
Assignee: nobody → smacleod
Status: NEW → ASSIGNED
Comment 6•8 years ago
|
||
I'm having trouble reproducing this locally, or on Try. Tim, were you able to?
Flags: needinfo?(ttaubert)
Comment 7•8 years ago
|
||
bug 916945 might make this show up more.
Assignee | ||
Comment 8•8 years ago
|
||
I currently have no idea what's causing this. I pushed a tiny patch to try that throws when we hit this line of code with this._windows[aWindow.__SSi] being undefined. https://tbpl.mozilla.org/?tree=Try&rev=ae19bdf67e3c
Flags: needinfo?(ttaubert)
Assignee | ||
Comment 9•8 years ago
|
||
Ok the try push didn't help much because I forgot to dump the stack trace. I thought the test harness does that, oh well :) At least I found an occurrence of the message that I can reproduce locally by running browser_637020.js.
Assignee | ||
Comment 10•8 years ago
|
||
It's caused by exactly what I said in comment #2. Not sure why my first attempt wasn't successful. Anyway, easy to fix.
Assignee | ||
Comment 11•8 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=d4a7ec669fd9
Assignee: smacleod → ttaubert
Attachment #808607 -
Flags: review?(smacleod)
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #11) > https://tbpl.mozilla.org/?tree=Try&rev=d4a7ec669fd9 I checked all m-bc logs of the above try changeset and I didn't see a single "this._windows[aWindow.__SSi] is undefined" error message.
Reporter | ||
Comment 13•8 years ago
|
||
Thank you for fixing this! :-)
Comment 14•8 years ago
|
||
Comment on attachment 808607 [details] [diff] [review] Don't call restoreHistory() for closed windows Review of attachment 808607 [details] [diff] [review]: ----------------------------------------------------------------- Looks all good to me!
Attachment #808607 -
Flags: review?(smacleod) → review+
Assignee | ||
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7a900d222edd
https://hg.mozilla.org/mozilla-central/rev/7a900d222edd
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Comment 17•8 years ago
|
||
Tim, can you please request Aurora approval for this please?
status-firefox25:
--- → unaffected
status-firefox26:
--- → affected
status-firefox27:
--- → fixed
status-firefox-esr24:
--- → unaffected
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 808607 [details] [diff] [review] Don't call restoreHistory() for closed windows [Approval Request Comment] Bug caused by (feature/regressing bug #): Has been around for a while. User impact if declined: None. Testing completed (on m-c, etc.): Landed on m-c without issues. Risk to taking this patch (and alternatives if risky): Very low-risk change. String or IDL/UUID changes made by this patch: None. We'd like to uplift this mostly to get rid of a couple of failures when running the test suites. It's quite unlikely that this is actually hit in the wild but it's also a very safe fix.
Attachment #808607 -
Flags: approval-mozilla-aurora?
Comment 19•8 years ago
|
||
Comment on attachment 808607 [details] [diff] [review] Don't call restoreHistory() for closed windows always a fan of low risk uplifts that help remove test failures, approving.
Attachment #808607 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 20•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/bb3cf62469e6
Updated•8 years ago
|
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•