[Session Restore] Remove window.__SS_dyingCache and replace it with some appropriate use of WeakMap

RESOLVED FIXED in Firefox 23

Status

()

Firefox
Session Restore
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Yoric, Assigned: ttaubert)

Tracking

(Blocks: 2 bugs, {addon-compat})

Trunk
Firefox 23
addon-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

window.__SS_dyingCache seems to be used to retain the state of a Window between the time it is closed and the time it is actually removed. Not only does this complicate the window objects, but I have seen leaks causes by this.

I believe that we should replace this by an appropriate use of WeakMap.
(Assignee)

Comment 1

5 years ago
For reference, __SS_dyingCache was implemented here (no bug #):

http://hg.mozilla.org/mozilla-central/rev/12eda8295609
(Assignee)

Comment 2

5 years ago
I commented out the line setting the __SS_dyingCache property and no tests failed, of course :/
(Assignee)

Comment 3

5 years ago
Created attachment 744806 [details] [diff] [review]
remove window.__SS_dyingCache and replace it with a WeakMap

Wrote a test \o/
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attachment #744806 - Flags: review?(dteller)
(In reply to Tim Taubert [:ttaubert] from comment #1)
> For reference, __SS_dyingCache was implemented here (no bug #):
> 
> http://hg.mozilla.org/mozilla-central/rev/12eda8295609

Bug 360408, fwiw.
Comment on attachment 744806 [details] [diff] [review]
remove window.__SS_dyingCache and replace it with a WeakMap

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

Looks good. The fight for cleaning up Session Restore is progressing!

::: browser/components/sessionstore/test/browser_dying_cache.js
@@ +5,5 @@
> +  TestRunner.run();
> +}
> +
> +/**
> + * This test ensures that after closing a window we keep a its state data around

Nit: "we keep its state"
Attachment #744806 - Flags: review?(dteller) → review+
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/253e46507c57
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/253e46507c57
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23

Updated

5 years ago
Blocks: 869900
Blocks: 874381
Keywords: addon-compat
You need to log in before you can comment on or make changes to this bug.