WICG / background-sync

A design and spec for ServiceWorker-based background synchronization
https://wicg.github.io/background-sync/spec/
Apache License 2.0
640 stars 83 forks source link

Guidance on background sync and shared workers #70

Closed jrburke closed 9 years ago

jrburke commented 9 years ago

Andrew Overholt of Mozilla suggested I post here about use cases that come up in the context of Firefox OS and service workers that want to do background sync. It may help with the use case document.

Context

The email app on Firefox OS. It wants to do background syncs every so often when there is connectivity available. The user indicates a rough time interval (5 mins, 10 minutes, 15 minutes, 30 minutes, 1 hour). The sync interval does not have to be exact, just approximate to those intervals, and only if there is connectivity available.

Background sync is used instead of something like a push API-backed server because it is a generic email application that can connect to IMAP/ActiveSync/POP3 servers that likely do not have support for push.

Firefox OS apps can have multiple browser windows opened per app, so the email app would want to use a shared worker to do the network and IndexedDB work for syncing and for handling serving slices of the email data to the browser windows. We just what one entity responsible for the network connections and data services, and holding the business logic related to the data models.

Issue

This gets a bit tricky though when considering a service worker with a background sync entry point:

However, service workers spinning up shared workers did not seem to be allowed, if I read the old issues list for service workers correctly.

So guidance on this use case is appreciated. Here are some things that were discussed when talking about the issue within our group:

Possibilities

Talk to shared worker: Maybe there is a way for the background sync entry point to communicate to the shared worker, and start up the shared worker if not running, and wait for it to complete its network and database work before signaling the sync is complete.

However, it is my understanding that this is not allowed in a service worker at the moment.

Service worker as the backend: Use the service worker as the shared worker, so embedding the code for the sync work in the service workers, and the user visible browser windows for the app would talk to it for the data slices.

However, we do not want to burden the service worker with more work, where it might interfere or slow down the service worker handling its basic fetch duties. Plus, the service worker seemed volatile, and not really designed to be more long-lived for use by user visible browser windows for business logic/data services for the UI.

openWindow: Perhaps the service worker could use openWindow to open a non-visible browser window that would then talk to/spin up the shared worker.

However, it is unclear if those windows would be user visible or not. In this case we would not want that window visible as it is just a pipeline to spin up the shared worker, and that window needs to be destroyed at some point. It seems like a better expression of intention to just spin up the shared worker directly, as something that is not visible and just related to data processing.

annevk commented 9 years ago

If the service worker only does asynchronous work, I think it should be able to be the backend. But I also don't see why we'd disallow opening workers from a service worker. They might be shortlived as well, but I'm not sure why we'd explicitly disallow running them.

wanderview commented 9 years ago

@annevk Step 10 in the SharedWorker spec says:

Add to worker global scope's list of the worker's Documents the Document objects in docs.

We don't have a Document in ServiceWorker. That would need to be adjusted somehow.

In general, though, I think the ServiceWorker is the "SharedWorker without a document" concept. Maybe we just need a way for them to attach to a separate ServiceWorker script (at a different scope?).

wanderview commented 9 years ago

@jrburke In regards to "the service worker seemed volatile" issue, I believe there are plans to implement a waitUntil(promise) function that will explicitly hold the ServiceWorker alive.

I agree, though, if you are doing sync work that should probably be on a separate worker.

wanderview commented 9 years ago

@jrburke Can you have a separate ServiceWorker scripts registered to handle background sync and fetch events? That way work done in the first SW would not delay handling a fetch event.

jakearchibald commented 9 years ago

Agree with @annevk here. @jrburke - are you expecting to do any heavy synchronous execution here?

I'm really worried about battery usage here so periodic sync should be seen as "beneficial" rather than "critical". Doesn't email fall more into "critical"?

How will you communicate with the IMAP/ActiveSync/POP3 servers? Do they have CORS-enabled HTTPS endpoints?

Good use-case though! Same applies for decentralised RSS reader, although I guess the CORS+HTTPS requirement will bite you there.

jrburke commented 9 years ago

Something I left out of the use case that might also help illustrate the issue more:

In addition to the background sync, the user has the option in the mail UI to trigger a sync via a button. Since multiple user-visible browser windows could trigger a sync, as well as the background sync, we only want one entity doing the actual sync work, since it can coordinate only doing one sync at a time, to accurately communicate sync positions to servers, and discarding others that perhaps are triggered during the same time.

This seemed to point using a shared worker for that sync work.

It sounds like the advice might be "use a separate service worker from the one that handles URL fetches", because perhaps spinning up other service workers is allowed from a service worker, and that also works in browser windows too. So, using the service worker as a shared worker, but since service workers are not bound by the shared worker "add the document" part of the shared worker spec, it side steps the shared worker issue?

