And by correctly, I mean never. For example, hidden. This won't be updated if the tab was never restored (via max_concurrent_tabs). So you could get into the case (as I have) where you had a tab visible (tab A) at one point when quitting, but have since changed groups (panorama) and never restored tab A. So tab A still thinks it's visible when quitting because we'll use browser.__SS_data if it's there, but that didn't get updated. This is pretty similar to bug 579868, just surfacing in a different way. Granted it's a bug that should only appear in crash cases (restore never finished) or max_concurrent_tabs == 0. So it shouldn't happen often and a lot of people will never see it, but for those that do... perhaps we should block on it.

I haven't thought of any other specific attributes that can be updated without actually selecting the tab (unless maybe an extension does something with the tab's attributes and also calls persistTabAttribute?). So hidden should be pretty easy to handle on it's own - we just want to set tab.linkedBrowser.__SS_data.hidden

Per bug 611330, extData seems to be an issue as well, but maybe only if __SS_data.extdata didn't already exist. I'll investigate.

Adding dependent bugs from bug 611330

Seems to fix it. Hidden & extData are now restored correctly. Nothing else seems to be of issue. I was originally planning on just listening for events for TabShow TabHide and updating tab.__SS_data, doing the same for pinned, and removing that update from _collectTabData. But since we have to update extData as well, might as well just do it all together. Tests are passing here, pushed to try to get complete coverage.

Comment on attachment 492003 [details] [diff] [review] Patch v0.1 >+ // If __SS_extdata is set then we'll use that since it might be newer. >+ if (aTab.__SS_extdata) >+ tabData.extData = aTab.__SS_extdata; Future bug. I'd prefer that we move to explicit phasing of the restore process, instead of making assumptions based on presence/absence of each individual property like we do now. In Firefox 5, of course. r=me.

When run with bug 598795 and the patch for this bug, the bug 590268 test still fails. Attaching a little patch that helps isolate the issue.

So, to see the failures I'm seeing, apply the patch for this bug, apply the "Panorama 590268" patch also attached to this bug, and apply the patch for bug 598795 (in that order I suppose, though I don't think that matters). Once you've done a make, run the 590268 test: TEST_PATH=browser/components/sessionstore/test/browser/browser_590268.js make -C <your-obj-folder> mochitest-browser-chrome You should get about 11 extData failures. Let me know if you have questions.

So far I've figured out that tabview is calling setTabValue. A LOT. before sessionrestore puts any data into the tab. open tab 1, setTabValue("tabview-tab") on tab 1 open tab 2, setTabValue on tab 1,2 open tab 3, setTabValue on tab 1,2,3 etc. There are also a bunch of getTabValue("tabview-tab") calls in there too (one per tab as it's opened?) Since these are happening way early, it's before we even put __SS_data on each tab's browser. So setTabValue sets up tab.__SS_extdata. Since that test is checking that getTabValue is correct before SSTabRestoring is fired on that tab (but browser.__SS_data exists), it's now failing. getTabValue looks at tab.__SS_extdata first if it exists (which in this case it now does). So we can fix this by deleting tab.__SS_extdata in restoreHistoryPrecursor. We already do it in restoreHistory (but that becomes async). That's probably the best thing to do. We don't want to merge existing __SS_extdata with tabdata.extData (will result in staleness and mixing of data). So we'll just wipe out what panorama has put there. Alternatively, perhaps we could do something to trigger TabItems.pauseReconnecting(). It looks like that is only triggered by pb transitions, but TabItems.resumeReconnecting is triggered by sessionstore-browser-state-restored. There's no way to say "session restore is about to set the entire browser state". Perhaps sessionstore should have an observer topic to say we're starting to wipe out the current state. I'm not even sure that would necessarily fix this particular issue, but it would be better behavior anyway so tab candy isn't pounding sessionstore like this. (sigh... bug 588217 would have made this whole issue moot)

Filed bug 614708 for Ian's issue since it's related-but-not-super-closely to this bug. Ian, it worked for me, but can you check that it fixes your issue?

Pushed http://hg.mozilla.org/mozilla-central/rev/f15120d5a96e then backed out (http://hg.mozilla.org/mozilla-central/rev/6f4d85285727) because I forgot to update the test & it caused orange. Pushed with the fix: http://hg.mozilla.org/mozilla-central/rev/b652145a0e6f

Verified fixed on Firefox 4.0b8