Closed Bug 611330 Opened 14 years ago Closed 14 years ago

setTabValue() couldn't save data for to-be-restored tabs

Categories

(Firefox :: Session Restore, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 607016

People

(Reporter: yuki, Unassigned)

References

Details

When setTabValue() is called, it stores the data to the property "aTab.__SS_extdata" of the tab. If the tab is not restored completely yet, (it will appear many times if you set browser.sessionstore.max_concurrent_tabs to 0) the data is stored to "aTab.linkedBrowser.__SS_data.extData" instead of "aTab.__SS_extdata". http://hg.mozilla.org/mozilla-central/file/7cf6faecbed3/browser/components/sessionstore/src/nsSessionStore.js#l1317 However, SessionStoreService.prototype._collectTabData() (which is called from SessionStoreService.prototype.saveState()) clears aTab.linkedBrowser.__SS_data.extData if the tab has no aTab.__SS_extdata. http://hg.mozilla.org/mozilla-central/file/7cf6faecbed3/browser/components/sessionstore/src/nsSessionStore.js#l1448 1539 if (aTab.__SS_extdata) 1540 tabData.extData = aTab.__SS_extdata; 1541 else if (tabData.extData) 1542 delete tabData.extData; As a result, if SessionStoreService.prototype.saveState() is called before the to-be-restored tab is completely restored, data saved by setTabValue() will be lost. This seems to be a regression caused by the bug 590268.
Steps to reproduce: 1. Open new browser window with a tab "about:config". 2. Set "browser.startup.page" to "3". 3. Set "browser.sessionstore.max_concurrent_tabs" to "0". 4. Set "devtools.errorconsole.enabled" to "true". 5. Open another tab and load "about:". 6. Select the first tab ("about:config"). 7. Quit Minefield. 8. Restart Minefield. Then there are one restored tab "about:blank" and another to-be-restored tab with the title "About:". 9. Open Error Console by Ctrl-Shift-J. 10. Copy following script into the "Evaluate" box and run it. ------------------------------------------------------------------- var ss = Components.classes['@mozilla.org/browser/sessionstore;1'] .getService(Components.interfaces.nsISessionStore); var wm = Components.classes['@mozilla.org/appshell/window-mediator;1'] .getService(Components.interfaces.nsIWindowMediator); var win = wm.getMostRecentWindow('navigator:browser'); win.gBrowser.tabContainer.addEventListener('TabClose', function(aEvent) { var tab = aEvent.originalTarget; var data = Date.now(); ss.setTabValue(tab, 'test-data', data); alert('stored data: '+data); }, false); win.gBrowser.tabContainer.addEventListener('SSTabRestored', function(aEvent) { var tab = aEvent.originalTarget; var data = ss.getTabValue(tab, 'test-data'); alert('restored data: '+data); }, false); ------------------------------------------------------------------- 11. Close the "About:" tab by closebox of the tab. Then an alert is shown with the current time like "stored data: 1289492130203". 12. Restore the tab by Ctrl-Shift-T. Actual Result: An alert is shown with the message "stored data: ". Expected Result: An alert is shown with the message "stored data: 1289492130203".
Adding Paul, as he wrote the original patch for bug 590268.
I believe this is what's blocking me on bug 598795 and bug 594644 (both of which are blockers); when I apply those patches, I get this series of errors in the test for bug 590268: TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 11: extData was correct - Got , expected 12898821010350.44926920051066277 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 10: extData was correct - Got , expected 12898821010350.17031393094726255 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 9: extData was correct - Got , expected 12898821010350.6403366812206732 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 8: extData was correct - Got , expected 12898821010350.4612792467367748 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 7: extData was correct - Got , expected 12898821010350.48433409830006124 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 6: extData was correct - Got , expected 12898821010350.1842668584955346 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 5: extData was correct - Got , expected 12898821010350.5407382056739869 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 4: extData was correct - Got , expected 12898821010350.43950214409426613 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 3: extData was correct - Got , expected 12898821010350.3894383361690794 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 2: extData was correct - Got , expected 12898821010350.7379392756827334 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | tab 1: extData was correct - Got , expected 12898821010350.47807025612296294 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_590268.js | checked the same number of uniq2 as we set - Got 0, expected 11 I believe this is because those patches (specifically bug 594644, but it depends on bug 598795) introduce new behavior in response to session restore, which is probably tickling this bug. Paul, any thoughts on how to fix this bug? If you want to run my patches, I'll have to get you updated versions.
Blocks: 598795, 594644
blocking2.0: --- → ?
I have an idea, will look at it soon. My guess is that if the tab didn't have extData before then we'll set it on the browser but when collecting tab data we take a shortcut and just use __SS_data since nothing should have been done to the tab, so we don't collect the new stuff on the browser. This has come up a couple times now with different attributes, so this might just end up being duped to bug 607016.
I'm just going to fix this over in bug 607016.
Status: NEW → RESOLVED
blocking2.0: ? → ---
Closed: 14 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.