Bug 197751

Created attachment 369516 [details] Patch

Comment on attachment 369516 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369516&action=review > Source/WebCore/ChangeLog:15 > + In r243324, when a document has had user agent scripts injected into it, payment APIs were > + disabled at runtime by having all entry points return falsy values or throw exceptions > + (e.g., ApplePaySession.canMakePayments() returns false). > + > + In the case of user scripts in particular (e.g., WKUserScript), since we know whether these > + exist at the time we create a document's DOMWindow, we can do better than r243324 by > + completely disabling the payment APIs in the presence of user scripts. It seems like it will be more confusing and more work for developers to have to deal with with these both (falsy values and completed disabled) cases. Since the developer already has to deal with the first case, why introduce this as well?

(In reply to Sam Weinig from comment #4) > Comment on attachment 369516 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=369516&action=review > > > Source/WebCore/ChangeLog:15 > > + In r243324, when a document has had user agent scripts injected into it, payment APIs were > > + disabled at runtime by having all entry points return falsy values or throw exceptions > > + (e.g., ApplePaySession.canMakePayments() returns false). > > + > > + In the case of user scripts in particular (e.g., WKUserScript), since we know whether these > > + exist at the time we create a document's DOMWindow, we can do better than r243324 by > > + completely disabling the payment APIs in the presence of user scripts. > > It seems like it will be more confusing and more work for developers to have > to deal with with these both (falsy values and completed disabled) cases. > Since the developer already has to deal with the first case, why introduce > this as well? We've found some cases where sites check for API availability but do not use the API properly. For instance, some sites will render Apple Pay buttons whenever window.ApplePaySession exists without first calling canMakePayments().

Comment on attachment 369516 [details] Patch We should add a test that stores a reference to a PaymentRequest, has scripts injected then verify that using that stored reference still gets falsy values.

Is this a bug in the new test? FAIL: (JS) JSTestEnabledForContext.cpp --- WebCore/bindings/scripts/test/JS/JSTestEnabledForContext.cpp 2019-05-09 12:47:19.000000000 -0700 +++ /var/folders/19/dls3q4zx3yvd609xbdvv189h0000gn/T/tmpOkWHzN/JSTestEnabledForContext.cpp 2019-05-09 12:47:36.000000000 -0700 @@ -69,7 +69,6 @@ JSTestEnabledForContextPrototype(JSC::VM& vm, JSC::JSGlobalObject*, JSC::Structure* structure) : JSC::JSNonFinalObject(vm, structure) { - didBecomePrototype(); } void finishCreation(JSC::VM&);

(In reply to Geoffrey Garen from comment #7) > Is this a bug in the new test? > > FAIL: (JS) JSTestEnabledForContext.cpp > --- WebCore/bindings/scripts/test/JS/JSTestEnabledForContext.cpp 2019-05-09 > 12:47:19.000000000 -0700 > +++ > /var/folders/19/dls3q4zx3yvd609xbdvv189h0000gn/T/tmpOkWHzN/ > JSTestEnabledForContext.cpp 2019-05-09 12:47:36.000000000 -0700 > @@ -69,7 +69,6 @@ > JSTestEnabledForContextPrototype(JSC::VM& vm, JSC::JSGlobalObject*, > JSC::Structure* structure) > : JSC::JSNonFinalObject(vm, structure) > { > - didBecomePrototype(); > } > > void finishCreation(JSC::VM&); No, I just need to rebase the result after r245082.

Created attachment 369620 [details] Patch

(In reply to Alex Christensen from comment #6) > Comment on attachment 369516 [details] > Patch > > We should add a test that stores a reference to a PaymentRequest, has > scripts injected then verify that using that stored reference still gets > falsy values. I added the test you suggested to attachment #369620 [details].

Created attachment 369889 [details] Patch

Comment on attachment 369889 [details] Patch Clearing flags on attachment: 369889 Committed r245314: <https://trac.webkit.org/changeset/245314>

All reviewed patches have been landed. Closing bug.