In gaia we need the ability to "clear private data" for web content inside the browser app and "clear app data" for an app from the settings app. See: https://github.com/mozilla-b2g/gaia/issues/1234 https://github.com/mozilla-b2g/gaia/issues/3124 for extensive discussions.

Marking blocking -- However, I would prioritize "clear private data" inside the browser app over "clear app data' per app. Both are important, but wanted to call this out if we had to prioritize.

Justin offered to own this work. I've marked Mounir's top-level "clear data jars" bug as a blocker of this bug.

Is this about clearing *all* data that the browser app is holding or only data that "tabs" own? I would tend to say it's the later. If that the case, it's not really related to bug 786293 which is about wiping data for an app. That bug would be more complex.

> If that the case, it's not really related to bug 786293 which is about wiping data for an > app. That bug would be more complex. I was thinking that clearing a browser tab's data is just like clearing an app's data. Both a browser tab and an app have a storage context identified by (app-id, true/false). When we clear an app, we clear the storage for (app-id, false). When we clear a browser tab, we clear the storage for (app-id, true). No?

Mounir: To be clear, in Gaia's UX specs there are two requirements: 1) Inside the browser app settings, the ability to clear private data stored by all of the web content which has been accessed from inside the browser (but not the data stored by the browser app itself). 2) Inside the settings app, the ability to clear private data stored by a particular app, the apps are listed and there's a "Clear app data" button in the permissions panel for each app.

Guessing LOE:M, although that's just for this bug -- the blockers (bug 786293) aren't counted in that estimate.

Jonas said we can use the Settings app instead for now so removing blocker.

Ben Francis helpfully clarified on IRC that this bug is essentially 2 things: a) adding the platform support to allow the Settings app to be able to clear the bug and b) the actual API the title refers to. I'm going to make this bug about adding the support and I've filed bug 791307 for the API itself.

Sorry if I didn't explain myself clearly but that isn't what I meant. The requirements here are: 1) Provide an API to clear the data stored by a particular app 2) Provide an API to clear the data stored by web content loaded inside an app 1 will be used by the settings app to clear app data for particular apps, 2 will be used by the browser app to clear private data stored by web content. Justin provided an estimate of LOE:M for this bug, but that refers only to the work to hook the browser API up to existing functionality (for both requirements 1 and 2), which will be implemented elsewhere, specifically in all the bugs which are marked as blocking bug 786293 Does that make sense?

I've put the bug back how it was to prevent further confusion

Stealing.

Thanks for taking this Ben! This bug potentially covers clearing quite a few different things (cookies, IndexedDB, localstorage, browser cache, appcache...) in two different contexts (the data jar of an app, or the data jar which contains all the web content loaded inside an app). Kev has clarified that the most essential requirement for the browser app is to be able to clear cookies for all the web content loaded inside the browser. The cache and "state" (not sure what that means) would come next. Would you prefer to split the highest priority work into a separate bug with smaller scope and less dependencies, or are you happy to tackle this all at once, in time for the feature freeze? The sooner you can give me an idea of what the API will look like, the sooner I can hook it up to the browser app. Thanks!

To address requirement #2 from comment 9, the API will look like this: interface mozIDOMApplication { // ... void clearBrowserData([optional] in DOMString domain); } So to use this you'd just do something like this: navigator.mozApps.getSelf().clearBrowserData(); I'm not sure if we care about requirement #1... I talked with sicking about this and we're not sure this can actually be done without just uninstalling the app. It's likely out of scope for this bug in any case. Does this sound ok?

I'm confused. Requirement #2 from comment 9 is "Provide an API to clear the data stored by web content loaded inside an app". This is the API the browser app needs to clear private data stored for e.g. Google.com when the user loads that web page. If it is to be part of the Apps API (which sounds reasonable) I would expect this to look something like navigator.mozApps.getSelf().clearBrowserData(); but you say this is for requirement #1 Requirement #1 from comment 9 is "Provide an API to clear the data stored by a particular app". This is the API the settings app needs to clear data stored by a particular app (e.g. the browser stores bookmarks in IndexedDB and the settings app would be able to clear this data). Both of these requirements are currently within the scope of this bug and are blocking https://github.com/mozilla-b2g/gaia/issues/3124 and https://github.com/mozilla-b2g/gaia/issues/1234 respectively. Could you clarify which of these you're working on, and move the parts you're not working on out to separate bug so it can be tracked separately if that's what you'd prefer? Thanks!

