w3c / ServiceWorker

Service Workers
https://w3c.github.io/ServiceWorker/
Other
3.63k stars 315 forks source link

Install and activate event recipients questions / observations. #198

Closed michael-nordman closed 10 years ago

michael-nordman commented 10 years ago

In algorithms.md, there are a few places where "all documents which match serviceWorkerRegistration.scope" are to receive an event related to installation and activations.

First an observation. The page that initially invokes serviceWorker.register() need not be in the registration scope so may not receive these events, and that seems like it may be an oversight.

My questions are around what the intent of cycling the these events thru documents are and which docs should receive them. There are 3 groups.

  1. all docs in registration scope
  2. docs in registration scope that are already associated with an active worker
  3. docs invoking the registration process (maybe out of scope)

If the intent is to let docs tied to an old version update finish work before having it swap out, 2 is the one that matters. If the intent is to allow the installing page to observe completion, 3 matters.

Question is, is the currently defined behavior (targeting 1) an approximation for targeting 2 and 3?

jakearchibald commented 10 years ago

Here's what I think the intent for each event is:

Good point about the installing page. I think we're exposing .state on serviceworker objects, maybe a statechange event would sort this?

kinu commented 10 years ago

So going back to the original question, when the algorithm says 'all docs in registration scope' it basically means 2., i.e. they're intended to be dispatched to the docs in the registration scope that already have an active worker, is that right?

Similar question: service_worker.ts says slightly different thing for onerror, i.e. it says it's "Called for any error from the active or installing SW's"-- so unlike install/activate family this one's supposed to be dispatched for both types of docs (i.e. ones that are in registration scope AND is installing the SW, which could be out of scope)? It also has a FIXME comment so I guessed it may not have been really fixed when it was written.

Reg: installing page having something like statechange sounds good to me.

kinu commented 10 years ago

