Last Comment Bug 669196 - Session Restore losing nearly all tabs
: Session Restore losing nearly all tabs
Status: VERIFIED FIXED
[patch v0.1 landed][patch 2 landed][qa!]
: dataloss, verified-aurora, verified-beta
Product: Firefox
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- critical with 4 votes (vote)
: Firefox 8
Assigned To: Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
:
: Mike de Boer [:mikedeboer]
Mentors:
: 668417 670308 673130 676170 (view as bug list)
Depends on: 670318 921413
Blocks: 673902
  Show dependency treegraph
 
Reported: 2011-07-04 11:04 PDT by Blair McBride [:Unfocused] (UNAVAILABLE)
Modified: 2014-05-08 07:34 PDT (History)
37 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+
+


Attachments
Patch v0.1 (3.18 KB, patch)
2011-07-13 10:49 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
dietrich: review+
Details | Diff | Splinter Review
Patch 2 v0.1 (1.38 KB, patch)
2011-08-01 15:24 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
gavin.sharp: review+
Details | Diff | Splinter Review
Patch 3 v0.1 (1.33 KB, patch)
2011-08-02 15:01 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
gavin.sharp: review+
Details | Diff | Splinter Review

Description Blair McBride [:Unfocused] (UNAVAILABLE) 2011-07-04 11:04:21 PDT
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).
Comment 1 Rob Campbell [:rc] (:robcee) 2011-07-04 11:05:25 PDT
ditto.
Comment 2 Rob Campbell [:rc] (:robcee) 2011-07-04 11:06:24 PDT
and from twitter.

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

datestamp claims "8 hours ago" which would likely put this on the July 3rd build.
Comment 3 Chris AtLee [:catlee] 2011-07-04 11:06:56 PDT
This hit me last week on the June 28th nightly. It also hit my brother who is running Firefox 5.
Comment 4 Ed Morley [:emorley] 2011-07-04 12:27:15 PDT
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.
Comment 5 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-07-05 11:04:25 PDT
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
Comment 6 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-05 12:08:15 PDT
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).
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-07-05 12:21:31 PDT
Not offhand; I generally wouldn't trust the shistory... :(
Comment 8 Rob Campbell [:rc] (:robcee) 2011-07-05 12:34:17 PDT
Note that closing the google+ tab allows sessionstore to save its session.
Comment 9 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-05 12:49:10 PDT
FWIW, I think this is affecting Firefox 5 users too, so whatever we do we'll probably want to push this onto beta.
Comment 10 Olli Pettay [:smaug] 2011-07-05 13:31:45 PDT
Can anyone reproduce this in a debug build? That might get some hint
where the NS_ERROR_FAILURE is happening.
Comment 11 Olli Pettay [:smaug] 2011-07-05 13:33:29 PDT
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.
Comment 12 Olli Pettay [:smaug] 2011-07-05 13:38:39 PDT
Also, I don't have a Google+ account to test this.
Comment 13 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-07-05 15:32:39 PDT
(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.
Comment 14 Boris Zbarsky [:bz] (still a bit busy) 2011-07-09 00:09:24 PDT
Per bug 670318 this also affects the back/forward navigation controls; those hit an exception too.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2011-07-09 00:11:48 PDT
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.
Comment 16 Ed Morley [:emorley] 2011-07-09 02:24:15 PDT
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.
Comment 17 Sheila Mooney 2011-07-11 14:48:07 PDT
We are going to track for the FF6 and Cheng will keep an eye on the feedback to see if there is any correlation.
Comment 18 Asa Dotzler [:asa] 2011-07-12 14:46:11 PDT
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?
Comment 19 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-13 10:49:58 PDT
Created attachment 545703 [details] [diff] [review]
Patch v0.1

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.
Comment 20 Dietrich Ayala (:dietrich) 2011-07-13 11:49:05 PDT
Comment on attachment 545703 [details] [diff] [review]
Patch v0.1

Review of attachment 545703 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 21 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-18 14:57:18 PDT
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?
Comment 22 Dão Gottwald [:dao] 2011-07-18 17:39:15 PDT
What's the alternative plan?
Comment 23 Dão Gottwald [:dao] 2011-07-22 05:33:15 PDT
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.
Comment 24 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-22 10:44:21 PDT
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).
Comment 25 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-22 11:29:06 PDT
*** Bug 673130 has been marked as a duplicate of this bug. ***
Comment 26 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-22 11:35:54 PDT
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.
Comment 27 Marco Bonardo [::mak] 2011-07-25 05:50:05 PDT
http://hg.mozilla.org/mozilla-central/rev/be62b2ea0b33
Comment 28 Ben Sizer 2011-07-25 06:01:20 PDT
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?
Comment 29 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-07-25 09:53:54 PDT
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?
Comment 30 Dão Gottwald [:dao] 2011-07-26 16:10:26 PDT
Just got the assert window in the current nightly -> verifying.
Comment 31 Mike Shaver (:shaver -- probably not reading bugmail closely) 2011-07-27 06:02:58 PDT
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?
Comment 32 Mike Shaver (:shaver -- probably not reading bugmail closely) 2011-07-27 06:06:47 PDT
Or, perhaps more usefully, undup bug 673130?
Comment 33 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-07-27 10:26:55 PDT
(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).
Comment 34 Peter Henkel [:Terepin] 2011-07-27 10:36:46 PDT
Windows from this bug is popping up quite often and it is extremely annoying.
Comment 35 remixedcat 2011-07-28 07:34:01 PDT
Had the assert message on hotmail today!

