Closed Bug 531519 Opened 11 years ago Closed 11 years ago

getBrowserState sometimes returns two entries for one window

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3.7a1

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

http://hg.mozilla.org/mozilla-central/rev/4a7f5376270a exposed a constant failure on all platforms:
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/sessionstore/test/browser/browser_526613.js | number of open browser windows according to getBrowserState - Got 3, expected 2

This happens even when running browser_526613.js alone.

I think the second window (from _windows) and the third one (from _statesToRestore) are the same, but I'm not sure.
Attached patch patch for discussion (obsolete) — Splinter Review
I didn't want to disable the test, so I pushed this in order to fix the failure:
http://hg.mozilla.org/mozilla-central/rev/8f943a699370

I don't think this is the right fix, though. It seems like _windows is in a state that it shouldn't be in in the first place.
Attachment #414913 - Flags: review?(zeniko)
Comment on attachment 414913 [details] [diff] [review]
patch for discussion

This looks about right, although slightly fragile. I'd prefer it if windows that still have their actual state stored in _statesToRestore had their tracked data in _windows marked with a flag that's added in sss_onLoad and removed right before the data is deleted from _statesToRestore (or something to that end).
Attachment #414913 - Flags: review?(zeniko)
Attached patch patchSplinter Review
yeah, this makes more sense...
Assignee: nobody → dao
Attachment #414913 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #415142 - Flags: review?(zeniko)
Comment on attachment 415142 [details] [diff] [review]
patch

>+    if (aWindow.__SS_restoreID)

Please make that !this._isWindowLoaded(aWindow) like we do everywhere else. r+=me with this change.
Attachment #415142 - Flags: review?(zeniko) → review+
Blocks: 518970
Summary: getBrowserState still returns weird zombie windows → getBrowserState sometimes returns two entries for one window
Blocks: 529875
http://hg.mozilla.org/mozilla-central/rev/8d196765d647
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
Are there steps to reproduce this bug from a QA perspective (i.e. How can I
verify?)?
Blocks: 558639
You need to log in before you can comment on or make changes to this bug.