1260360 - Session Restore needs to honor "Clear history when Firefox closes -> Cookies" in a clean shut-down

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Kestrel]

Simple addition to Bug 529899 to include the case where the user has the preference "Clear history when Firefox closes -> Cookies" so that session cookies are cleared in a clean shutdown.

Comment 1

[Kestrel]

Attachment: bug1260360.patch - Purge cookies on clean shutdown with "Clear history when Firefox closes -> Cookies"

(Will need help with a try push if that is necessary)

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8735731 [details] [diff] [review]
bug1260360.patch - Purge cookies on clean shutdown with "Clear history when Firefox closes -> Cookies"

Mike is in charge of this, so deferring.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8735731 [details] [diff] [review]
bug1260360.patch - Purge cookies on clean shutdown with "Clear history when Firefox closes -> Cookies"

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

Thanks for the patch, Kestrel!

::: browser/components/sessionstore/SessionSaver.jsm
@@ +211,5 @@
>        }
>      }
>  
>      // If this is the final write on a clean shutdown, and the user changed
> +    // their cookie preferences to "Keep until I close Firefox" or 

There's some trailing whitespace that should be removed in this patch.

@@ +218,2 @@
>      // cookies when restarting due to a Firefox update.
>      if (RunState.isClosing &&

It's a little difficult to read the logic in here at first glance. To help, what I'd suggest is this:

if (RunState.isClosing) {
  let sessionCookiesOnly = Services.prefs.getIntPref("network.cookie.lifetimePolicy") == Services.cookies.ACCEPT_SESSION;
  let sanitizeCookies = Services.prefs.getBoolPref("privacy.sanitize.sanitizeOnShutdown") &&
                        Services.prefs.getBoolPref("privacy.clearOnShutdown.cookies");
  let willResume = Services.prefs.getBoolPref("browser.sessionstore.resume_session_once");

  if ((sessionCookiesOnly || sanitizeCookies) && !willResume) {
    // Do the deletion
  }
}

Comment 4

[Kestrel]

Attachment: bug1260360.patch v2 - Purge cookies on clean shutdown according to sanitize preference

Good idea, the comment description can also be trimmed down this way.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=27ae8106e06a

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8738063 [details] [diff] [review]
bug1260360.patch v2 - Purge cookies on clean shutdown according to sanitize preference

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

Patch and try looks good! Thanks!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/4209cc859abc

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4209cc859abc