795203 - B2g: clear private data: cookies

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Jason Duell]

Attachment: v1

This just switches the cookieSvc to use the "webapps-clear-data" instead of 'webapps-uninstall': the former provides support for both clear data and uninstall (i.e. provides 'browserOnly' flag).

Relies on the NS_GetAppInfoFromClearDataNotification nsNetUtil.h function I'm adding in bug 786299.

Pretty simple patch--not sure if it really needs a cookie peer, honestly.  Just hooking things up to external plumbing...

Comment 1

[Mounir Lamouri (:mounir)]

Comment on attachment 665760 [details] [diff] [review]
v1

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

This is indeed pretty simple. But could you rename AppUninstallObserver to something like ClearAppDataObserver?

::: netwerk/cookie/nsCookieService.cpp
@@ +557,3 @@
>  
>      uint32_t appId;
> +    bool browserOnly = false;

If you initialize browserOnly, you can do that for appId too ;)

@@ +562,5 @@
> +    NS_ENSURE_SUCCESS(rv, rv);
> +
> +    nsCOMPtr<nsICookieManager2> cookieManager
> +      = do_GetService(NS_COOKIEMANAGER_CONTRACTID);
> +    if (cookieManager) {

!cookieManager should not happen. You should at least assert if that happens to be the case.

Comment 2

[Mounir Lamouri (:mounir)]

BTW, we need tests for that.

Comment 3

[Andrew Overholt [:overholt]]

Clear private data needs to clear cookies.

Comment 4

[Jason Duell]

Attachment: v2: w/mounir's fixes

As mentioned before, mostly external plumbing, so not sure we need full review from cookie peer.  Up to mconnor...

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 666122 [details] [diff] [review]
v2: w/mounir's fixes

lgtm, sorry for delay here.

Comment 6

[Jason Duell]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29cb1f0d796c

Comment 7

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/29cb1f0d796c

Comment 8

[Mounir Lamouri (:mounir)]

Jason, this feature should have tests. Do you think you can find cycles to write them?

Comment 9

[Jason Duell]

I've filed bug 799212 for the test.