Created attachment 323086 [details] Patch

Attachment 323086 [details] did not pass style-queue: ERROR: LayoutTests/platform/ios-wk2/TestExpectations:31: Unrecognized modifier 'sierra+' [test/expectations] [5] ERROR: LayoutTests/platform/ios-wk2/TestExpectations:32: Unrecognized modifier 'sierra+' [test/expectations] [5] ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/ios-wk2/TestExpectations:31: Unrecognized modifier 'sierra+' [test/expectations] [5] ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/ios-wk2/TestExpectations:32: Unrecognized modifier 'sierra+' [test/expectations] [5] Total errors found: 4 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 323086 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323086&action=review > LayoutTests/platform/ios-wk2/TestExpectations:41 > +imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ Skip ] > +imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ Skip ] I think these should be WontFix (?), not Skip, right?

(In reply to Tim Horton from comment #4) > Comment on attachment 323086 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=323086&action=review > > > LayoutTests/platform/ios-wk2/TestExpectations:41 > > +imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ Skip ] > > +imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ Skip ] > > I think these should be WontFix (?), not Skip, right? I didn't know about that, but yes. Thanks for the review!

(In reply to Andy Estes from comment #6) > (In reply to Tim Horton from comment #4) > > Comment on attachment 323086 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=323086&action=review > > > > > LayoutTests/platform/ios-wk2/TestExpectations:41 > > > +imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ Skip ] > > > +imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ Skip ] > > > > I think these should be WontFix (?), not Skip, right? > > I didn't know about that, but yes. It’s actually important, not just pedantic, because Ryan has a new project to drive ‘Skip’s to zero (but not WontFix, of course). > Thanks for the review!

Created attachment 323097 [details] Patch

Comment on attachment 323097 [details] Patch Clearing flags on attachment: 323097 Committed r223021: <http://trac.webkit.org/changeset/223021>

All reviewed patches have been landed. Closing bug.

LayoutTest http/tests/paymentrequest/payment-request-abort-method.https.html is a flaky failure: https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r223044%20(4889)/results.html https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fpaymentrequest%2Fpayment-request-abort-method.https.html LayoutTest http/tests/paymentrequest/payment-request-show-method.https.html is hitting an assertion failure: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fpaymentrequest%2Fpayment-request-show-method.https.html ERROR: +[PKPaymentAuthorizationViewController requestViewControllerWithPaymentRequest:completion:] error Error Domain=PKPassKitErrorDomain Code=4 "(null)" /Volumes/Data/slave/sierra-debug/build/Source/WebKit/UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm(56) : auto WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI(const WebCore::URL &, const Vector<WebCore::URL> &, const WebCore::ApplePaySessionPaymentRequest &, WTF::Function<void (bool)> &&)::(anonymous class)::operator()(PKPaymentAuthorizationViewController *, NSError *) const ASSERTION FAILED: m_state == State::Activating /Volumes/Data/slave/sierra-debug/build/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp(110) : auto WebKit::WebPaymentCoordinatorProxy::showPaymentUI(const WTF::String &, const Vector<WTF::String> &, const WebCore::ApplePaySessionPaymentRequest &, bool &)::(anonymous class)::operator()(bool) const 1 0x1096f793d WTFCrash 2 0x10d007d91 WebKit::WebPaymentCoordinatorProxy::showPaymentUI(WTF::String const&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, bool&)::$_2::operator()(bool) const 3 0x10d007d14 WTF::Function<void (bool)>::CallableWrapper<WebKit::WebPaymentCoordinatorProxy::showPaymentUI(WTF::String const&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, bool&)::$_2>::call(bool) 4 0x10c6e6f20 WTF::Function<void (bool)>::operator()(bool) const 5 0x10d013194 WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI(WebCore::URL const&, WTF::Vector<WebCore::URL, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, WTF::Function<void (bool)>&&)::$_0::operator()(PKPaymentAuthorizationViewController*, NSError*) const 6 0x10d013106 WTF::BlockPtr<void (PKPaymentAuthorizationViewController*, NSError*)> WTF::BlockPtr<void (PKPaymentAuthorizationViewController*, NSError*)>::fromCallable<WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI(WebCore::URL const&, WTF::Vector<WebCore::URL, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, WTF::Function<void (bool)>&&)::$_0>(WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI(WebCore::URL const&, WTF::Vector<WebCore::URL, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, WTF::Function<void (bool)>&&)::$_0)::'lambda'(void*, PKPaymentAuthorizationViewController*, NSError*)::operator()(void*, PKPaymentAuthorizationViewController*, NSError*) const 7 0x10d0130b8 WTF::BlockPtr<void (PKPaymentAuthorizationViewController*, NSError*)> WTF::BlockPtr<void (PKPaymentAuthorizationViewController*, NSError*)>::fromCallable<WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI(WebCore::URL const&, WTF::Vector<WebCore::URL, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, WTF::Function<void (bool)>&&)::$_0>(WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI(WebCore::URL const&, WTF::Vector<WebCore::URL, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::ApplePaySessionPaymentRequest const&, WTF::Function<void (bool)>&&)::$_0)::'lambda'(void*, PKPaymentAuthorizationViewController*, NSError*)::__invoke(void*, PKPaymentAuthorizationViewController*, NSError*) 8 0x7fffbe970a22 __91+[PKPaymentAuthorizationViewController requestViewControllerWithPaymentRequest:completion:]_block_invoke_4 9 0x7fffc3ed8524 _dispatch_call_block_and_release 10 0x7fffc3ecf8fc _dispatch_client_callout 11 0x7fffc3edcaac _dispatch_main_queue_callback_4CF 12 0x7fffae20cd69 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ 13 0x7fffae1ce04d __CFRunLoopRun 14 0x7fffae1cd544 CFRunLoopRunSpecific 15 0x7fffafbfe4e2 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] 16 0x10821cb84 WTR::TestController::platformRunUntil(bool&, double) 17 0x1081fbf39 WTR::TestController::runUntil(bool&, double) 18 0x1081fbae7 WTR::TestController::resetStateToConsistentValues(WTR::TestOptions const&) 19 0x10821f8a5 WTR::TestInvocation::invoke() 20 0x1082031c5 WTR::TestController::runTest(char const*) 21 0x108204231 WTR::TestController::runTestingServerLoop() 22 0x1081f6b66 WTR::TestController::run() 23 0x1081f660a WTR::TestController::TestController(int, char const**) 24 0x1081f6d23 WTR::TestController::TestController(int, char const**) 25 0x1081d5bcf main 26 0x7fffc3f05235 start The assertion failure is reproducible with the following command: run-webkit-tests --debug http/tests/paymentrequest/payment-request-show-method.https.html -fg --iter 20

Reverted r223021 for reason: LayoutTests added with this change are failing. Committed r223053: <http://trac.webkit.org/changeset/223053>

Created attachment 323210 [details] Patch

I added a mock PaymentCoordinator so that show() and hide() do not try to invoke actual PassKit UI. Tim reviewed this in person.

Created attachment 323211 [details] Patch

The commit-queue encountered the following flaky tests while processing attachment 323211 [details]: imported/w3c/web-platform-tests/custom-elements/microtasks-and-constructors.html bug 178097 (author: youennf@gmail.com) The commit-queue is continuing to process your patch.

The commit-queue encountered the following flaky tests while processing attachment 323211 [details]: ietestcenter/css3/multicolumn/column-containing-block-003.htm bug 120854 (author: dtharp@codeaurora.org) The commit-queue is continuing to process your patch.

Comment on attachment 323211 [details] Patch Clearing flags on attachment: 323211 Committed r223076: <http://trac.webkit.org/changeset/223076>

All reviewed patches have been landed. Closing bug.