One more related question: DocumentInstallEvent has .previous field, should this be always pointing to the current active worker (so it's basically same as .active-- iff we assume the event is dispatched to the docs that have active workers), or could it be a previous pendingWorker if a new worker is being installed while there's another pending worker that is not yet activated (and will never be activated)?

jakearchibald commented 10 years ago

@kinu

So going back to the original question, when the algorithm says 'all docs in registration scope' it basically means 2., i.e. they're intended to be dispatched to the docs in the registration scope that already have an active worker, is that right?

Oops, sorry for missing the main bit of the question. The intention is 1. fire the event for all documents within the registration scope. This means it works for the first install where there's no active worker.

Similar question: service_worker.ts says slightly different thing for onerror

Ugh, yeah. That's not been thought about properly. Will start a ticket for that.

jakearchibald commented 10 years ago

One more related question: DocumentInstallEvent has .previous field

I've renamed this to .activeWorker to match InstallEvent.

Yeah, it'll match .active in most cases, unless a tab has been shift+reloaded so .active is null, then an upgrade starts.

It's a bit messy that bit, I'm happy to drop activeWorker from that event. What do you think? + @slightlyoff @annevk

jakearchibald commented 10 years ago

Summed up the onerror issues https://github.com/slightlyoff/ServiceWorker/issues/104#issuecomment-38455427

michael-nordman commented 10 years ago

Ok, based on the listed intents, I think these event should be targeted at 2 and 3. Sounds like the current behavior (1) is a poor approximation for (2 and 3).

2. docs in registration scope that are already associated with an active worker 3. docs invoking the registration process (maybe out of scope)

jakearchibald commented 10 years ago

Hmm, I still think 1 is best. onactivate must fire for all in-scope pages as it signals a change to serviceWorker.activate

michael-nordman commented 10 years ago

The serviceWorker.active property refers to the worker associated with the page (the one actively receiving fetch events from the doc) , right? Will those "non-controlled" pages suddenly become "controlled" upon activate? I thought not per the one of the rules of service workers about how the a doc typically spends its life with one sw (or none).

jakearchibald commented 10 years ago

I thought non-controlled pages would become controlled if .replace() was called. I've created a separate ticket to clear this up. https://github.com/slightlyoff/ServiceWorker/issues/202

michael-nordman commented 10 years ago

Yes, but if .replace() is called, those pages are controlled and will receive the activate event, (although they will not have seen the install event).

Completely agree that pages for which serviceWorker.active will change should receive the activate event. In the usual case for an initial install (not .replace), serviceWorker.active does not change for already open pages in the registration-scope. (At least that's my understanding).

jakearchibald commented 10 years ago

In the usual case for an initial install (not .replace), serviceWorker.active does not change for already open pages in the registration-scope. (At least that's my understanding).

As far as my brain will carry me this morning, the only way .active can change during the life of the page is via .replace().

jakearchibald commented 10 years ago

So, why shouldn't in-scope pages get an install event?

The install could have been triggered by a registration call or a background update check - shouldn't we allow pages to show UI for these if they want?

michael-nordman commented 10 years ago

To me, the question is why should they get the install event?

I guess the more important point is that docs invoking the registration process should receive these events too. As spec'd now they may not. The other distinction about already associated vs not was useful for thinking thru who should see these events and why.

For an initial install, I think users will expect to see some UI about progress in the page where they did something to get it started (this page might not be in-scope at all).

For an update, currently associated pages need to know since things are changing underneath them and to present ui. In this case, any in-scope pages will see these events because they'll be tied to controller.

I don't see a strong reason to raise these events into unaffected in-scope pages but also don't see a strong reason not to.

alecf commented 10 years ago

This is making me think that the oninstall/onactive events need to move from ServiceWorkerContainer and into ServiceWorker.

Earlier we had argued that having them on ServiceWorker was too onerous for developers:

We were comparing this:

navigator.serviceWorker.register("/script.js").then(function(sw) {
    sw.oninstall = function(...) {
        ...
    }
    sw.onactivate = function (...) {
        ...
    }
});

against this:

navigator.serviceWorker.register("/script.js");

navigator.serviceWorker.oninstall = function(...) {
    ...
};

navigator.serviceWorker.onactivate = function(...) {
    ...
};

The latter certainly feels cleaner, but as we're finding, it might be less reflective of the reality of the installation/activate events.

Here's another approach: IDB defines a bubbling order for events that there's a somewhat artificial hierarchy of objects inside an IDB object, that provide a bubbling pattern (IDBRequest -> IDBTransaction) - at times it seems odd but it can be convenient, especially for error handling.

What if we defined a bubbling pattern similar to this - that really we're dispatching the event at the ServiceWorker, but that main in turn dispatch itself up to the ServiceWorkerContainer, if in fact the ServiceWorkerContainer "owns" the ServiceWorker (i.e. if it's hooked up via .activate) - in the case of registrations run out of scope, this wouldn't be hooked up and the event wouldn't fire against the ServiceWorkerContainer - and really we're defining this part DOM terms (event bubbling/propagation) now, no longer in ServiceWorker terms (i.e. "all documents in scope" or whatever) so it might be more easily swallowed by developers.

nikhilm commented 10 years ago

This is making me think that the oninstall/onactive events need to move from ServiceWorkerContainer and into ServiceWorker.

This solves both 2 and 3 of the OP. I just don't remember why we decided against it. I'm not sure dispatching up to the container is a great idea though. The IDB request and transaction have a sort of 'parent-child' relationship, but the ServiceWorkerContainer is just a 'manager' for ServiceWorkers, which are ephemeral in comparison.

michael-nordman commented 10 years ago

The .active serviceWorker is not so ephemeral, typically its value never changes. Alec's idea about bubbling these events to the container only applies to the .active instance. Interesting idea.

On Thu, Mar 27, 2014 at 3:32 PM, Nikhil Marathe notifications@github.comwrote:

This is making me think that the oninstall/onactive events need to move from ServiceWorkerContainer and into ServiceWorker.

This solves both 2 and 3 of the OP. I just don't remember why we decided against it. I'm not sure dispatching up to the container is a great idea though. The IDB request and transaction have a sort of 'parent-child' relationship, but the ServiceWorkerContainer is just a 'manager' for ServiceWorkers, which are ephemeral in comparison.

— Reply to this email directly or view it on GitHubhttps://github.com/slightlyoff/ServiceWorker/issues/198#issuecomment-38869349 .

jakearchibald commented 10 years ago
navigator.serviceWorker.register("/script.js").then(function(sw) {
    sw.oninstall = function(...) {
        ...
    }
    sw.onactivate = function (...) {
        ...
    }
});

The above only works for initial registrations, it doesn't cater for browser-initiated updates. I'm interested in the idea of moving these events onto the SW though.

How about:

Then serviceworker instances have a state property, and a onstatechange event.

jakearchibald commented 10 years ago

Some examples using the above proposal:

Show ui updates for a new registration:

navigator.serviceWorker.register("/script.js").then(function(sw) {
  if (sw.state == 'installing') {
    showInstallingMessage();
    sw.addEventListener('statechange', function() {
      switch (sw.state) {
        case "installed":
          hideInstallingMessage();
          showNewVersionReadyMessage();
          break;
        case "activating":
          hideNewVersionReadyMessage();
          showNewVersionInstalledMessage();
          break;
      }
    });
  }
});

The above works even if the registration is out of scope.

Alternatively, showing ui updates for any in-scope update, including registrations initiated by another page and automatic updates:

navigator.serviceWorker.addEventListener('pendingchange', function() {
  showUpdatingMessage();
  navigator.serviceWorker.pending.addEventListener('statechange', function() {
    switch (sw.state) {
      case "installed":
        hideUpdatingMessage();
        showNewVersionReadyMessage();
        break;
      case "activating":
        hideNewVersionReadyMessage();
        showNewVersionInstalledMessage();
        break;
    }
  });
});
michael-nordman commented 10 years ago

I think this sounds reasonable to me.

Do you have in mind to replace the currently described installphase events with these in some way, or do you see these as in addition to the currently described events. I think the latter, but just checking.

kinu commented 10 years ago

It looks much clearer to me, thanks. So... to clarify: the series of events a doc-in-scope will see in a regular registration flow will be:

Does this sound same as what you have in mind? (I'm a bit unsure if [1] and [2] are fired, while I assume the registering doc can see all these state changes on the ServiceWorker returned by .register)

jakearchibald commented 10 years ago

@michael-nordman I figured these would replace

…seems like unnecessary repetition to keep them around, but I am jetlagged, so I could be missing something.

kinu commented 10 years ago

@jakearchibald I vote we remove these duplicated installphase events if we see no problems (though I could be missing something too)

jakearchibald commented 10 years ago

@kinu I hadn't considered parsed -> installing. I don't have a strong opinion, but I think it makes sense. I guess there'll be some failure state, which may happen synchronously when oninstall is called in the SW, so being able to listen to onstatechange onstatechange before that is useful.

As for activating -> active, yeah, I don't see why not.

TODO for me: update the algorithms for this

michael-nordman commented 10 years ago

Ok, and statechange events provide the .waitUtil() mechanism for the event recipient to delay install/activate workflow.

For a document that is in scope and is sitting in a tab prior to registration. What event sequence does it see? Once the worker is fully up and active, what is the value of serviceWorker.active as seen by that tab? I guess null.

pendingchange [to have a new pending] pending.statechange ... pending.statechange (installed -> activating) pendingchange [becomes null] ... and that's it, because this document is not supposed to get associated with the new worker

jakearchibald commented 10 years ago

@michael-nordman I think we've got crossed wires. I'm just talking about changing navigator.serviceWorker.*, I think the events within the SW scope stay as they are.

michael-nordman commented 10 years ago

No crossing of wires. Of course the events dispatched into sw scope don't change.

I thought documents that (previously) also received oninstall/onactivate events could defer progress via .waitUntil() as the sw itself can. But if documents don't need that delay mechanism provided to them... great... less is more.

jakearchibald commented 10 years ago

Hah, the wire-crossing was all mine then, sorry. Yeah, the in-page events don't need waitUntil, they can just be simple events.

kinu commented 10 years ago

(Wanted clarification for most basic case...) In my understanding when SW doesn't call .replace() .pending won't become .active during the lifetime of a doc. In this case is Activation delayed, and will the sequence of events be:

Existing in-scope doc will see:

Next time an in-scope doc is loaded it (the first one) will see:

is that right? Or maybe activation just happens? ...in the case what will be the last event an existing in-scope doc will see? (.pending.onstatechange for activating or active??)

jakearchibald commented 10 years ago

Yeah, that correct, assuming that SW is allowed to transition from pending to active (as in, there's no existing active SW that's being used by another page).

The statechange of activating -> active may happen before the doc loads, as in:

Page loads:

Then that tab is closed, and was the only tab in that scope. The current active worker is terminated and GC'd. The pending worker starts activating, then activates.

Some time later, a page in the scope is loaded

In this case there isn't a statechange.

It makes sense to allow a SW to go through the activating phase as soon as possible (as soon as the previous SW can be thrown out), it's holding up requests while it's activating so best to do that while there isn't a page to hold up.

michael-nordman commented 10 years ago

Even better then... we just simplified the install/update sequence by eliminating that deferal mechanism.

michael-nordman commented 10 years ago

I think we still have a question about the most basic case. The most basic case does not involve .replace() and does not involve update, and does not involve multiple pages.

The most basic case: A page loads and calls sw.register() for the first time ever. There is no previous registration in this scope. The page itself is in-scope.

What set events will the page see? What set of events will the worker itself see?

You see there's an odd disconnect. The worker should not become active for that installing page. It should live its life out w/o its requests being hijacked. Yet the worker itself should go merrily thru the install sequence and land in the ACTIVE state such that new navigations will be subject to it.

So, the install/activate sequence has run to completion. Now the question is what does the installing page see for .pending and .active when in this state?

I'll have follow up questions, but lets start with just this one.

edit: also here's a tentative answer In the installing page, .pending will refer to the newly installed sw in that case, but if you read the.pending.status value, it will return ACTIVE.

jakearchibald commented 10 years ago

Good question.

The page will get:

.active remains null.

So navigator.serviceWorker.pending and .active are related to the page, whereas .state of the SW is it's lifecycle.

If another tab opened, its .pending would be null, and it's .active would be the same SW as the .pending in the first tab.

michael-nordman commented 10 years ago

sgtm

kinu commented 10 years ago

sgtm2. For error reporting (onerror stuff) we could probably do similar? E.g. moving .onerror handler from container to SW object, so that doc can explicitly know which SW the error is coming from (though it may need to attach handlers to multiple SWs).

jakearchibald commented 10 years ago

If we're willing to lose error events for syntax errors (which happen before pendingchange), then yes, which I think is fine. Going to post more thoughts on #104

dominiccooney commented 10 years ago

Implementer feedback: Blink has implemented "state" and "statechange", these should show up in Chrome Canary tomorrow. We don't currently produce the full set of states (for example, we may jump from "installing" to "active" without going via "activating") but we intend to.

jakearchibald commented 10 years ago

Return from leave tomorrow, will update the algorithms to reflect the changes we made On 15 Apr 2014 03:35, "Dominic Cooney" notifications@github.com wrote:

Implementer feedback: Blink has implemented "state" and "statechange", these should show up in Chrome Canary tomorrow. We don't currently produce the full set of states (for example, we may jump from "installing" to "active" without going via "activating") but we intend to.

— Reply to this email directly or view it on GitHubhttps://github.com/slightlyoff/ServiceWorker/issues/198#issuecomment-40439977 .

KenjiBaheux commented 10 years ago

@michael-nordman @jakearchibald Tentatively closing this issue. Separate issues might work better if there are remaining aspects to discuss.