Does PaymentManager need to be exposed to Worker scope?

[danyao]

The PaymentManager interface currently has [Exposed=(Window,Worker)] in the spec. I wonder if the Worker scope is necessary.

Currently PaymentManager is only ever used through the service worker registration's paymentManager instance. Chrome's current implementation doesn't use [Exposed], which defaults to Window.

Digging through history, it seems that @romandev and @ianbjacobs added the window and worker scopes in #199 to allow the static method PaymentManager.requestPermission to be used in script. But @rsolomakhin removed requestPermission in #277.

I propse that we remove the Worker scope. Does this make sense?

[ianbjacobs]

@danyao,

At this level of detail, I defer to the implementers. I hope @romandev and @rsolomakhin and you will reach consensus! Ian

[romandev]

Hi, @danyao

I left inline comments :)

The PaymentManager interface currently has [Exposed=(Window,Worker)] in the spec. I wonder if the Worker scope is necessary.

Do you have any special reasons to introduce the restriction? For example, requestPermission() contained a permission UI prompt. As you know, the UI should be invoked on main thread. So the [Exposed=Window] restriction was added to requestPermission().

On the other hand, the PaymentManager is exposed as an attribute to extended ServiceWorkerRegistration. Today, we can access the ServiceWorkerRegistration in Worker scope(including ServiceWorker) as well as Window scope[1]. Therefore, it is reasonable that the attribute is exposed to the equivalent scope as ServiceWorkerRegistration if there is no special reasons.

Currently PaymentManager is only ever used through the service worker registration's paymentManager instance. Chrome's current implementation doesn't use [Exposed], which defaults to Window.

It might be a Chrome bug. Looking into the history at Chromium[2], when I implemented this feature first(2016), there was no "Exposed" extended attribute in the spec. After that, while implementing/standardizing requestPermission(), we have added a restriction that expose the method to Window scope only because of a permission UI prompt. And I realized that we missed Exposed extended attribute for PaymentManager. So, I added it as well. (2017) However, I forgot to reflect the spec change to Chromium/Blink and the PaymentHandler was just shipped without fixing the change in Chrome.

Thank you.

[1] https://w3c.github.io/ServiceWorker/#serviceworkerregistration-interface [2] https://chromium.googlesource.com/chromium/src/+/60a84ca8de6308a26f8500600ad8ba7c1ce2a41c/third_party/WebKit/Source/modules/payments/PaymentAppManager.idl

[rsolomakhin]

The Worker scope allows a service worker to update the installed instruments via paymentManager.instruments.set() in response to a push from a server without showing a webpage. I propose that we keep it.

[romandev]

@rsolomakhin, Yeah, but if we want to expose the PaymentManager in ServiceWorker(not pure Worker) and Window, then we can change it as follows: [Exposed=(Window, ServiceWorker)]

That being said, I believe we should keep it for consistency according to my earlier comment if there is no special reason(e.g. security risk) not to expose to Worker.

[rsolomakhin]

@romandev : I'm not familiar with the difference between Worker and ServiceWorker. Can you point me to documentation, please?

[danyao]

@romandev Sorry about the delay in responding to your comments! Please see inline below.

Do you have any special reasons to introduce the restriction? For example, requestPermission() contained a permission UI prompt. As you know, the UI should be invoked on main thread. So the [Exposed=Window] restriction was added to requestPermission().

I don't see this as a restriction. IMHO this is a case where the spec evolved beyond the implementation, and we now need to make a decision whether the implementation should follow the spec, or if we should revert the spec change if there is no longer a good reason for this to be in the spec.

On the other hand, the PaymentManager is exposed as an attribute to extended ServiceWorkerRegistration. Today, we can access the ServiceWorkerRegistration in Worker scope(including ServiceWorker) as well as Window scope[1]. Therefore, it is reasonable that the attribute is exposed to the equivalent scope as ServiceWorkerRegistration if there is no special reasons.

Even without exposing the PaymentManager interface to Worker scope, service worker code can access its methods via the registration.paymentManager instance. I'm not convinced that consistency with ServiceWorkerRegistration scope without a concrete use case is enough justification to expand the scope.

WDYT?

[skyclouds2001]

My opinion, since ServiceWorkerRegistration object is defined as exposing to window and worker, which goes the same to the paymentManager read-only property; we should do the same to the PaymentManager object.

It'll be quite strange that one object is not available in the worker scope while its instance is available in the worker scope. We should align their expose scope.