Bug 1112469 implemented the periodic service worker updates for desktop and Android. This doesn't yet work on b2g since we cannot depend on the app content process to be running when the idle-daily event fires, so we'll need another approach. I don't know how to make some code start the right content process for an app, which is needed to make sure that the SW will get updated in the correct process. Fabrice, can you please help with this, or help us find a good person to work on this? Thanks!

As far as I know, we already have this problem with the Push notification and the RequestSync API. And we will have the same issue with the BackgroundSync API. I think for now we can open the app in background and close it after the update (if the use didn't make it visible). Then we can find a generic solution that can work for all the existing APIs. How does it sound?

Even on b2g you don't necessarily need a new process to run as an app. Just creating an <iframe mozapp=$manifestURL> is enough. I tend to think that not involving gaia would make things simpler, and one option there is to create these iframes in shell.html (either remote or not). Is it enough to load a well-known url to trigger the service worker update? how do we know that the update completed?

(In reply to Fabrice Desré [:fabrice] from comment #2) > Even on b2g you don't necessarily need a new process to run as an app. Just > creating an <iframe mozapp=$manifestURL> is enough. How well does that play with the same app being loaded normally at that time? > I tend to think that not involving gaia would make things simpler, and one > option there is to create these iframes in shell.html (either remote or not). Sounds good to me, if the answer to the above question is yes. > Is it enough to load a well-known url to trigger the service worker update? No. We'd need to do that programmatically. > how do we know that the update completed? Well, it depends on how you do it. For example, if you just use the Web exposed SW API, you can use the updatefound event. See the test for bug 1112469.

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #3) > (In reply to Fabrice Desré [:fabrice] from comment #2) > > Even on b2g you don't necessarily need a new process to run as an app. Just > > creating an <iframe mozapp=$manifestURL> is enough. > > How well does that play with the same app being loaded normally at that time? There should be no difference with "normal" pages, so I think it's fine. But I'm also sure this is unchartered territory ;)

Maybe this is totally fine but, what about if we start the updating of a SW as fabrice is suggesting and then the user opens the same app? I guess we should keep the previous SW until the updating operation is fully completed.

(In reply to Andrea Marchesini (:baku) from comment #5) > Maybe this is totally fine but, what about if we start the updating of a SW > as fabrice is suggesting and then the user opens the same app? I guess we > should keep the previous SW until the updating operation is fully completed. That should be taken care of by the normal SW lifecycle, right?

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #6) > (In reply to Andrea Marchesini (:baku) from comment #5) > > Maybe this is totally fine but, what about if we start the updating of a SW > > as fabrice is suggesting and then the user opens the same app? I guess we > > should keep the previous SW until the updating operation is fully completed. > > That should be taken care of by the normal SW lifecycle, right? Yes

Couple of things: Since the Cache API is used for SW scripts, we can't run the update in the parent process, nor can we run it in an arbitrary child (<browser remote="true"> launches a new child). We *have* to run this in a child that has the same principal/origin/appid/isInBrowser and other identifiers. Unfortunately our process launching and window management code is hairy in both desktop and b2g, so any ideas about how to expose nicer APIs for them are welcome. For example, I have no idea how to launch such a process as I described before, inject a frame script into it and have the child run the ServiceWorkerManager. That said, I'm removing this as a v1 blocker because we aren't shipping b2g any time soon and we shouldn't block shipping SW in 41 due to this.

Let's block then v2 and see how these b2g blocking bugs can be prioritized. Thanks!

(In reply to Nikhil Marathe [:nsm] (back on July 13) from comment #8) > Couple of things: > Since the Cache API is used for SW scripts, we can't run the update in the > parent process, nor can we run it in an arbitrary child (<browser > remote="true"> launches a new child). We *have* to run this in a child that > has the same principal/origin/appid/isInBrowser and other identifiers. > Unfortunately our process launching and window management code is hairy in > both desktop and b2g, so any ideas about how to expose nicer APIs for them > are welcome. For example, I have no idea how to launch such a process as I > described before, inject a frame script into it and have the child run the > ServiceWorkerManager. > I can confirm that this requirements is true. This is actually happening right now with our implementation of about:sw in the Settings app. When we request an update for a sw from the Settings app, it only gets updated when the application owning that sw is running. I'll be fixing this at bug 1181544 > That said, I'm removing this as a v1 blocker because we aren't shipping b2g > any time soon and we shouldn't block shipping SW in 41 due to this.

Andrea, could you take a look at this patch, please? This allows chrome requestSync registrations. The notifications are done via the Observer API instead of SystemMessages as we do for content registrations. I'll be adding a test in another patch. Thanks!

This patch makes use of the RequestSyncService to schedule periodic sw updates on B2G. I still need to test this more and write some comments before asking for revision. Unfortunately, this depends on bug 1181544 which also depends on bug 1182117 to work in all cases (i.e. when the owner of the sw being updated is not opened).

Comment on attachment 8637201 [details] [diff] [review] Part 2: Schedule periodic SW updates Andrea, I could use some feedback about the usage of the RequestSyncService. Thanks!

Comment on attachment 8637819 [details] [diff] [review] Part 3: RequestSyncService test Review of attachment 8637819 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/requestsync/tests/test_request_sync_service_chrome_registration.html @@ +29,5 @@ > + > +function go() { > + let requestSyncMinInterval; > + try { > + requestSyncMinInterval = Services.prefs.getIntPref("dom.requestSync.minInterval"); can you do this: SpecialPowers.pushPrefEnv({"set": [["dom.requestSync.minInterval", 1]]}, function() { .. all the rest... }); @@ +35,5 @@ > + > + ok(RequestSyncService, "RequestSyncService exists"); > + let principal = Services.scriptSecurityManager.getSystemPrincipal(); > + let key = RequestSyncService.principalToKey(principal); > + ok(RequestSyncService._registrations[key] === undefined, is(RequestSyncService._registrations[key] undefined, ... @@ +42,5 @@ > + Services.prefs.setIntPref("dom.requestSync.minInterval", 1); > + > + let observer = (subject, topic, data) => { > + Services.obs.removeObserver(observer, "request-sync-test"); > + ok(topic == "request-sync-test", is @@ +45,5 @@ > + Services.obs.removeObserver(observer, "request-sync-test"); > + ok(topic == "request-sync-test", > + "Observed request-sync-test notification"); > + let dataObj = JSON.parse(data); > + ok(dataObj.data == "data", is

Comment on attachment 8637198 [details] [diff] [review] Part 1: Allow chrome registrations on RequestSyncService Review of attachment 8637198 [details] [diff] [review]: ----------------------------------------------------------------- I would like to see it again with this comments applied. Thanks! ::: dom/requestsync/RequestSyncService.jsm @@ +332,2 @@ > if (!("wakeUpPage" in aParams) || > + aParams.wakeUpPage.length == 0 && why this? @@ +332,3 @@ > if (!("wakeUpPage" in aParams) || > + aParams.wakeUpPage.length == 0 && > + !("observerTopic" in aParams)) { a || b && c is confusing I think you mean: a || (b && c) @@ +355,5 @@ > if (!this.validateRegistrationParams(aData.params)) { > + if (aTarget) { > + aTarget.sendAsyncMessage("RequestSync:Register:Return", > + { requestID: aData.requestID, > + error: "ParamsError" } ); I don't like this use of register. What about if: 1. you rename register, unregister and all the other methods used in 'receiveMessage' like: registerReceived, unregisterReceived, and so on... 2. the exposed API will have 'register' without aTarget 3. register and registerReceived will do 2 different validations and 2 callbacks if success or failure. these 2 callbacks can be used in line 398 and line 405. @@ +668,5 @@ > + > + if (!manifestURL || !pageURL) { > + dump("ERROR!! RequestSyncService - Failed to create URI for the page or the manifest\n"); > + aObj.active = false; > + this.updateObjectInDB(aObj); Moving this block you break the logic because we don't remove the timer. @@ +723,5 @@ > + }, function() { > + debug("promise rejected"); > + taskCompleted(); > + }); > + } else if (aObj.data.observerTopic) { we are 100% sure we have the observerTopic here. Right? @@ +725,5 @@ > + taskCompleted(); > + }); > + } else if (aObj.data.observerTopic) { > + // Chrome sync request. > + Services.obs.notifyObservers(null, aObj.data.observerTopic, instead having null, you can use the target.

Comment on attachment 8637201 [details] [diff] [review] Part 2: Schedule periodic SW updates Review of attachment 8637201 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/components/ServiceWorkers.jsm @@ +128,5 @@ > + if (Services.prefs.prefHasUserValue(RSYNC_MIN_INTERVAL_PREF)) { > + minInterval = Services.prefs.getIntPref(RSYNC_MIN_INTERVAL_PREF); > + } > + } catch(e) { > + dump(e); debug(e); // or something better return; @@ +137,5 @@ > + if (Services.prefs.prefHasUserValue(RSYNC_WIFI_ONLY_PREF)) { > + wifiOnly = Services.prefs.getBoolPref(RSYNC_WIFI_ONLY_PREF); > + } > + } catch(e) { > + dump(e); here too. @@ +196,5 @@ > + let data; > + try { > + data = JSON.parse(aData); > + } catch(e) { > + dump(e); debug or a better dump message.

We are finally going to be using the idle-daily mechanism on B2G as well, so I am closing this as WONTFIX.

FWIW the spec around this has changed completely! So the work done in bug 1112469 will need to get reverted. Sorry for all of the wasted time on this!