nsCookieService listens for non-existent shutdown-cleanse with profile-before-change

RESOLVED DUPLICATE of bug 820613

Status

()

RESOLVED DUPLICATE of bug 820613
6 years ago
6 years ago

People

(Reporter: froydnj, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
http://dxr.mozilla.org/mozilla-central/netwerk/cookie/nsCookieService.cpp#l1448

checks for "shutdown-cleanse" as the data sent along with "profile-before-change".  But:

http://dxr.mozilla.org/mozilla-central/profile/public/notifications.txt#l59

indicates that "shutdown-persist" is the new hotness and we should be using that instead.

Unsure if this is the right component for it, but we also have:

http://dxr.mozilla.org/mozilla-central/extensions/cookie/nsPermissionManager.cpp#l1181
http://dxr.mozilla.org/mozilla-central/extensions/cookie/test/unit/test_cookies_thirdparty_session.js#l56
http://dxr.mozilla.org/mozilla-central/extensions/cookie/test/unit/test_cookies_persistence.js#l64

which should be fixed as well (different bug?  not sure how the cookie responsibilities divvy up here).
(Reporter)

Comment 1

6 years ago
Created attachment 719971 [details] [diff] [review]
remove shutdown-cleanse support from nsCookieService

Removing shutdown-cleanse support here seems like the right thing to do.
Attachment #719971 - Flags: review?(ehsan)
(Reporter)

Comment 2

6 years ago
Created attachment 719972 [details] [diff] [review]
remove shutdown-cleanse support from extensions/cookie/

Here too.
Attachment #719972 - Flags: review?(ehsan)

Updated

6 years ago
Attachment #719971 - Flags: review?(ehsan) → review+

Comment 3

6 years ago
Comment on attachment 719972 [details] [diff] [review]
remove shutdown-cleanse support from extensions/cookie/

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

::: extensions/cookie/test/unit/test_cookies_persistence.js
@@ +60,5 @@
>    do_check_eq(Services.cookies.countCookiesFromHost(uri1.host), 4);
>    do_check_eq(Services.cookies.countCookiesFromHost(uri2.host), 0);
>  
>    // cleanse them
> +  do_close_profile(test_generator, "shutdown-persist");

Why are you sending shutdown-persist as opposed to nothing?
(Reporter)

Comment 4

6 years ago
(In reply to :Ehsan Akhgari from comment #3)
> Comment on attachment 719972 [details] [diff] [review]
> remove shutdown-cleanse support from extensions/cookie/
> 
> Review of attachment 719972 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: extensions/cookie/test/unit/test_cookies_persistence.js
> @@ +60,5 @@
> >    do_check_eq(Services.cookies.countCookiesFromHost(uri1.host), 4);
> >    do_check_eq(Services.cookies.countCookiesFromHost(uri2.host), 0);
> >  
> >    // cleanse them
> > +  do_close_profile(test_generator, "shutdown-persist");
> 
> Why are you sending shutdown-persist as opposed to nothing?

Because I failed to look at do_close_profile carefully enough.  It looks like you can just omit the second argument:

http://dxr.mozilla.org/mozilla-central/extensions/cookie/test/unit/head_cookies.js#l87

and indeed, all the other tests do just that.  Patch OK with "shutdown-persist" removed?  And then OK follow-on patch to remove the second argument from do_close_profile?
(Reporter)

Comment 5

6 years ago
Comment on attachment 719972 [details] [diff] [review]
remove shutdown-cleanse support from extensions/cookie/

Actually, never mind.  The tests require more tweaking than just mindlessly removing stuff.
Attachment #719972 - Flags: review?(ehsan)
(Reporter)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 820613
You need to log in before you can comment on or make changes to this bug.