> Could you clarify which of these you're working on, and move the parts you're not working on out to > separate bug so it can be tracked separately if that's what you'd prefer? We have so many different bugs on this, I cannot keep them straight. At this point, I personally would prefer to morph this bug into whatever it is that bent is doing. We do not gain much by continuing to move this work around into different bugs. > but you say this is for requirement #1 That's not how I read comment 14. I read comment 14 as saying "this is my API for requirement #2, and I'm not sure if we care about requirement #1; it may not be possible, and is likely out of scope."

(In reply to Ben Francis [:benfrancis] from comment #15) > but you say this is for requirement #1 Nope, I said that that API will address requirement #2. > Requirement #1 from comment 9 is "Provide an API to clear the data stored by > a particular app". This is the API the settings app needs to clear data > stored by a particular app (e.g. the browser stores bookmarks in IndexedDB > and the settings app would be able to clear this data). Right. I'm not convinced that this can be done generically without just uninstalling the app, but even setting aside my doubts, this should be a different bug. Let's let this bug track the platform side of requirement #2 (https://github.com/mozilla-b2g/gaia/issues/3124) and get something else going for requirement #1 (https://github.com/mozilla-b2g/gaia/issues/1234).

(In reply to ben turner [:bent] from comment #17) > get something else going for requirement #1 Bug 792892

Justin you're right, I got confused, sorry. Great, so this bug is about Clear Private Data which blocks https://github.com/mozilla-b2g/gaia/issues/3124 and Bug 792892 is about Clear App Data (I will rename), which blocks https://github.com/mozilla-b2g/gaia/issues/1234 Thank you :) Now, if this is going to be part of the Apps API rather than the Browser API (which sounds perfectly reasonable), then does this mean the browser app will need an additional permission or will it always work via getSelf() without permissions? Also if it is part of the apps API, we should rename the titles.

Actually, the interface is not going to mess with domains at the moment: interface mozIDOMApplication { // ... void clearBrowserData(); }

The internal notification is going to use the observer service. topic: "mozapps-clear-data" subject: mozIApplicationClearPrivateDataParams data: null interface mozIApplicationClearPrivateDataParams { readonly attribute unsigned long appId; readonly attribute boolean browserOnly; };

Here's what consumers of this notification should do: If the browserOnly flag is false, clear all data associated with the specified appID. If the browserOnly flag is true, *only* clear data which has the isInBrowserElement (aka browser-flag) set to true. The appID will never be NO_APP or UNKNOWN_APP.

Notice that this will get you bug 792892 for free. :)

That's not entirely by accident :) This is what we're planning on using for both various "clear private data" as well as for uninstall.

(In reply to ben turner [:bent] from comment #21) > The internal notification is going to use the observer service. > topic: "mozapps-clear-data" Hm, everything else uses "webapps-*" instead, so it's actually going to be "webapps-clear-data".

Not sure who should review this... jlebar, shall we start with you?

Okay, but I probably can't get to this until tomorrow.

It's a pain to make instances of some observer class just to catch this notification. Mounir apparently ran into this already. This is basically the same patch except that I'm using the category manager as well as the observer service to notify components.

> This is basically the same patch except that I'm using the category manager as well as the observer > service to notify components. Can you point me to some code which uses the category manager, so I can see how much better this is? (In general, using two mechanisms for notifying instead of one feels like overkill, particularly when just notifying the category service takes so much boilerplate.)

Comment on attachment 664183 [details] [diff] [review] Patch, v2 >diff --git a/dom/interfaces/apps/nsIDOMApplicationRegistry.idl b/dom/interfaces/apps/nsIDOMApplicationRegistry.idl >--- a/dom/interfaces/apps/nsIDOMApplicationRegistry.idl >+++ b/dom/interfaces/apps/nsIDOMApplicationRegistry.idl >@@ -36,16 +36,19 @@ interface mozIDOMApplication : nsISuppo > /* > * fires a nsIDOMApplicationEvent when a change in appcache download or status happens > */ > attribute nsIDOMEventListener onprogress; > > /* startPoint will be used when several launch_path exists for an app */ > nsIDOMDOMRequest launch([optional] in DOMString startPoint); > nsIDOMDOMRequest uninstall(); >+ >+ /* Clear data that has been collected through mozbrowser elements. */ >+ void clearBrowserData(); > }; Change the UUID. :) >diff --git a/dom/apps/src/Webapps.js b/dom/apps/src/Webapps.js >+ clearBrowserData: function() { >+ let browserChild = >+ BrowserElementPromptService.getBrowserElementChildForWindow(this._window); >+ browserChild.messageManager.sendAsyncMessage("Webapps:ClearBrowserData"); >+ }, Please add a null-check here; not all windows have a BEC. (Particularly outside of b2g, and we share this code with desktop apps, right?) I'd prefer that we moved getBrowserElementChildForWindow out of the "prompt service" and into something else (BrowserElementLookupService?), but if you don't want to do that here, that's fine. Just please file a follow-up that I can take? >diff --git a/dom/apps/src/Webapps.jsm b/dom/apps/src/Webapps.jsm >+ _notifyCategoryAndObservers: function(subject, topic, data) { This is pretty complicated in comparison to observerService.notifyObservers(). I'm willing to be convinced by a demonstration of how much easier this makes life for observers, but another question is, why should this particular notification get special treatment? Doing things differently in one place (i.e., special treatment) just makes our code confusing. >diff --git a/dom/browser-element/BrowserElementChild.js b/dom/browser-element/BrowserElementChild.js >--- a/dom/browser-element/BrowserElementChild.js >+++ b/dom/browser-element/BrowserElementChild.js >@@ -670,16 +670,21 @@ BrowserElementChild.prototype = { > > sendAsyncMsg('securitychange', {state: stateDesc, extendedValidation: isEV}); > }, > > onStatusChange: function(webProgress, request, status, message) {}, > onProgressChange: function(webProgress, request, curSelfProgress, > maxSelfProgress, curTotalProgress, maxTotalProgress) {}, > }, >+ >+ // Expose the message manager for WebApps and others. >+ get messageManager() { >+ return global; Can we just return {sendAsyncMessage: global.sendAsyncMessage, sendSyncMessage: global.sendSyncMessage, addMessageListener: global.addMessageListener} instead of the whole global? >diff --git a/dom/browser-element/BrowserElementParent.js b/dom/browser-element/BrowserElementParent.js >--- a/dom/browser-element/BrowserElementParent.js >+++ b/dom/browser-element/BrowserElementParent.js >@@ -8,16 +8,21 @@ let Cu = Components.utils; > let Ci = Components.interfaces; > let Cc = Components.classes; > let Cr = Components.results; > > Cu.import("resource://gre/modules/Services.jsm"); > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > Cu.import("resource://gre/modules/BrowserElementPromptService.jsm"); > >+XPCOMUtils.defineLazyGetter(this, "DOMApplicationRegistry", function () { >+ Cu.import("resource://gre/modules/Webapps.jsm"); >+ return DOMApplicationRegistry; >+}); >+ > const NS_PREFBRANCH_PREFCHANGE_TOPIC_ID = "nsPref:changed"; > const BROWSER_FRAMES_ENABLED_PREF = "dom.mozBrowserFramesEnabled"; > const TOUCH_EVENTS_ENABLED_PREF = "dom.w3c_touch_events.enabled"; > > function debug(msg) { > //dump("BrowserElementParent - " + msg + "\n"); > } > >@@ -245,16 +250,27 @@ function BrowserElementParent(frameLoade > // down to the child. > this._window.addEventListener('mozvisibilitychange', > this._ownerVisibilityChange.bind(this), > /* useCapture = */ false, > /* wantsUntrusted = */ false); > > // Insert ourself into the prompt service. > BrowserElementPromptService.mapFrameToBrowserElementParent(this._frameElement, this); >+ >+ // If this browser represents an app then let the Webapps module register for >+ // any messages that it needs. >+ let appManifestURL = this._frameElement.getAttribute('mozapp'); Please do nsIMozBrowserFrame.appManifestURL; that will (soon) do the appropriate permission check and return the empty string if this frame doesn't have permission to be an app. I'd like to see the updated code if you decide to move the code out of BrowserElementPromptService. If you decide not to do that, feel free to reset r? without updating the patch once you show me how the category service listener is registered.

