getBrowserState sometimes returns two entries for one window

RESOLVED FIXED in Firefox 3.7a1

Status

()

Firefox
Session Restore
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 3.7a1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 414911 [details]
problematic browser state

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.
(Assignee)

Comment 1

9 years ago
Created attachment 414913 [details] [diff] [review]
patch for discussion

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 2

9 years ago
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)
(Assignee)

Comment 3

9 years ago
Created attachment 415142 [details] [diff] [review]
patch

yeah, this makes more sense...
Assignee: nobody → dao
Attachment #414913 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #415142 - Flags: review?(zeniko)

Comment 4

9 years ago
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+
(Assignee)

Updated

9 years ago
Blocks: 518970
(Assignee)

Updated

9 years ago
Summary: getBrowserState still returns weird zombie windows → getBrowserState sometimes returns two entries for one window
(Assignee)

Updated

9 years ago
Blocks: 529875
(Assignee)

Comment 5

9 years ago
http://hg.mozilla.org/mozilla-central/rev/8d196765d647
Status: ASSIGNED → RESOLVED
Last Resolved: 9 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?)?

Updated

8 years ago
Blocks: 558639
You need to log in before you can comment on or make changes to this bug.