1042745 - Expose network enable/disable for specific apps to Gaia

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[Marshall Culpepper [:marshall_law]]

To enable basic control of data usage per app, we need to expose an API from Gecko that allows the settings app to toggle network connectivity for a specific app.

We'll need the necko support in Bug 786419 to land first before we can expose this as a WebAPI

Comment 1

[Andrew Overholt [:overholt]]

Are we just exposing something to the settings app?

Comment 2

[(no longer active)]

Please post something to dev-webapi when you know more about the details here, but I think this should be a straightforward addition to the mozApps API.

Comment 3

[Marshall Culpepper [:marshall_law]]

(In reply to Andrew Overholt [:overholt] from comment #1)
> Are we just exposing something to the settings app?

The bug wording was a little hasty, it turns out most of this will be used from settings within costcontrol.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Please note that ioservice.setAppOffline(appId, Ci.nsIAppOfflineInfo.OFFLINE) cannot be called from the content process - it would return NS_ERROR_FAILURE.
The mozApps API should send the command to the parent process, and run it there.

Comment 5

[Marshall Culpepper [:marshall_law]]

Attachment: enableMobileData - v1

This is just the first implementation and API, looking for a little feedback.
I'm in the process of writing mochitests that cover the new APIs.

Comment 6

[Marshall Culpepper [:marshall_law]]

I forgot to mention, this is based on Val's current series of patches in Bug 786419

Comment 7

[(no longer active)]

Comment on attachment 8483739 [details] [diff] [review]
enableMobileData - v1

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

The IDL changes look good!  Note that you'd need to get a DOM peer review on those bits.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8483739 [details] [diff] [review]
enableMobileData - v1

Looks good.

We might want to consider if at some point in the future, we might want to set an app offline, even on Wifi. But even if that's the case, we'd probably have another interface that does that.

Comment 9

[Hema Koka [:hema]]

Marshall, 

How far are we to get this wrapped up? Can you work with the necko team and get an ETA for the dependency and closure of this feature?

Thanks
Hema

Comment 10

[Valentin Gosu [:valentin] (he/him)]

The necko bits are close to landing. We're waiting on Ben Turner to review a final patch.

Comment 11

[Hema Koka [:hema]]

(In reply to Valentin Gosu [:valentin] from comment #10)
> The necko bits are close to landing. We're waiting on Ben Turner to review a
> final patch.

Thanks Val for the update! We would like to wrap this up in this sprint, its been open for a while now. 

Thanks!
Hema

Comment 12

[Valentin Gosu [:valentin] (he/him)]

The patch just got an r-.
I'm looking into fixing the issue really quickly, or considering landing the patch without the ServiceWorker bits, as the implementation has taken way more than expected.

Comment 13

[Hema Koka [:hema]]

(In reply to Valentin Gosu [:valentin] from comment #12)
> The patch just got an r-.
> I'm looking into fixing the issue really quickly, or considering landing the
> patch without the ServiceWorker bits, as the implementation has taken way
> more than expected.

Val, any update? We have been holding up work on this bug (chained dependency) because of dependent bug: https://bugzilla.mozilla.org/show_bug.cgi?id=786419 -- its been several weeks. Can we please get a definitive ETA on when we you can wrap up the dependency, so we can get a closure on overall feature (https://bugzilla.mozilla.org/show_bug.cgi?id=1035553)

Comment 14

[Valentin Gosu [:valentin] (he/him)]

I just got a final review, and pushed the changes to inbound. Assuming it sticks, we can close the bug in the next few days. Sorry it took so long. It was fairly complicated and we uncovered a few bugs after we pushed it the first time.

Comment 15

[Hema Koka [:hema]]

(In reply to Valentin Gosu [:valentin] from comment #14)
> I just got a final review, and pushed the changes to inbound. Assuming it
> sticks, we can close the bug in the next few days. Sorry it took so long. It
> was fairly complicated and we uncovered a few bugs after we pushed it the
> first time.

thanks so much for the update!

Comment 16

[Jason Duell]

If we want to be able to block the Browser app specifically (and/or Everything.me apps), then we need to figure out a story as to how to distinguish them (since they all show up right now as {appID=[System app ID, i.e. 22], inbrowser=true}.  There's been talk of using the manifest for this but no conclusion yet (see bug 1060108 and bug 1070944).

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'd like to enable apps to have at least the following policies:

* No internet connectivity
* Full internet connectivity
* Wifi only (i.e. no mobile data)
* No background data
* Wifi only when in background (i.e. no mobile data when in background).

"background" here is defined as "not foreground app". This is something that currently only the system app knows, but I think we'll need to start tracking that in Gecko as well. Likely by having the system app tell Gecko which app/apps are currently "in foreground".

Comment 18

[[:fabrice] Fabrice Desré]

gecko knows which app is in the background or foreground, since we rely on that to set process priority for instance.

Comment 19

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jonas Sicking (:sicking) from comment #17)
> I'd like to enable apps to have at least the following policies:
> 
> * No internet connectivity
> * Full internet connectivity
> * Wifi only (i.e. no mobile data)
> * No background data
> * Wifi only when in background (i.e. no mobile data when in background).

The first 3 are supported by the API implemented in Bug 786419. To get the other ones, we could either have gaia set the apps offline/wifi-only when they go to the background, or consider filing another bug to add those states to nsIAppOfflineInfo.

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Either of those are fine. My thinking was definitely that Gaia can take responsibility for handling the background/foreground transition.

Mainly I was concerned that if we expose App.mobileDataEnabled, then it seems weird that it would fluctuate back and forth as an app transitions between forground and background.

Comment 21

[Mossroy]

If I understood correctly, this API is intended to let the user go in the Settings, and choose from here which applications should have network enabled or disabled.

Would there be a way for the application itself to set it? (probably with a user confirmation)
I would like, from within an application (it's a privileged one), to let the user enable/disable any network access for this app.

If it's not possible, would there be a way for the app to simply know its own status (network enabled or disabled)? And a way to redirect the user to the corresponding Settings page, so that he might change this status?

Comment 22

[Valentin Gosu [:valentin] (he/him)]

(In reply to Mossroy from comment #21)
> 
> If it's not possible, would there be a way for the app to simply know its
> own status (network enabled or disabled)? And a way to redirect the user to
> the corresponding Settings page, so that he might change this status?

Not sure about the previous questions, but the app can always check navigator.onLine to see if it has connectivity. Indeed, it might be more useful to show a "This app has restricted web access" instead of the regular "You are offline" message.

Comment 23

[Julien Wajsberg [:julienw]]

hey Marshall, Valentin, we'd like to know the status for this bug. Alternatively, we (the gaia team) could maybe help wrapping this up if all dependency landed. I worked in Webapps some time ago :)

Comment 24

[Valentin Gosu [:valentin] (he/him)]

Hi Julien, the necko API is in place, and bug 1060108 isn't specifically blocking this one.
I don't see any problem in landing this as soon as possible.

Comment 25

[Jacob Shen]

Valentin, that sounds really good.  Can you then provide us a date this will be landed so we can track to it?  Much appreciated.

Comment 26

[Valentin Gosu [:valentin] (he/him)]

I think Marshall should be able to provide an ETA for this.

Comment 27

[Marshall Culpepper [:marshall_law]]

I'm working on finishing up the current rev of this patch and having it up for review late tonight, and hope to have it wrapped up early next week

Comment 28

[Marshall Culpepper [:marshall_law]]

Attachment: Part 1: Apps.webidl changes

Comment 29

[Marshall Culpepper [:marshall_law]]

Attachment: Part 2: setMobileDataEnabled implementation

Comment 30

[Marshall Culpepper [:marshall_law]]

Attachment: Part 3: Mochitests

Comment 31

[[:fabrice] Fabrice Desré]

Comment on attachment 8531763 [details] [diff] [review]
Part 1: Apps.webidl changes

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

When we added the "enabled" attribute, we did it slightly differently:

readonly attribute boolean enabled;
void setEnabled(DOMApplication app, boolean state);	
attribute EventHandler onenabledstatechange;

The nice thing with this approach is that if appA (let's say settings) is changing the state of appB, any other app with access to mozApps.mgmt will be notified of the status change, while you would have to poll with your current design. So for instance the homescreen could listen to this event and tweak the icon based on this new state.

Comment 32

[Chris Lord [:cwiiis]]

(In reply to Fabrice Desré [:fabrice] from comment #31)
> Comment on attachment 8531763 [details] [diff] [review]
> Part 1: Apps.webidl changes
> 
> Review of attachment 8531763 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> When we added the "enabled" attribute, we did it slightly differently:
> 
> readonly attribute boolean enabled;
> void setEnabled(DOMApplication app, boolean state);	
> attribute EventHandler onenabledstatechange;
> 
> The nice thing with this approach is that if appA (let's say settings) is
> changing the state of appB, any other app with access to mozApps.mgmt will
> be notified of the status change, while you would have to poll with your
> current design. So for instance the homescreen could listen to this event
> and tweak the icon based on this new state.

This would be desirable for the data-restricted error page as well, in case a user has an app with the error page visible in the background, we can automatically refresh it (or rather, remove the error page and refresh it when it gets foregrounded).

Comment 33

[(no longer active)]

Comment on attachment 8531763 [details] [diff] [review]
Part 1: Apps.webidl changes

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

::: dom/webidl/Apps.webidl
@@ +98,5 @@
>    Promise<DOMApplication> import(Blob blob);
>    Promise<any> extractManifest(Blob blob);
>  
>    void setEnabled(DOMApplication app, boolean state);
> +  DOMRequest setMobileDataEnabled(DOMApplication app, boolean state);

What Fabrice said.  Also, I think enableMobileData/disableMobileData methods are better as they make things more explicit at the callsite.

Comment 34

[Jacob Shen]

[Tracking Requested - why for this release]:

Comment 35

[Justin D'Arcangelo [:justindarc]]

Bulk-clearing old NI? requests. If this still needs my attention, please re-flag me. Thanks.

Comment 36

[Valentin Gosu [:valentin] (he/him)]

Closing bug as b2g code is getting removed (bug 1306391).