Closed Bug 669196 Opened 13 years ago Closed 13 years ago

Session Restore losing nearly all tabs

Categories

(Firefox :: Session Restore, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Firefox 8
Tracking Status
firefox6 + ---
firefox7 + ---
firefox8 + ---

People

(Reporter: Unfocused, Assigned: zpao)

References

Details

(Keywords: dataloss, verified-aurora, verified-beta, Whiteboard: [patch v0.1 landed][patch 2 landed][qa!])

Attachments

(3 files)

There's been a few reports of this in #developers - updating to the July 4th nightly is resulting in nearly all tabs being lost. Some apptabs are restored, and all tab groups are restored (although they're empty).
and from twitter.

https://twitter.com/#!/cwiiis/status/87809506796519424

datestamp claims "8 hours ago" which would likely put this on the July 3rd build.
This hit me last week on the June 28th nightly. It also hit my brother who is running Firefox 5.
Summary: July 4th Nightly loses nearly all tabs → Session Restore losing nearly all tabs
Happened on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110703 Firefox/7.0a1 for me.

Sessionstore.js was reset to the default about:home startup page, sessionstore.bak appeared to contain most/all of my tabs, but would not work when renamed to .js, so resorted to grep + manually opening tabs.
From what I can tell, session restore stops saving data when it starts hitting this error:

Error: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsISHistory.getEntryAtIndex]
Source file: resource:///components/nsSessionStore.js
Line: 1670
From running some scripts in a broken browser, Google+ is doing this _somehow_.

It's failing in a loop from 0 to history.count, and it seems that once it hits a bad one, there are many more following it.