My concern with that approach: right now the (plain) worker in email handles the server syncing but also IDB and JS model work around the IDB data, so the browser windows use the worker for data/model services for the UI, and for memory/code co-location concerns I would ideally like to keep that logic in the worker that does the server synchronizing vs spinning up a specialized worker just for the sync part.

If there are ways to make sure the service worker that had the sync code could stay up and available for the data actions that seems to be ideal. Perhaps it is as simple as the browser window holding on to the reference for that service worker instance for as long as the browser window is up?

@jakearchibald on your questions:

Battery usage is a concern for us too if you mean the cost of doing a polling action vs relying on something like a push api where the app is only woken up when there is new data. However, given the sensitivity of email credentials and email contents, we do not want to, for example, run a generic email server proxy to do the sync checking/push API work as it would be a very tempting target for hackers.

As to "beneficial" vs "critical", I am not familiar if those terms mean specific things in this context. Syncing for the email app is best effort given circumstances: not wanting to run a server piece/proxy, the user understanding the variability of network on mobile. The "last sync" time is shown in the UI, and the user can trigger a sync from the UI if they want to be assured of the most recent data visible, which I believe helps with bridging gaps in any automated syncing.

Currently we use mozTCPSocket, a Firefox OS-specific API only visible to certified apps, to talk to the servers, since this is a generic email client that can connect to any IMAP/ActiveSync/POP3 server.

I think those details are more side notes though. The general use case is:

We have data work to do that is possible to trigger from user visible UI or the background sync entry point, and we only want one entity doing that work to enforce data coherency and to use the network efficiently.

That entity could also be providing data services to user visible UI outside of the basic data synchronization. This is done for code co-location and memory concerns: many of the JS data objects are used for the sync-to-IDB storage step and browser UI, and some APIs are shared with actions triggered from the browser UI (moving/deleting messages, starring emails for example). Ideally loading that code again in a separate dedicated service worker just for syncing could be avoided because of extra memory and code loading configuration concerns.

Sounds like the guidance is:

Use a separate service worker from the one use for URL fetches. That separate service worker can handle the background sync entry point and browser windows can spin it up and hold on to it for data services, communicating with it using normal worker communication methods. As long as the browser windows hold on to that service worker reference, it will stay up and act like a shared worker in that case.

If that is the case, feel free to close this issue.

wanderview commented 9 years ago

Holding the ServiceWorker ref does not keep the worker alive. You will need something like slightlyoff/ServiceWorker#669.

Also, its unclear to me if the current API gives you an easy way to spin up the ServiceWorker if its not already running in the "manual sync button" case. We may need some new API here.

jungkees commented 9 years ago

Maybe it's a good use case of navigator.connect() with the concept of stash MessagePorts proposed by @mkruisselbrink.

@jrburke I'd imagine a setting where you register two separate SWs: (A) the sync worker and (B) the business logic worker. (B) replaces the existing shared worker.

Email clients (documents) connect to (B) and do the channel messaging as they've done with the shared worker. In this setting, the clients and (B) can communicate regardless of (B)'s lifetime.

(A) also can connect to (B), and they can do the channel messaging regardless of the lifetime of both ends.

Upon (A)'s sync request, (B) can run update and get the latest data ready without the existence of clients.

// from a Window to (B)
navigator.serviceWorker.ready.then(registration => {
  registration.getStashedPorts('toB')
    .then(port => { port.postMessage('doSync'); });
});

// from (A) to (B)
self.addEventListener('sync', function(e) {
  self.registration.getStashedPorts('toB')
    .then(port => { port.postMessage('doSync'); });
});

(B) running sync operations in message events' waitUntil(p) seems not worse than running them in a shared worker I suppose.

And how about introducing a SW's persistency mode like the following?

// makes the service worker run as long as *any* client is being loaded or running.
navigator.serviceWorker.register("sw.js", { persistency: "max-binding" });

/cc @mkruisselbrink, @jakeleichtling, @slightlyoff, @reillyeon, @KenjiBaheux

jakearchibald commented 9 years ago

In addition to the background sync, the user has the option in the mail UI to trigger a sync via a button.

I suggested .syncNow over at https://github.com/slightlyoff/BackgroundSync/issues/72#issuecomment-88446724 - this is another good use-case for it.

but since service workers are not bound by the shared worker "add the document" part of the shared worker spec

The spec only uses documents for life cycle, they should be easily replaceable with something more generic. A some kind of worker within the SW is a way simpler solution than managing two serviceworker registrations. Even as an "in the meantime" hack, I'd sooner go for splitting the work into chunks with setTimeout than a separate registration.

Battery usage is a concern for us too if you mean the cost of doing a polling action vs relying on something like a push api where the app is only woken up when there is new data.

No, I'm more concerned about the amount of pure JavaScript processing going on. If your code is doing simple network fetches and adding to a cache, that's all async, you don't need another worker for this. Your need for another thread makes it sound like you're doing heavy synchronous processing in JS, and that makes me worry about battery.

