SW registration code unclear/incorrect

[ithinkihaveacat]

The sample service worker registration code included in the spec is potentially misleading in that it gives the impression that a payment handler is ready to handle payments before it actually is.

The registration example code at https://w3c.github.io/payment-handler/#register-example registers a service worker in a typical way:

  const { registration } = await navigator.serviceWorker.register('/sw.js');
  await addInstruments(registration);

My understanding is that after these two lines have been executed, the service worker defined in sw.js is not necessarily active: the service worker controlling the page may be a previous service worker (one not able to handle payment-specific events such as onpaymentrequest) and therefore at this point it would not be appropriate to tell that user that they can e.g. "Now Pay with Bob Pay."

Whilst the sample code doesn't not explicitly make this claim, I think many readers unfamiliar with the details of the service worker lifecycle would assume that after the service worker is registered and instruments added, the user is ready to go with Bob Pay. Instead, a more complicated algorithm must be followed to determine the point at which sw.js is active.

/cc @jakearchibald

[romandev]

Absolutely.

As far as I remember, that part was in the spec, but it seems to be removed now[1].
When I implemented PaymentHandler API in Chromium/Blink, I did make instrument.set() be only allowed in the active worker according to the spec[2].

I guess that it was just a mistake that it was removed. When we moved from PaymentApp API to PaymentHandler API, there are much changes. So, it's just missed. (I think)

@ianbjacobs, Is there any special reason why the spec text was removed?
If no reasons, we can restore the spec text simply :)

[1] https://github.com/w3c/payment-handler/pull/107/files#diff-eacf331f0ffc35d4b482f1d15a887d3bL828
[2] https://codereview.chromium.org/2790013002 (PaymentInstruments.cpp, 38 line)

[ianbjacobs]

Hi all,

@romandev, I suspect any changes to code that created problems are mistakes. :) I suggest that you and @rsolomakhin look at the code and fix / restore anything that's needed. (I am not sure to be of much help here.)

Ian

[romandev]

@ianbjacobs Thank you.
I'll send a pull request to restore/fix them and then write some tests in web-platform-tests.
I think that current Chrome implementation is already correct
but if there are some bugs, @rsolomakhin or I can fix them.

[jakearchibald]

navigator.serviceWorker.register('/sw.js');
const registration = await navigator.serviceWorker.ready;
await addInstruments(registration);

…should be fine.

Btw, const { registration } = … is performing destructuring, so the result will be undefined.

[rsolomakhin]

@romandev Thank you for volunteering the send the the pull request to the spec. Chromium/Blink implementation is already correct. I had to use the .ready promise in my own samples and in Chromium's integration tests.

[ianbjacobs]

@rsolomakhin, is this issue still relevant?

Ian

[romandev]

According to w3c/ServiceWorker#920, they decided not to wait until active worker. We should use navigator.serviceWorker.ready and add it to our example. I'll update #224 PR today including the example change.

[rsolomakhin]

Thank you @romandev