Closed Bug 618151 Opened 9 years ago Closed 9 years ago

Overwriting state can lead to unrestored tabs

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b9
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: zpao, Assigned: zpao)

References

Details

Attachments

(1 file, 1 obsolete file)

Apparently browser.stop is async so we can still get load events from a tab that's being overwritten with a new state.

If a load event happens after we set __SS_data on the browser (in restoreHistoryPrecursor) but before we call restoreTab on that tab, we'll enter onTabLoad, where we delete __SS_data. __SS_data is used by restoreTab (and is expected to be there) so we then break.

I've somehow managed to reproduce this reliably in my test for bug 615394, so making that into a simpler test should be easy enough. Simply bailing out in onTabLoad if __SS_restoreState is truthy seems to do the trick. Patch sometime soon.
Other sources (read: gavin) say stop isn't async, but that load might still be
triggered after stop has been called. Regardless of how it's happening, it's happening.
Attached patch Patch v0.1 (obsolete) — Splinter Review
Reduced the tests from bug 615394 that was causing the issue. The test times out without the change.
Attachment #497389 - Flags: review?(dietrich)
Blocks a blocker so just making blocking explicit.

This is causing a consistent timeout with browser_600545.js... looking into it.
blocking2.0: --- → ?
Attached patch Patch v0.2Splinter Review
Fixed the timeout issue. We only want to bail out of onTabLoad if __SS_restoreState == TAB_STATE_NEEDS_RESTORE, not TAB_STATE_RESTORING. Fixed the timeout issue.
Attachment #497389 - Attachment is obsolete: true
Attachment #498646 - Flags: review?(dietrich)
Attachment #497389 - Flags: review?(dietrich)
Whiteboard: [has patch][needs review dietrich]
blocking2.0: ? → betaN+
Comment on attachment 498646 [details] [diff] [review]
Patch v0.2

r=me. file a bug on the unidentified-and-uncool load events that are happening after stop?
Attachment #498646 - Flags: review?(dietrich) → review+
Whiteboard: [has patch][needs review dietrich] → [has patch]
Pushed http://hg.mozilla.org/mozilla-central/rev/98b8d44e0096 and filed bug 621586.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [has patch]
Target Milestone: --- → Firefox 4.0b9
Is there a way I can verify this bug?
I haven't hit this in real life, but maybe calling setBrowserState while in the middle of restoring would do it. Otherwise the test is passing.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.