1155758 - Make about:serviceworkers work in B2G

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Attachment: WIP

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

It seems that the approach I was following here is not going to work :(

As you can see in the attached patches, I was trying to load app://system.gaiamobile.org/about_service_workers.html when loading about:serviceworkers from the Browser app. I was hoping to access the ServiceWorkerManager information from the System app via content/chrome event messaging, but even if the about_service_workers.html page is part of the System app, it is executed in the context of the Browser app (as a child iframe) and we cannot communicate with the platform from there (unless we expose ServiceWorkerManager to content, which it's quite unlikely to happen).

So instead of this, I believe that we can show this information on the developers menu within the Settings app. I can still use the System app to access the ServiceWorkerManager information and I can expose an IAC based API to Settings so it can consume this data.

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Screenshot

This is how the settings panel looks

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8596756 [details] [review]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master

Alive, could you review the System app bits, please? Arthur, could you take a look at the Settings app ones, please?
Thanks!

Comment 8

[[:fabrice] Fabrice Desré]

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

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

r=me with nits fixed.

::: b2g/components/AboutServiceWorkers.jsm
@@ +21,5 @@
> +XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
> +                                  "resource://gre/modules/NetUtil.jsm");
> +
> +function debug(aMsg) {
> +  //dump('AboutServiceWorkers - ' + aMsg + '\n');

nit: you started this file with double quotes, which I approve ;) let's go on! (there are other single quotes later too).

@@ +31,5 @@
> +  }
> +
> +  let result = {};
> +
> +  Object.keys(aServiceWorkerInfo).forEach((property) => {

nit: forEach(property => {...

@@ +85,5 @@
> +    let message = aEvent.detail;
> +
> +    debug('Got content event ' + JSON.stringify(message));
> +
> +    if (!message.id || !message.name) {

maybe console.log() a warning?

@@ +101,5 @@
> +        };
> +
> +        let data = gServiceWorkerManager.getAllRegistrations();
> +        if (!data) {
> +          sendError(message.id, "NO_SERVICE_WORKERS_REGISTRATIONS");

nit: can we change the errors to CamelCase instead?

@@ +113,5 @@
> +            enabled: this.enabled,
> +            registrations: registrations
> +          });
> +          return;
> +        }

I think it's fine to remove this block this the next for() loop will just not be entered.

@@ +149,5 @@
> +          return;
> +        }
> +
> +        let principal = Services.scriptSecurityManager.getAppCodebasePrincipal(
> +          NetUtil.newURI(message.principal.origin),

could be Services.io.newURI(message.principal.origin, null, null) so we would not need NetUtil.jsm

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8596756 [details] [review]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master

Looks fine, only naming nits

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Do we need UI review on this? I don't think Service Workers sounds friendly to ordinary users.

Our UI should at least explain these workers might consume data and/or battery at the background, for example.

Comment 11

[Fernando Jiménez Moreno [:ferjm]]

Well, this is a panel inside the developers menu that "ordinary" users are probably not interested in. Do we want to explain all the details and panels inside the developer menu?

We can probably design a "Background Services" or "Running apps" menu to allow users to see which apps and services are running on their device. But that's IMHO out of the scope of this bug. What we want to add here is a way for developers to see which service workers are registered and to allow them to manage them (update, unregister).

Comment 12

[Stephany Wilkes]

Agree with Fernando: "service workers" is acceptable in the Developer menu, I think.

I also agree with Tim: we should explain that service workers might consume data and/or battery in the background. Let me know if UX can help with copy there (ni? me if so), but hopefully we have a string that's already being used for similar warnings (if such exist).

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ah, sorry I didn't realize this is in the developer menu.

Comment 14

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8596756 [details] [review]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master

Thanks for working on this. Please check my comments in github.

Comment 15

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8596756 [details] [review]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master

Thanks for the feedback folks!

Arthur, I just updated the PR with all your comments addressed, except for the one regarding the localization of "Service Workers". This string is not localized on Desktop or Android and I believe it shouldn't also be localized on b2g.

Comment 16

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8596756 [details] [review]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master

r=me with the last comment addressed, thanks!

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ce400f5f5b4

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9ce400f5f5b4

Comment 19

[Fernando Jiménez Moreno [:ferjm]]

https://github.com/mozilla-b2g/gaia/commit/43f1ab4d599c666c74f555e79cd4a1845598e3ec

Comment 20

[Noemí Freire (:noemi)]

Hi,

just adding that while a SW is loaded being Settings->Developer->Service Worker open, the SW won't be shown in settings till closing and opening it again.

STR:
1- Go to Settings->Developer->Service Worker; no SW -> OK
2- Go to Browser and register a SW
3- Go to Settings->Developer->Service Worker; no SW -> I'd expect to see the SW details
4- Close Settings
5- Go to Settings->Developer->Service Worker; SW details are properly shown

Please let me know if a follow up should be open. Thanks!

Comment 21

[Noemí Freire (:noemi)]

I forgot to mention that going back to Settings->Developer and accessing again from there to Service Worker is the same, the info is not refreshed, it is necessary to close and open Settings in order to see the refreshed information (loaded/unloaded... service workers). Thanks!

Comment 22

[Fernando Jiménez Moreno [:ferjm]]

Thanks Noemi. I filed bug 1162948