Sounds like the guidance is: Use a separate service worker from the one use for URL fetches.

I'd advise against that, but it would work.

jakearchibald commented 9 years ago

Using background sync for CPU intensive processing (the kind that would lock up the ServiceWorker thread) sounds like an antipattern. IMO, sync should be for little more than a network fetch & storage, maybe showing a notification if something new has been found.

If that data requires heavy processing, that should be deferred to the next opening of the app, where extended CPU use is more reasonable.

wanderview commented 9 years ago

If that data requires heavy processing, that should be deferred to the next opening of the app, where extended CPU use is more reasonable.

Doesn't this disadvantage web apps then compared to native? Are native apps discouraged from pre-processing sync'd data ahead of the app opening? Seems like we're forcing UX jank with this policy.

jrburke commented 9 years ago

It seems like it would just be nice to change shared workers to track things other than documents to keep them alive. So if there are pointers on how to accomplish that, it seems like it will solve my issue.

Then the service worker onsync listener would:

I am not sure navigator.connect() helps directly with this issue, as the domain that registers the service worker will also create the shared worker, and my assumption is that we could use normal worker-to-worker communication in that scenario.

I agree with @jakearchibald that service worker work should be kept light. However, for this sync case, deeper work needs to be done, and it would be nice for the service worker to just hand off to a shared worker in that case. That helps keep the division of responsibilities clearer.

Some notes on not wanting to delay processing of the sync in this particular email case:

I can see cases where immediate processing may not need to be done for a sync, for things like ebook syncing, but for email, the data processing desires seem more immediate. So it seems best if we could just delegate to a shared worker then in that case.

slightlyoff commented 9 years ago

So it seems we need a few things here (OH HAI @jrburke!!!):

annevk commented 9 years ago

A shared worker per specification can already survive navigation. But I don't think it should be able to last long without a browsing context as that would have serious privacy implications. However, opening one from a service worker we should seek to make work somehow. There's no reason why service worker threads, even though they are shortlived, should be banned from spawning parallel threads (that take the lifetime of the service worker if tied to nothing else).

jakearchibald commented 9 years ago

Will reply to other point later, but was chatting to some Android engineers. Seems their liberal stance on background sync is part of the battery problem Android has, so I don't think we should simply copy native.

Also, just because work is on a different thread, doesn't mean it's free. Doing processing work in background sync is attractive because you're not going to slow down or jank your app, but you may affect someone else's.

On Fri, 3 Apr 2015 11:49 Anne van Kesteren notifications@github.com wrote:

A shared worker per specification can already survive navigation. But I don't think it should be able to last long without a browsing context as that would have serious privacy implications. However, opening one from a service worker we should seek to make work somehow. There's no reason why service worker threads, even though they are shortlived, should be banned from spawning parallel threads (that take the lifetime of the service worker if tied to nothing else).

— Reply to this email directly or view it on GitHub https://github.com/slightlyoff/BackgroundSync/issues/70#issuecomment-89251794 .

jakearchibald commented 9 years ago

I'm still trying to get a handle on the predicted hot points of this script.

Is the worry that the amount of code needed to communicate with various email servers will negatively impact the spin-up time of the ServiceWorker for unrelated actions? If so, then making (shared)workers work within a SW seems like the best answer, although would this be a problem with things like (code caching)[http://blog.chromium.org/2015/03/new-javascript-techniques-for-rapid.html]?

If the worry is pure JS-processing after the data has been fetched, then I think we need another kind of sync for that (#77).

jrburke commented 9 years ago

The main issue is that we have a shared worker that handles the data layer for the email app: it converts data from network calls into JS objects suitable for the UI to consume and stores that data in IndexedDB.

We only want one thing doing that work, to properly manage/constrain network connections, provide a coherent data model, and isolate code for developer understanding. So we want the shared worker to do that.

Even without backgroundsync, we want this structure to support the multiple browser windows that can be open to the email app. Email sync can be triggered manually in the UI view, and that sync would go to the shared worker.

So for this use case, I believe it is enough to focus on enabling the service worker to spin up a shared worker, and the shared worker to treat a service worker (instead of just documents) as something that can participate in the ref counting that keeps the shared worker alive.

If there is another place to lobby for that capability instead of here, feel free to point me to that place. If it is helpful, I can point to this thread as motivation for that capability.

I did not follow how that capability would cause a problem for code caching. I do not believe adding a second "processingsync" entry point for a service worker helps this case, but I will comment more in #77 on that, and maybe we can use that issue to discuss hot points.

jakearchibald commented 9 years ago

Ok, I get it now, thanks for dragging me through!

Completely agree, allowing the ServiceWorker to create a SharedWorker and contribute to the refcount is the right answer here.

This should be closed in favour of https://github.com/slightlyoff/ServiceWorker/issues/678

jrburke commented 9 years ago

This use case has been explored, and it looks like slightlyoff/ServiceWorker#678 will be enough to support it, closing this ticket.