Boris, Olli - any insight here? We can stop this in sessionstore with a try-catch easy enough, but it seems like a bigger problem (since we're only asking for theoretically valid entries).
Not offhand; I generally wouldn't trust the shistory... :(
Note that closing the google+ tab allows sessionstore to save its session.
FWIW, I think this is affecting Firefox 5 users too, so whatever we do we'll probably want to push this onto beta.
Can anyone reproduce this in a debug build? That might get some hint
where the NS_ERROR_FAILURE is happening.
And, is this bug about something which is new to July 4th build, or is this something older, or are there two different bugs?

Regression range is a must-have here.
Also, I don't have a Google+ account to test this.
(In reply to comment #11)
> And, is this bug about something which is new to July 4th build, or is this
> something older, or are there two different bugs?

From some reports, it seems to be happening in older builds too, and possibly affecting fx5.
Depends on: 670318
Per bug 670318 this also affects the back/forward navigation controls; those hit an exception too.
If someone can figure out consistent STR and ideally give me or Olli a way to reproduce (no google+ account here either), that would be really really useful.
This happened to me (comment 4 for more info) and I have a google apps account which is currently not able to access google+ (invitation or not). 

Are we sure the STR have to involve google+? At the time I had gmail, TPBL, b.m.o tabs open and that was about it.
We are going to track for the FF6 and Cheng will keep an eye on the feedback to see if there is any correlation.
Paul, it might be worthwhile to get the workaround in in case we don't get something better. Can you get us a patch for that?
Attached patch Patch v0.1Splinter Review
This should keep us saving, and use an assert for non-release builds so that when it happens, people know.

FWIW, the back/forward buttons are still broken with this problem too because they do the same sort of thing.
Attachment #545703 - Flags: review?(dietrich)
Comment on attachment 545703 [details] [diff] [review]
Patch v0.1

Review of attachment 545703 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #545703 - Flags: review?(dietrich) → review+
Blocks: 670308
Whiteboard: [temp fix for 6 avail in patch v0.1]
Should we get my patch landed on m-c and let that simmer, then come back around with a proper fix if we can get reliable STR?
What's the alternative plan?
Can this land, please? We probably need people to confirm it successfully works around the issue for the patch to land on aurora and beta.
Landed on inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/be62b2ea0b33

When it gets merged to m-c, I think we should either keep this open or file a followup bug to fix shistory (when we figure out what's actually wrong).
Whiteboard: [temp fix for 6 avail in patch v0.1] → [patch v0.1 landed][inbound]
Boris, Olli. From bug 673130, Shaver has a page that is reproducing this problem and isn't G+.

http://forums.somethingawful.com/showthread.php?threadid=3397539&userid=0&perpage=30&pagenumber=199

You might need to reload a couple times, but you'll notice that reload breaks & then session restore is broken along with back/forward.
http://hg.mozilla.org/mozilla-central/rev/be62b2ea0b33
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
I'm intrigued as to why this patch seems to be assuming the problem is in saving sessionstore.js - it's been noted above that the session appears to have been saved and is present in sessionstore.bak when you start up, but that this isn't loaded properly when renamed to .js. Isn't that the real problem, or at least a significant part of it?
Blocks: 673902
It seems like there is not a common, reliable set of steps to reproduce this bug for verification purposes. Marco, any feedback you can provide to help with verification?
Just got the assert window in the current nightly -> verifying.
Status: RESOLVED → VERIFIED
Attachment #545703 - Flags: approval-mozilla-beta?
Attachment #545703 - Flags: approval-mozilla-aurora?
I got the assert window too (holy crap, I wish I didn't see that all the time), but the problems with reload/back are still there.  Should I undup bug 673170?
(In reply to comment #32)
> Or, perhaps more usefully, undup bug 673130?

I think that the back/forward/reload brokenness is covered by bug 670318 (at least it was originally).

The patch in this bug with the assert is just meant to be a wallpaper until session history is fixed properly, then we can back it out. It might make sense to wallpaper fix b/f/r as well (either in your bug or somewhere else).
Windows from this bug is popping up quite often and it is extremely annoying.
Had the assert message on hotmail today!

http://i.imgur.com/WJL1J.jpg
Please push this to Aurora also. I lost my session like 10 times last month. I make backups multiple times a day but it is still extremely irritating.
Reverting from verified to resolved, as I'm not at all sure that this actually fixed something, despite me seeing the assert window :(
I'll remove my approval requests as well.
Status: VERIFIED → RESOLVED
Closed: 13 years ago13 years ago
Attachment #545703 - Flags: approval-mozilla-beta?
Attachment #545703 - Flags: approval-mozilla-aurora?
Had the assert message today on vk.com
Me too on facebook (Gecko/20110730 Firefox/8.0a1)
Me too on facebook Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110731 Firefox/8.0a1 ID:20110731030821

ASSERT: SessionStore failed gathering complete history for the focused window/tab. See bug 669196.
Stack Trace: 
0:sss_collectTabData([object XULElement])
1:sss_saveWindowHistory([object ChromeWindow])
2:sss_collectWindowData([object ChromeWindow])
3:([object ChromeWindow])
4:sss_forEachBrowserWindow(function (aWindow) {
    if (!this._isWindowLoaded(aWindow)) {
        return;
    }
    if (aUpdateAll ||
        this._dirtyWindows[aWindow.__SSi] || aWindow == activeWindow) {
        this._collectWindowData(aWindow);
    } else {
        this._updateWindowFeatures(aWindow);
    }
})
5:sss_getCurrentState(undefined,false)
6:sss_saveState()
7:sss_observe([xpconnect wrapped nsITimer],timer-callback,null)
I just got the same assert message.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110731 Firefox/8.0a1
I received this message when I attempted to search on Grooveshark.
Latest Fx8.0a1
2011-08-01 build, changeset: http://hg.mozilla.org/mozilla-central/rev/f92e021f1f44
still hit the same bug randomly

by random I mean I cannot identify which site(s) is/are causing this.
Tried G+ and got an assert. next load the session restored and the window that contained G+ could not refresh, back, or forward.
Dao says in bug 670308 that the assert isn't preventing dataloss, and so I looked again. serializeSessionStorage also loops from 0 to shistory.count and attempts to getEntryAtIndex without catching. So that should be fixed too. Reopening until that lands... patch in a minute.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Patch 2 v0.1Splinter Review
try/catch getEntryAtIndex in _serializeSessionStorage
Assignee: nobody → paul
Patch 2 landed on fx-team: http://hg.mozilla.org/integration/fx-team/rev/d93bbef62f0b
Whiteboard: [patch v0.1 landed][inbound] → [patch v0.1 landed][patch 2 landed fx-team]
http://hg.mozilla.org/mozilla-central/rev/d93bbef62f0b
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Whiteboard: [patch v0.1 landed][patch 2 landed fx-team] → [patch v0.1 landed][patch 2 landed]
Got the assert message on hotmail again today! latest nightly.

ASSERT: SessionStore failed gathering complete history for the focused window/tab. See bug 669196.
Stack Trace: 
0:sss_collectTabData([object XULElement])
1:sss_saveWindowHistory([object ChromeWindow])
2:sss_collectWindowData([object ChromeWindow])
3:([object ChromeWindow])
4:sss_forEachBrowserWindow(function (aWindow) {
    if (!this._isWindowLoaded(aWindow)) {
        return;
    }
    if (aUpdateAll ||
        this._dirtyWindows[aWindow.__SSi] || aWindow == activeWindow) {
        this._collectWindowData(aWindow);
    } else {
        this._updateWindowFeatures(aWindow);
    }
})
5:sss_getCurrentState(undefined,false)
6:sss_saveState()
7:sss_observe([xpconnect wrapped nsITimer],timer-callback,null)
Attached patch Patch 3 v0.1Splinter Review
If we're landing the wallpaper patches back onto beta/aurora, then we should land this too. Tab sync (and about:sync-tabs) is broken here because it tries to look at a nonexistent entry.

I think we're ok on m-c right now due to bug 670318 being fixed and don't need to take it now. Furthermore, I'm not seeing the assert anymore on pages I did see it, so we could probably take the patches out here (but I don't want to put money on that yet).
Attachment #550228 - Flags: review?(gavin.sharp)
Comment on attachment 550228 [details] [diff] [review]
Patch 3 v0.1

Can you generate a rollup patch?
Attachment #550228 - Flags: review?(gavin.sharp) → review+
For the record, I could repro this on Aurora, but not on Beta (using the url from comment #26 - load, click next page, reload a few times...). Can anybody who's using Beta confirm ASAP? (if we're going to land these patches there, then we want it to happen today). Open that url and do some things, open new tabs, then check that sessionstore.js is updating. On Aurora you'll see errors in the console coming from session restore and the file will stop updating
I got the assert message on G+ again as well
Depends on: 676170
No longer depends on: 676170
Message on G+ today, using Nightly Win32 from 2 August.
No longer blocks: 670308
Paul: Can we get explicit confirmation we cannot reproduce this on beta? Can we also get analysis related to the root cause? (like this landed on such and such a date so it missed beta6 and made it into 7, etc)
I have this issue on FF 5.0.1 too.

Thanks,
Cnon
(In reply to Christian Legnitto [:LegNeato] from comment #57)
> Paul: Can we get explicit confirmation we cannot reproduce this on beta?

I have not been able to reproduce on beta.

> Can
> we also get analysis related to the root cause? (like this landed on such
> and such a date so it missed beta6 and made it into 7, etc)

I'm quite sure the issue here is really just bug 670318 and has the same regression range (though the regression range there makes it look like it should be in beta?)

(In reply to cnon from comment #58)
> I have this issue on FF 5.0.1 too.

Are you sure it's the same issue? Do you see the errors mentioned in comment #5?
No, no error message pops up.

I'm also using Ad Blocker, plus, if that make a difference.

Cnon
Well, I uninstalled Ad Blocker plus, and this bug still occurred.

Cnon
How can I verify this as being Resolved Fixed?
Is it enough if I install a nightly released before the 4th of July and update to the latest version?
Are there other STR to reproduce this?
Thanks
(In reply to Vlad from comment #62)
> How can I verify this as being Resolved Fixed?

I used the url from comment 26, went to a few different pages, went back, reloaded and I would consistently see back/forward break and the assert (before bug 670318 landed). I no longer see that happen.
I have followed the steps from comment26 and comment63 but I have to press the back button approximately to go back one page. This is intended?
The forum loads in page 199. I clicked on the first page and then on the 5th page. I should hit the back button three times to go to the first visited page but instead I have to press aprox 15 times the button.

This is normal?
"approximately 15 times"
Vlad, this all depends on the number of pageloads which are happening. You can see a history of your back button by holding it down. I suspect this website is filling your browsing history with a lot of entries, giving the user the perception that they aren't going back when pressing back.
I realized that this particular site fills the history with a lot of pages but I had to make sure that this is the right behavior even though I had to press the back button several times. 

On the other hand, if I press the forward button, it goes forward only in those 5 entries that the site creates and not in the pages that I visited on that site.

This is the intended behavior?
Quite simply, the back button should move one page down in the list, the forward button should move one page forward in the list.
I'm referring in comment67 that "forward" doesn't work as expected in my opinion:
STR:
1. Load the site (page 199 is loaded)
2. Go to first page
3. Hit back five times. You are taken back to 199th page.
4. Go forward again to the first page.
You can't.

So my question is if this is the intended behavior.
(In reply to Vlad from comment #69)
> I'm referring in comment67 that "forward" doesn't work as expected in my
> opinion:
> STR:
> 1. Load the site (page 199 is loaded)
> 2. Go to first page
> 3. Hit back five times. You are taken back to 199th page.
> 4. Go forward again to the first page.
> You can't.
> 
> So my question is if this is the intended behavior.

I'm not sure if that is intended or not, but that's not what this bug was about. Can you file a new bug so we can investigate it in a new report? Thanks.
Tracking for Firefox Update 8 in case it gets re-opened.
I was registering on a forum today (not going to disclose due to being highly personal) and I didn't have the proper password requ. I pressed the back link (on forum not on browser) and it shot be backwards 10 steps (me browsing a few threads before signup) and put me on the first thread link that came from the google search result. 

the forum was powered by SMF (Simple Machines Forums) though. 

was any of these sites you've mentioned, Vlad powered by SMF? was that one forum powered by Vbulletin the only site you've experienced erratic back behaviour.
Whiteboard: [patch v0.1 landed][patch 2 landed] → [patch v0.1 landed][patch 2 landed][qa+]
I have verified this using the steps from comment26 and reloading the page doesn't break the session - back/forward works fine after reloading.

Setting resolution to Verified Fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 6.1; rv:9.0a2) Gecko/20111005 Firefox/9.0a2
Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111006 Firefox/10.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a2) Gecko/20111006 Firefox/9.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a1) Gecko/20111006 Firefox/10.0a1

Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111005 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111004 Firefox/10.0a1

Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 5.1; rv:9.0a2) Gecko/20111006 Firefox/9.0a2
Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111006 Firefox/10.0a1
Status: RESOLVED → VERIFIED
Whiteboard: [patch v0.1 landed][patch 2 landed][qa+] → [patch v0.1 landed][patch 2 landed][qa!]
It's still broken on FF 8 on Win 7.
It's still broken on FF 8.01 on Win 7 too.
You need to log in before you can comment on or make changes to this bug.