Bent: This isn't yet sending the notification during app uninstall, is it? Would you mind adding that in this bug so that people don't have to hook up listening to both webapp uninstall and clear-private-data for now.

(In reply to Justin Lebar [:jlebar] from comment #29) > Can you point me to some code which uses the category manager, so I can see > how much better this is? (In general, using two mechanisms for notifying > instead of one feels like overkill, particularly when just notifying the > category service takes so much boilerplate.) I think we should get rid of the observerservice notification and *just* use the category manager.

We use the observer service /everywhere/ in Gecko. Are you suggesting we should deprecate it, or that this situation is somehow different from the other situations where we use the observer service? I'd really like to see some listener code so I can better understand what you're trying to accomplish here.

Comment on attachment 664183 [details] [diff] [review] Patch, v2 Ok, chatted on irc, I think we're on the same page here now!

Filed bug 794255 to track the refactoring.

Comment on attachment 664183 [details] [diff] [review] Patch, v2 Can we make a deal to rip out the category manager code if nobody is using it in a month?

(In reply to Justin Lebar [:jlebar] from comment #36) > Can we make a deal to rip out the category manager code if nobody is using > it in a month? Sure! (But I'm planning on using it for bug 786295 this week ;) )

Here are the changes I've made based on your comments.

(In reply to Jonas Sicking (:sicking) from comment #31) > Bent: This isn't yet sending the notification during app uninstall, is it? > Would you mind adding that in this bug so that people don't have to hook up > listening to both webapp uninstall and clear-private-data for now. Bug 784378.

(In reply to ben turner [:bent] from comment #39) > Bug 784378. Er, bug 794563.

Comment on attachment 665051 [details] [diff] [review] Review changes These changes look great.

Grr, forgot to 'hg add'... https://hg.mozilla.org/integration/mozilla-inbound/rev/7771d1d6f0d8

Comment on attachment 665078 [details] [diff] [review] Final patch Review of attachment 665078 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/interfaces/apps/mozIApplicationClearPrivateDataParams.idl @@ +7,5 @@ > + > +#include "nsISupports.idl" > + > +[scriptable, uuid(ba0e6c8e-8c03-4b9b-8f9b-4fb14216f56e)] > +interface mozIApplicationClearPrivateDataParams This needs to inherit from nsISupports.

A couple of questions about using this API: 1) It looks like there's no callback of any kind to notify content when the clearing operation has completed, is this correct? The reason that I ask is that the UX spec requires the button to be disabled once the operation is complete to indicate there is now no data to clear (https://www.dropbox.com/sh/ygwfxk6chpshxdj/w_SAwiqXhu/Apps/Browser/R1_Browser-Settings_v4.pdf). I can fake this by just immediately disabling the button until this screen is next displayed, but I just want to make sure. 2) What types of data will be cleared by this API in v1? The prompt in the UI mockups say "Saved passwords and cookies will be deleted." which I'm guessing isn't entirely accurate? Thanks!

(In reply to Ben Francis [:benfrancis] from comment #46) > 1) It looks like there's no callback of any kind to notify content when the > clearing operation has completed, is this correct? The reason that I ask is > that the UX spec requires the button to be disabled once the operation is > complete to indicate there is now no data to clear > (https://www.dropbox.com/sh/ygwfxk6chpshxdj/w_SAwiqXhu/Apps/Browser/ > R1_Browser-Settings_v4.pdf). I can fake this by just immediately disabling > the button until this screen is next displayed, but I just want to make sure. You should do that, indeed. > 2) What types of data will be cleared by this API in v1? The prompt in the > UI mockups say "Saved passwords and cookies will be deleted." which I'm > guessing isn't entirely accurate? What *might* be deleted is: localstorage, app cache, cookies and IndexedDB. Saved passwords will not be deleted. Actually, is B2G saving passwords?

(In reply to Mounir Lamouri (:mounir) from comment #47) > What *might* be deleted is: localstorage, app cache, cookies and IndexedDB. OK, thanks. > Saved passwords will not be deleted. Actually, is B2G saving passwords? Not as far as I know.

https://hg.mozilla.org/mozilla-central/rev/b5d656109b8a https://hg.mozilla.org/mozilla-central/rev/7771d1d6f0d8

w00t! Thanks guys!

(In reply to Jason Duell (:jduell) from comment #45) > This needs to inherit from nsISupports. Bug 794973.

Awesome work, thanks everyone.