Bug 178043 - [Payment Request] Implement PaymentRequest.show() and PaymentRequest.hide()
Summary: [Payment Request] Implement PaymentRequest.show() and PaymentRequest.hide()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on:
Blocks: 174796
  Show dependency treegraph
 
Reported: 2017-10-06 22:20 PDT by Andy Estes
Modified: 2017-10-09 15:41 PDT (History)
11 users (show)

See Also:


Attachments
Patch (37.65 KB, patch)
2017-10-06 22:36 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (37.67 KB, patch)
2017-10-07 03:54 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-cq-01 for mac-elcapitan (998.54 KB, application/zip)
2017-10-07 05:07 PDT, WebKit Commit Bot
no flags Details
Patch (38.34 KB, patch)
2017-10-07 11:42 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (60.57 KB, patch)
2017-10-09 13:35 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (60.56 KB, patch)
2017-10-09 13:41 PDT, Andy Estes
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2017-10-06 22:20:57 PDT
[Payment Request] Implement PaymentRequest.show() and PaymentRequest.hide()
Comment 1 Andy Estes 2017-10-06 22:36:41 PDT
Created attachment 323086 [details]
Patch
Comment 2 Build Bot 2017-10-06 22:39:28 PDT
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 3 Andy Estes 2017-10-06 22:55:15 PDT
rdar://problem/34076639
Comment 4 Tim Horton 2017-10-06 23:24:00 PDT
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?
Comment 5 Andy Estes 2017-10-07 03:54:18 PDT Comment hidden (obsolete)
Comment 6 Andy Estes 2017-10-07 03:54:44 PDT
(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!
Comment 7 WebKit Commit Bot 2017-10-07 05:07:54 PDT Comment hidden (obsolete)
Comment 8 WebKit Commit Bot 2017-10-07 05:07:56 PDT Comment hidden (obsolete)
Comment 9 Tim Horton 2017-10-07 10:34:45 PDT
(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!
Comment 10 Andy Estes 2017-10-07 11:42:51 PDT
Created attachment 323097 [details]
Patch
Comment 11 WebKit Commit Bot 2017-10-07 12:24:08 PDT
Comment on attachment 323097 [details]
Patch

Clearing flags on attachment: 323097

Committed r223021: <http://trac.webkit.org/changeset/223021>
Comment 12 WebKit Commit Bot 2017-10-07 12:24:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Ryan Haddad 2017-10-09 10:58:04 PDT
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
Comment 14 Ryan Haddad 2017-10-09 11:01:32 PDT
Reverted r223021 for reason:

LayoutTests added with this change are failing.

Committed r223053: <http://trac.webkit.org/changeset/223053>
Comment 15 Andy Estes 2017-10-09 13:35:44 PDT
Created attachment 323210 [details]
Patch
Comment 16 Andy Estes 2017-10-09 13:39:42 PDT
I added a mock PaymentCoordinator so that show() and hide() do not try to invoke actual PassKit UI. Tim reviewed this in person.
Comment 17 Andy Estes 2017-10-09 13:41:25 PDT
Created attachment 323211 [details]
Patch
Comment 18 WebKit Commit Bot 2017-10-09 15:04:24 PDT
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.
Comment 19 WebKit Commit Bot 2017-10-09 15:04:36 PDT
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 20 WebKit Commit Bot 2017-10-09 15:41:02 PDT
Comment on attachment 323211 [details]
Patch

Clearing flags on attachment: 323211

Committed r223076: <http://trac.webkit.org/changeset/223076>
Comment 21 WebKit Commit Bot 2017-10-09 15:41:04 PDT
All reviewed patches have been landed.  Closing bug.