http://i.imgur.com/WJL1J.jpg
Comment 36 Boris Prpic 2011-07-28 10:37:49 PDT
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.
Comment 37 Dão Gottwald [:dao] 2011-07-28 10:41:35 PDT
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.
Comment 38 Yakove 2011-07-30 11:43:34 PDT
Had the assert message today on vk.com
Comment 39 gadjo 2011-07-31 07:35:04 PDT
Me too on facebook (Gecko/20110730 Firefox/8.0a1)
Comment 40 Greg Karz 2011-07-31 14:46:17 PDT
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)
Comment 41 Joe Wilson 2011-07-31 16:43:23 PDT
I just got the same assert message.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0a1) Gecko/20110731 Firefox/8.0a1
Comment 42 mdew 2011-08-01 03:38:15 PDT
I received this message when I attempted to search on Grooveshark.
Comment 43 Rumos Mok 2011-08-01 08:18:09 PDT
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.
Comment 44 remixedcat 2011-08-01 11:00:05 PDT
Tried G+ and got an assert. next load the session restored and the window that contained G+ could not refresh, back, or forward.
Comment 45 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-01 15:20:53 PDT
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.
Comment 46 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-01 15:24:41 PDT
Created attachment 549945 [details] [diff] [review]
Patch 2 v0.1

try/catch getEntryAtIndex in _serializeSessionStorage
Comment 47 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-01 16:24:55 PDT
Patch 2 landed on fx-team: http://hg.mozilla.org/integration/fx-team/rev/d93bbef62f0b
Comment 48 Tim Taubert [:ttaubert] 2011-08-02 05:28:27 PDT
http://hg.mozilla.org/mozilla-central/rev/d93bbef62f0b
Comment 49 remixedcat 2011-08-02 09:45:57 PDT
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)
Comment 50 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-02 15:01:17 PDT
Created attachment 550228 [details] [diff] [review]
Patch 3 v0.1

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).
Comment 51 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-08-02 15:27:39 PDT
Comment on attachment 550228 [details] [diff] [review]
Patch 3 v0.1

Can you generate a rollup patch?
Comment 52 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-02 15:46:49 PDT
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
Comment 53 remixedcat 2011-08-02 15:48:54 PDT
I got the assert message on G+ again as well
Comment 54 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-02 20:30:19 PDT
*** Bug 676170 has been marked as a duplicate of this bug. ***
Comment 55 Mathieu Marquer 2011-08-03 03:40:38 PDT
Message on G+ today, using Nightly Win32 from 2 August.
Comment 56 Dão Gottwald [:dao] 2011-08-04 07:14:01 PDT
*** Bug 670308 has been marked as a duplicate of this bug. ***
Comment 57 christian 2011-08-08 14:57:38 PDT
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)
Comment 58 Cnon 2011-08-08 20:53:25 PDT
I have this issue on FF 5.0.1 too.

Thanks,
Cnon
Comment 59 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-08 21:37:08 PDT
(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?
Comment 60 Cnon 2011-08-10 18:05:57 PDT
No, no error message pops up.

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

Cnon
Comment 61 Cnon 2011-08-16 19:05:41 PDT
Well, I uninstalled Ad Blocker plus, and this bug still occurred.

Cnon
Comment 62 Vlad [QA] 2011-08-19 05:07:37 PDT
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
Comment 63 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-19 10:06:07 PDT
(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.
Comment 64 Vlad [QA] 2011-08-22 02:54:02 PDT
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?
Comment 65 Vlad [QA] 2011-08-22 05:48:51 PDT
"approximately 15 times"
Comment 66 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-08-22 07:05:43 PDT
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.
Comment 67 Vlad [QA] 2011-08-23 00:50:47 PDT
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?
Comment 68 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-08-23 09:23:23 PDT
Quite simply, the back button should move one page down in the list, the forward button should move one page forward in the list.
Comment 69 Vlad [QA] 2011-08-24 04:20:44 PDT
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.
Comment 70 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-08-24 07:12:50 PDT
(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.
Comment 71 Christopher Blizzard (:blizzard) 2011-08-25 14:42:52 PDT
Tracking for Firefox Update 8 in case it gets re-opened.
Comment 72 remixedcat 2011-08-25 21:32:04 PDT
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.
Comment 73 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-29 13:46:26 PDT
*** Bug 668417 has been marked as a duplicate of this bug. ***
Comment 74 Vlad [QA] 2011-10-06 08:03:28 PDT
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
Comment 75 Cnon 2011-11-10 18:27:52 PST
It's still broken on FF 8 on Win 7.
Comment 76 Cnon 2011-11-26 20:01:18 PST
It's still broken on FF 8.01 on Win 7 too.

Note You need to log in before you can comment on or make changes to this bug.