[ApplePay] Remove remaining custom bindings from the ApplePay code
Created attachment 297114 [details] Patch
Attachment 297114 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/applepay/ApplePaySession.h:137: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 297124 [details] Patch
Attachment 297124 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/applepay/ApplePaySession.h:137: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 297124 [details] Patch Attachment 297124 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2723621 New failing tests: imported/w3c/web-platform-tests/IndexedDB/interfaces.worker.html
Created attachment 297137 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 297124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297124&action=review Didn’t finish reviewing, but here are my first few comments. > Source/WebCore/Modules/applepay/ApplePayLineItem.h:36 > + using Type = WebCore::PaymentRequest::LineItem::Type; I don’t think the WebCore prefix is needed here. > Source/WebCore/Modules/applepay/ApplePayLineItem.idl:31 > +[ > + Conditional=APPLE_PAY, > +] enum ApplePayLineItemType { > + "pending", > + "final" > +}; Bad to rely on this, I suppose, but a conditional applied to the dictionary in this file applies to the entire file and does not need to be repeated on the enum. > Source/WebCore/Modules/applepay/ApplePayPaymentRequest.h:37 > +#include <heap/Strong.h> > +#include <wtf/Optional.h> > +#include <wtf/Vector.h> > +#include <wtf/text/WTFString.h> Are any of these redundant? I think some of the other headers above already include these. > Source/WebCore/Modules/applepay/ApplePayPaymentRequest.h:45 > + using ShippingType = WebCore::PaymentRequest::ShippingType; I don’t think you need the WebCore prefix here. > Source/WebCore/Modules/applepay/ApplePayPaymentToken.h:31 > +#include <wtf/text/WTFString.h> No need for this include.
Created attachment 297196 [details] Patch
Attachment 297196 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/applepay/ApplePaySession.h:137: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 297236 [details] Patch
Attachment 297236 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/applepay/ApplePaySession.h:137: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 1 in 43 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 297247 [details] Patch
Attachment 297247 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/applepay/ApplePaySession.h:137: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 1 in 44 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 297247 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297247&action=review > Source/WebCore/Modules/applepay/ApplePayPaymentPass.h:35 > +namespace WebCore { > + > + > +struct ApplePayPaymentPass { Extra blank line here. > Source/WebCore/Modules/applepay/ApplePaySession.h:46 > class URL; > +struct ApplePayLineItem; I normally use a blank line and put the structs into a separate paragraph from the classes. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4548 > - push(@headerContent, GenerateDictionariesHeaderContent(undef, $otherDictionaries)) if $otherDictionaries; > + push(@headerContent, GenerateDictionariesHeaderContent($dictionary, $otherDictionaries)) if $otherDictionaries; I would expect this change to break the build, because MediaTrackConstraints does not contain a type named MediaTrackConstrants::ConstraintSet or MediaTrackConstrants::ConstrainLong. Why doesn’t the change cause a problem?
(In reply to comment #14) > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4548 > > - push(@headerContent, GenerateDictionariesHeaderContent(undef, $otherDictionaries)) if $otherDictionaries; > > + push(@headerContent, GenerateDictionariesHeaderContent($dictionary, $otherDictionaries)) if $otherDictionaries; > > I would expect this change to break the build, because MediaTrackConstraints > does not contain a type named MediaTrackConstrants::ConstraintSet or > MediaTrackConstrants::ConstrainLong. Why doesn’t the change cause a problem? I explicitly set the ImplementedAs extended attribute on the MediaTrackConstrants types to avoid the issue.
Committed revision 209927.
(In reply to comment #15) > I explicitly set the ImplementedAs extended attribute on the > MediaTrackConstrants types to avoid the issue. Ah! I went looking but missed that. I guess I’m not 100% thrilled with the rule we made that removes prefixes and nests types inside other types, but it seems like something we can easily refine over time.