Bug 912717 moved the list of cookies from state.windows[x].cookies to state.cookies. We forgot to update the SessionSaver that purges cookies on shutdown, when requested by the user's preferences.

Comment on attachment 8861339 [details] [diff] [review] 0001-Bug-1359344-Properly-clear-cookies-on-clean-shutdown.patch Review of attachment 8861339 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for catching this in time! I should've noticed it as well :/ LGTM! Oh, btw, this should've been caught by a unit test. Can you do that here or file a bug, blocking this one, that one should be written at some point? I'm thinking of something simple that makes sure that 1) cookies are in the JSON when prefs are in default state and not there when clear-on-shutdown is enabled.

(In reply to Mike de Boer [:mikedeboer] from comment #2) > Oh, btw, this should've been caught by a unit test. Can you do that here or > file a bug, blocking this one, that one should be written at some point? I'm > thinking of something simple that makes sure that 1) cookies are in the JSON > when prefs are in default state and not there when clear-on-shutdown is > enabled. Yeah, we should have a test for that. The reason we didn't add one back then is simply that we need to actually shutdown the browser. I'll file another bug, shouldn't be too hard to extend the marionette stuff to make this work.

(In reply to Tim Taubert [:ttaubert] from comment #3) > Yeah, we should have a test for that. The reason we didn't add one back then > is simply that we need to actually shutdown the browser. I'll file another > bug, shouldn't be too hard to extend the marionette stuff to make this work. Ugh, can't we emulate/ mock the properties in RunState.jsm? Or, like mock all the things so we don't need to go full-marionette?

(In reply to Mike de Boer [:mikedeboer] from comment #4) > Ugh, can't we emulate/ mock the properties in RunState.jsm? Or, like mock > all the things so we don't need to go full-marionette? We could probably do that somehow, but having real tests would be a lot nicer, no?

(In reply to Tim Taubert [:ttaubert] from comment #5) > We could probably do that somehow, but having real tests would be a lot > nicer, no? Of course.

Pushed by ttaubert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3894e3c101a8 Properly clear cookies on clean shutdown if requested by the user r=mikedeboer