Remove browser.__SS_data

RESOLVED FIXED in Firefox 41

Status

()

Firefox
Session Restore
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: ttaubert, Assigned: ttaubert)

Tracking

Trunk
Firefox 41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

Looked at what browser.__SS_data is still required for and it's only the tab icon and browser.userTyped{Value,Clear} that we need for when the tab is pending or has been restored. We should simply put those in the TabStateCache.
Created attachment 8608136 [details] [diff] [review]
0001-Bug-1166757-Remove-browser.__SS_data.patch
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #8608136 - Flags: review?(wmccloskey)
Comment on attachment 8608136 [details] [diff] [review]
0001-Bug-1166757-Remove-browser.__SS_data.patch

Review of attachment 8608136 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this looks great. Since we restore it so early, I guess all that complexity with __SS_extdata was unnecessary even without the changes in this patch? Nice to see it gone.

::: browser/components/sessionstore/TabState.jsm
@@ +183,3 @@
>      this.copyFromCache(tab, tabData, options);
>  
> +    // Store the tab icon.

The code after copyFromCache could use a comment explaining why we do it conditionally. It's explained in SessionStore.jsm, but someone might not have seen that when looking at this code.
Attachment #8608136 - Flags: review?(wmccloskey) → review+
(In reply to Bill McCloskey (:billm) from comment #2)
> Thanks, this looks great. Since we restore it so early, I guess all that
> complexity with __SS_extdata was unnecessary even without the changes in
> this patch? Nice to see it gone.

Yeah, not sure when we got to that state or if it was ever needed but there it goes.

> ::: browser/components/sessionstore/TabState.jsm
> @@ +183,3 @@
> >      this.copyFromCache(tab, tabData, options);
> >  
> > +    // Store the tab icon.
> 
> The code after copyFromCache could use a comment explaining why we do it
> conditionally. It's explained in SessionStore.jsm, but someone might not
> have seen that when looking at this code.

Will do, thanks!

Comment 4

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/4ad0782c686c

Comment 5

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/5fb95b4c1d74
Blocks: 1139986
https://hg.mozilla.org/mozilla-central/rev/4ad0782c686c
https://hg.mozilla.org/mozilla-central/rev/5fb95b4c1d74
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Depends on: 1170226
Depends on: 1173857

Updated

a year ago
Depends on: 1235368
You need to log in before you can comment on or make changes to this bug.