Closed Bug 671517 Opened 9 years ago Closed 9 years ago

Port relevant bits of Bug 588506 [nsSessionStartup is keeping restored session in memory]

Categories

(SeaMonkey :: Session Restore, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: misak.bugzilla, Assigned: misak.bugzilla)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
From parent bug:

Errrr, this isn't so awesome.

If you crashed & can resume, or will be restoring (sessionstore.resume_session_once || startup.page == 3) then we keep your entire session in memory for the entirety of your session, even if you go into PB mode.

This might actually be useful for bug 588482, but in general it's a bad thing to do. For those people who always restore and/or have multi-megabyte sessions (I'm looking at you Wayne), this is automatically devouring that memory.

So we can do this a couple ways, which will depend on how I go about bug 588482. We'll either want to add a method that wipes, or autowipe after a certain notification ("sessionstore-windows-restored" would make sense right now, but that might have to change soon).

I wouldn't block on this until we figure out what's going to happen elsewhere. But if we can do it, we might as well. I'm raising the blocking idea just because it might require API changes. We've been like this for many releases (since session store landed?) so I guess it's not super pressing...
Attachment #545869 - Flags: review?(neil)
Comment on attachment 545869 [details] [diff] [review]
patch

>       Services.obs.removeObserver(this, "sessionstore-windows-restored");
Please remove this line instead...

>-      // no need in repeating this, since session type won't change
>-      Services.obs.removeObserver(this, "sessionstore-windows-restored");
...so that you can keep the comment too.
Attachment #545869 - Flags: review?(neil) → review+
Pished with nit:

http://hg.mozilla.org/comm-central/rev/8bb302ea0895
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 545869 [details] [diff] [review]
patch

We need to land this on comm-aurora too to fix regression, see https://bugzilla.mozilla.org/show_bug.cgi?id=527360#c42
Attachment #545869 - Flags: approval-comm-aurora?
Attachment #545869 - Flags: approval-comm-aurora? → approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.