[Desktop Payment Request] Show spinner while querying payment methods.

Before this patch, the Payment Request dialog on desktop would not be
shown after the merchant called PaymentRequest.show(). The dialog would
be shown only after the payment handlers have been queried.

This patch makes the Payment Request dialog UI appear with a
"Processing" spinner when PaymentRequest.show() is called.  When the
payment handler query has finished, the dialog hides the processing
spinner and becomes interactive. A few minor improvements flow from this
change:

- There's no longer a need for a hidden dialog that is eventually shown.
  The dialog is always shown after PaymentRequest.show().

- The tests are updated to expect the processing spinner before the
  "dialog opened" event.

- To better debug incorrect expectations in tests, the variable
  |events_| is renamed into |expected_events_| and the variable |event|
  is renamed into |actual_event| in test_event_waiter.h.

- Because the JourneyLogger is owned privately in PaymentRequest object
  and the "event shown" needs to be recorded before the "dialog opened"
  testing event is fired from the UI, the PaymentRequest object now
  exposes the RecordDialogShownEventInJourneyLogger() method to be
  called from the UI layer.

After this patch, the Payment Request dialog on desktop is shown in a
spinning state with "Processing" message after the merchant calls
PaymentRequest.show(), before the payment handlers have been queried.


Bug: 783811
Change-Id: I820c9e8a093f74b30e5e7f29ef39675f60c17158
Reviewed-on: https://chromium-review.googlesource.com/1159183Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581731}

chrome/browser/payments/chrome_payment_request_delegate.cc

@@ -63,9 +63,8 @@ ChromePaymentRequestDelegate::~ChromePaymentRequestDelegate() {}
 
 void ChromePaymentRequestDelegate::ShowDialog(PaymentRequest* request) {
   DCHECK_EQ(nullptr, shown_dialog_);
-  hidden_dialog_ = std::unique_ptr<PaymentRequestDialog>(
-      chrome::CreatePaymentRequestDialog(request));
-  MaybeShowHiddenDialog(request);
+  shown_dialog_ = chrome::CreatePaymentRequestDialog(request);
+  shown_dialog_->ShowDialog();
 }
 
 void ChromePaymentRequestDelegate::CloseDialog() {
@@ -73,9 +72,6 @@ void ChromePaymentRequestDelegate::CloseDialog() {
     shown_dialog_->CloseDialog();
     shown_dialog_ = nullptr;
   }
-
-  if (hidden_dialog_)
-    hidden_dialog_.reset();
 }
 
 void ChromePaymentRequestDelegate::ShowErrorMessage() {
@@ -180,10 +176,7 @@ ChromePaymentRequestDelegate::GetDisplayManager() {
 void ChromePaymentRequestDelegate::EmbedPaymentHandlerWindow(
     const GURL& url,
     PaymentHandlerOpenWindowCallback callback) {
-  if (hidden_dialog_) {
-    shown_dialog_ = hidden_dialog_.release();
-    shown_dialog_->ShowDialogAtPaymentHandlerSheet(url, std::move(callback));
-  } else if (shown_dialog_) {
+  if (shown_dialog_) {
     shown_dialog_->ShowPaymentHandlerScreen(url, std::move(callback));
   } else {
     std::move(callback).Run(/*success=*/false,
@@ -192,14 +185,4 @@ void ChromePaymentRequestDelegate::EmbedPaymentHandlerWindow(
   }
 }
 
-void ChromePaymentRequestDelegate::MaybeShowHiddenDialog(
-    PaymentRequest* request) {
-  if (request->SatisfiesSkipUIConstraints()) {
-    request->Pay();
-  } else {
-    shown_dialog_ = hidden_dialog_.release();
-    shown_dialog_->ShowDialog();
-  }
-}
-
 }  // namespace payments

chrome/browser/payments/chrome_payment_request_delegate.h

@@ -58,21 +58,6 @@ class ChromePaymentRequestDelegate : public ContentPaymentRequestDelegate {
   // for testing.
   PaymentRequestDialog* shown_dialog_;
 
-  // The instance of the dialog that was created but not shown yet. Since it
-  // hasn't been shown, it's still owned by it's creator. This is non null only
-  // when the current Payment Request supports skipping the payment sheet (see
-  // PaymentRequest::SatisfiesSkipUIConstraints) and is reset once the
-  // underlying pointer becomes owned by the views:: machinery (when the dialog
-  // is shown).
-  std::unique_ptr<PaymentRequestDialog> hidden_dialog_;
-
-  // Shows |hidden_dialog_| if the current Payment Request doesn't support the
-  // skip UI flow. This also transfer its ownership to the views dialog code and
-  // keep a reference to it in |shown_dialog_|.
-  // Otherwise, this calls Pay() on the current Payment Request to allow the
-  // skip UI flow to carry on.
-  void MaybeShowHiddenDialog(PaymentRequest* request);
-
  private:
   // Not owned but outlives the PaymentRequest object that owns this.
   content::WebContents* web_contents_;

chrome/browser/ui/views/payments/modifiers_browsertest.cc

@@ -93,7 +93,7 @@ IN_PROC_BROWSER_TEST_F(
   card.set_card_type(autofill::CreditCard::CardType::CARD_TYPE_CREDIT);
   AddCreditCard(card);
 
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { "
@@ -130,7 +130,7 @@ IN_PROC_BROWSER_TEST_F(
   card.set_card_type(autofill::CreditCard::CardType::CARD_TYPE_CREDIT);
   AddCreditCard(card);
 
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { "
@@ -167,7 +167,7 @@ IN_PROC_BROWSER_TEST_F(
   card.set_card_type(autofill::CreditCard::CardType::CARD_TYPE_CREDIT);
   AddCreditCard(card);
 
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { "
@@ -204,7 +204,7 @@ IN_PROC_BROWSER_TEST_F(
   card.set_card_type(autofill::CreditCard::CardType::CARD_TYPE_CREDIT);
   AddCreditCard(card);
 
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { "
@@ -263,7 +263,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestModifiersTest,
   card.set_billing_address_id(profile.guid());
   AddCreditCard(card);
 
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { "
@@ -300,7 +300,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestModifiersTest,
   card.set_billing_address_id(profile.guid());
   AddCreditCard(card);
 
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { document.getElementById('no_total').click(); })();";

chrome/browser/ui/views/payments/payment_request_browsertest.cc

@@ -234,7 +234,7 @@ class PaymentRequestPaymentMethodIdentifierTest
   PaymentRequestPaymentMethodIdentifierTest() {}
 
   void InvokePaymentRequestWithJs(const std::string& js) {
-    ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+    ResetEventWaiterForDialogOpened();
 
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), js));
 

chrome/browser/ui/views/payments/payment_request_browsertest_base.cc

@@ -254,7 +254,7 @@ void PaymentRequestBrowserTestBase::OnInterfaceRequestFromFrame(
 }
 
 void PaymentRequestBrowserTestBase::InvokePaymentRequestUI() {
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
 
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
@@ -767,6 +767,12 @@ void PaymentRequestBrowserTestBase::ResetEventWaiterForSequence(
       std::move(event_sequence));
 }
 
+void PaymentRequestBrowserTestBase::ResetEventWaiterForDialogOpened() {
+  ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                               DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                               DialogEvent::DIALOG_OPENED});
+}
+
 void PaymentRequestBrowserTestBase::WaitForObservedEvent() {
   event_waiter_->Wait();
 }

chrome/browser/ui/views/payments/payment_request_browsertest_base.h

@@ -253,6 +253,8 @@ class PaymentRequestBrowserTestBase
   // Resets the event waiter for a given |event| or |event_sequence|.
   void ResetEventWaiter(DialogEvent event);
   void ResetEventWaiterForSequence(std::list<DialogEvent> event_sequence);
+  // Resets the event waiter for the events that trigger when opening a dialog.
+  void ResetEventWaiterForDialogOpened();
   // Wait for the event(s) passed to ResetEventWaiter*() to occur.
   void WaitForObservedEvent();
 

chrome/browser/ui/views/payments/payment_request_can_make_payment_metrics_browsertest.cc

@@ -39,6 +39,8 @@ class PaymentRequestCanMakePaymentMetricsTest
     // the Payment Request is shown.
     ResetEventWaiterForSequence({DialogEvent::CAN_MAKE_PAYMENT_CALLED,
                                  DialogEvent::CAN_MAKE_PAYMENT_RETURNED,
+                                 DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
                                  DialogEvent::DIALOG_OPENED});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "queryShow();"));
     WaitForObservedEvent();
@@ -388,7 +390,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCanMakePaymentMetricsTest,
 
   // Start the Payment Request, CanMakePayment should not be called in this
   // test.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -430,7 +432,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCanMakePaymentMetricsTest,
 
   // Start the Payment Request, CanMakePayment should not be called in this
   // test.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -478,7 +480,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCanMakePaymentMetricsTest,
 
   // Start the Payment Request, CanMakePayment should not be called in this
   // test.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 

chrome/browser/ui/views/payments/payment_request_completion_status_metrics_browsertest.cc

@@ -43,6 +43,8 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest, Completed) {
   // Payment Request is shown.
   ResetEventWaiterForSequence({DialogEvent::CAN_MAKE_PAYMENT_CALLED,
                                DialogEvent::CAN_MAKE_PAYMENT_RETURNED,
+                               DialogEvent::PROCESSING_SPINNER_SHOWN,
+                               DialogEvent::PROCESSING_SPINNER_HIDDEN,
                                DialogEvent::DIALOG_OPENED});
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "queryShow();"));
   WaitForObservedEvent();
@@ -80,7 +82,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -125,7 +127,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -172,7 +174,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -220,7 +222,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -262,7 +264,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -304,7 +306,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 
@@ -348,7 +350,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestCompletionStatusMetricsTest,
   base::HistogramTester histogram_tester;
 
   // Start the Payment Request.
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "noQueryShow();"));
   WaitForObservedEvent();
 

chrome/browser/ui/views/payments/payment_request_dialog_view.cc

@@ -83,6 +83,11 @@ PaymentRequestDialogView::PaymentRequestDialogView(
   if (!request->state()->is_get_all_instruments_finished()) {
     request->state()->AddObserver(this);
     ShowProcessingSpinner();
+  } else if (observer_for_testing_) {
+    // When testing, signal that the processing spinner events have passed, even
+    // though the UI does not need to show it.
+    observer_for_testing_->OnProcessingSpinnerShown();
+    observer_for_testing_->OnProcessingSpinnerHidden();
   }
 
   ShowInitialPaymentSheet();
@@ -212,13 +217,16 @@ void PaymentRequestDialogView::OnSpecUpdated() {
 
 void PaymentRequestDialogView::OnGetAllPaymentInstrumentsFinished() {
   HideProcessingSpinner();
-  if (observer_for_testing_) {
-    // The OnGetAllPaymentInstrumentsFinished() method is called if the payment
-    // instruments were retrieved asynchronously. This method hides the
-    // "Processing" spinner, so the UI is now ready for interaction. Any test
-    // that opens UI can now interact with it. The OnDialogOpened() call
-    // notifies the tests of this event.
-    observer_for_testing_->OnDialogOpened();
+  if (request_->state()->are_requested_methods_supported()) {
+    request_->RecordDialogShownEventInJourneyLogger();
+    if (observer_for_testing_) {
+      // The OnGetAllPaymentInstrumentsFinished() method is called if the
+      // payment instruments were retrieved asynchronously. This method hides
+      // the "Processing" spinner, so the UI is now ready for interaction. Any
+      // test that opens UI can now interact with it. The OnDialogOpened() call
+      // notifies the tests of this event.
+      observer_for_testing_->OnDialogOpened();
+    }
   }
 }
 
@@ -385,14 +393,16 @@ void PaymentRequestDialogView::ShowInitialPaymentSheet() {
                             request_->spec(), request_->state(), this),
                         &controller_map_),
                     /* animate = */ false);
-  if (observer_for_testing_ &&
-      request_->state()->is_get_all_instruments_finished()) {
-    // The is_get_all_instruments_finished() method returns true if all payment
-    // instruments were retrieved synchronously. There's no "Processing" spinner
-    // to hide, so the UI is ready instantly. Any test that opens UI can now
-    // interact with it. The OnDialogOpened() call notifies the tests of this
-    // event.
-    observer_for_testing_->OnDialogOpened();
+  if (request_->state()->is_get_all_instruments_finished() &&
+      request_->state()->are_requested_methods_supported()) {
+    request_->RecordDialogShownEventInJourneyLogger();
+    if (observer_for_testing_) {
+      // The is_get_all_instruments_finished() method returns true if all
+      // payment instruments were retrieved synchronously. Any test that opens
+      // UI can now interact with it. The OnDialogOpened() call notifies the
+      // tests of this event.
+      observer_for_testing_->OnDialogOpened();
+    }
   }
 }
 

chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc

@@ -97,7 +97,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestJourneyLoggerNoSupportedPaymentMethodTest,
   NavigateTo("/payment_request_bobpay_test.html");
   base::HistogramTester histogram_tester;
 
-  ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+  ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                               DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                               DialogEvent::NOT_SUPPORTED_ERROR});
   content::WebContents* web_contents = GetActiveWebContents();
   const std::string click_buy_button_js =
       "(function() { document.getElementById('buy').click(); })();";
@@ -1045,7 +1047,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestIframeTest, CrossOriginIframe) {
       browser()->tab_strip_model()->GetActiveWebContents();
   GURL iframe_url =
       https_server()->GetURL("b.com", "/payment_request_iframe.html");
-  ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+  ResetEventWaiterForDialogOpened();
   EXPECT_TRUE(content::NavigateIframeToURL(tab, "test", iframe_url));
   WaitForObservedEvent();
 

chrome/browser/ui/views/payments/payment_request_no_update_with_browsertest.cc

@@ -18,7 +18,7 @@ class PaymentRequestNoUpdateWithTest : public PaymentRequestBrowserTestBase {
 
   void RunJavaScriptFunctionToOpenPaymentRequestUI(
       const std::string& function_name) {
-    ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+    ResetEventWaiterForDialogOpened();
 
     content::WebContents* web_contents = GetActiveWebContents();
     ASSERT_TRUE(content::ExecuteScript(web_contents, function_name + "();"));

chrome/browser/ui/views/payments/payment_request_payment_app_browsertest.cc

@@ -172,7 +172,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, NotSupportedError) {
     // SetDownloaderAndIgnorePortInAppScopeForTesting again.
     SetDownloaderAndIgnorePortInAppScopeForTesting();
 
-    ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+    ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                                 DialogEvent::NOT_SUPPORTED_ERROR});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "buy();"));
     WaitForObservedEvent();
     ExpectBodyContains({"NotSupportedError"});
@@ -195,7 +197,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, NotSupportedError) {
     // SetDownloaderAndIgnorePortInAppScopeForTesting again.
     SetDownloaderAndIgnorePortInAppScopeForTesting();
 
-    ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+    ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                                 DialogEvent::NOT_SUPPORTED_ERROR});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "buy();"));
     WaitForObservedEvent();
     ExpectBodyContains({"NotSupportedError"});
@@ -332,7 +336,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, BlockAlicePay) {
     // SetDownloaderAndIgnorePortInAppScopeForTesting again.
     SetDownloaderAndIgnorePortInAppScopeForTesting();
 
-    ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+    ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                                 DialogEvent::NOT_SUPPORTED_ERROR});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "buy();"));
     WaitForObservedEvent();
     ExpectBodyContains({"NotSupportedError"});
@@ -355,7 +361,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, BlockAlicePay) {
     // SetDownloaderAndIgnorePortInAppScopeForTesting again.
     SetDownloaderAndIgnorePortInAppScopeForTesting();
 
-    ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+    ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                                 DialogEvent::NOT_SUPPORTED_ERROR});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "buy();"));
     WaitForObservedEvent();
     ExpectBodyContains({"NotSupportedError"});
@@ -382,7 +390,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, CanNotPayWithBobPay) {
     // SetDownloaderAndIgnorePortInAppScopeForTesting again.
     SetDownloaderAndIgnorePortInAppScopeForTesting();
 
-    ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+    ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                                 DialogEvent::NOT_SUPPORTED_ERROR});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "buy();"));
     WaitForObservedEvent();
     ExpectBodyContains({"NotSupportedError"});
@@ -405,7 +415,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, CanNotPayWithBobPay) {
     // SetDownloaderAndIgnorePortInAppScopeForTesting again.
     SetDownloaderAndIgnorePortInAppScopeForTesting();
 
-    ResetEventWaiter(DialogEvent::NOT_SUPPORTED_ERROR);
+    ResetEventWaiterForSequence({DialogEvent::PROCESSING_SPINNER_SHOWN,
+                                 DialogEvent::PROCESSING_SPINNER_HIDDEN,
+                                 DialogEvent::NOT_SUPPORTED_ERROR});
     ASSERT_TRUE(content::ExecuteScript(GetActiveWebContents(), "buy();"));
     WaitForObservedEvent();
     ExpectBodyContains({"NotSupportedError"});
@@ -462,7 +474,9 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest, SkipUIEnabledWithBobPay) {
     // Since the skip UI flow is available, the request will complete without
     // interaction besides hitting "pay" on the website.
     ResetEventWaiterForSequence(
-        {DialogEvent::DIALOG_OPENED, DialogEvent::DIALOG_CLOSED});
+        {DialogEvent::PROCESSING_SPINNER_SHOWN,
+         DialogEvent::PROCESSING_SPINNER_HIDDEN, DialogEvent::DIALOG_OPENED,
+         DialogEvent::PROCESSING_SPINNER_SHOWN, DialogEvent::DIALOG_CLOSED});
     content::WebContents* web_contents = GetActiveWebContents();
     const std::string click_buy_button_js =
         "(function() { document.getElementById('buy').click(); })();";
@@ -522,7 +536,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestPaymentAppTest,
     // Since the skip UI flow is not available because the payer's email is
     // requested, the request will complete only after clicking on the Pay
     // button in the dialog.
-    ResetEventWaiter(DialogEvent::DIALOG_OPENED);
+    ResetEventWaiterForDialogOpened();
     content::WebContents* web_contents = GetActiveWebContents();
     const std::string click_buy_button_js =
         "(function() { "

chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc

@@ -24,9 +24,8 @@ TestChromePaymentRequestDelegate::TestChromePaymentRequestDelegate(
       is_browser_window_active_(is_browser_window_active) {}
 
 void TestChromePaymentRequestDelegate::ShowDialog(PaymentRequest* request) {
-  hidden_dialog_ =
-      std::make_unique<PaymentRequestDialogView>(request, observer_);
-  MaybeShowHiddenDialog(request);
+  shown_dialog_ = new PaymentRequestDialogView(request, observer_);
+  shown_dialog_->ShowDialog();
 }
 
 bool TestChromePaymentRequestDelegate::IsIncognito() const {

components/autofill/core/browser/test_event_waiter.h

@@ -31,7 +31,7 @@ template <typename Event>
 class EventWaiter {
  public:
   explicit EventWaiter(
-      std::list<Event> event_sequence,
+      std::list<Event> expected_event_sequence,
       base::TimeDelta timeout = base::TimeDelta::FromSeconds(0));
   ~EventWaiter();
 
@@ -44,16 +44,16 @@ class EventWaiter {
   void OnEvent(Event event);
 
  private:
-  std::list<Event> events_;
+  std::list<Event> expected_events_;
   base::RunLoop run_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(EventWaiter);
 };
 
 template <typename Event>
-EventWaiter<Event>::EventWaiter(std::list<Event> event_sequence,
+EventWaiter<Event>::EventWaiter(std::list<Event> expected_event_sequence,
                                 base::TimeDelta timeout)
-    : events_(std::move(event_sequence)) {
+    : expected_events_(std::move(expected_event_sequence)) {
   if (!timeout.is_zero()) {
     base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE, run_loop_.QuitClosure(), timeout);
@@ -65,23 +65,23 @@ EventWaiter<Event>::~EventWaiter() {}
 
 template <typename Event>
 bool EventWaiter<Event>::Wait() {
-  if (events_.empty())
+  if (expected_events_.empty())
     return true;
 
   DCHECK(!run_loop_.running());
   run_loop_.Run();
-  return events_.empty();
+  return expected_events_.empty();
 }
 
 template <typename Event>
-void EventWaiter<Event>::OnEvent(Event event) {
-  if (events_.empty())
+void EventWaiter<Event>::OnEvent(Event actual_event) {
+  if (expected_events_.empty())
     return;
 
-  ASSERT_EQ(events_.front(), event);
-  events_.pop_front();
+  ASSERT_EQ(expected_events_.front(), actual_event);
+  expected_events_.pop_front();
   // Only quit the loop if no other events are expected.
-  if (events_.empty() && run_loop_.running())
+  if (expected_events_.empty() && run_loop_.running())
     run_loop_.Quit();
 }
 

components/payments/content/payment_request.cc

@@ -93,16 +93,8 @@ void PaymentRequest::Init(mojom::PaymentRequestClientPtr client,
     LOG(ERROR) << "SSL certificate is not valid";
 
   if (!allowed_origin || invalid_ssl) {
-    // Don't show UI. Resolve .canMakepayment() with "false". Reject .show()
-    // with "NotSupportedError".
-    spec_ = std::make_unique<PaymentRequestSpec>(
-        mojom::PaymentOptions::New(), mojom::PaymentDetails::New(),
-        std::vector<mojom::PaymentMethodDataPtr>(), this,
-        delegate_->GetApplicationLocale());
-    state_ = std::make_unique<PaymentRequestState>(
-        web_contents_, top_level_origin_, frame_origin_, spec_.get(), this,
-        delegate_->GetApplicationLocale(), delegate_->GetPersonalDataManager(),
-        delegate_.get(), &journey_logger_);
+    // Intentionally don't set |spec_| and |state_|. Don't show UI. Resolve
+    // .canMakepayment() with "false". Reject .show() with "NotSupportedError".
     return;
   }
 
@@ -178,10 +170,16 @@ void PaymentRequest::Show(bool is_user_gesture) {
     return;
   }
 
+  if (!state_) {
+    // SSL is not valid.
+    AreRequestedMethodsSupportedCallback(false);
+    return;
+  }
+
   is_show_user_gesture_ = is_user_gesture;
 
-  // TODO(crbug.com/783811): Display a spinner when checking whether
-  // the methods are supported asynchronously for better user experience.
+  display_handle_->Show(this);
+
   state_->AreRequestedMethodsSupported(
       base::BindOnce(&PaymentRequest::AreRequestedMethodsSupportedCallback,
                      weak_ptr_factory_.GetWeakPtr()));
@@ -205,10 +203,8 @@ void PaymentRequest::Retry(mojom::PaymentValidationErrorsPtr errors) {
 void PaymentRequest::AreRequestedMethodsSupportedCallback(
     bool methods_supported) {
   if (methods_supported) {
-    journey_logger_.SetEventOccurred(JourneyLogger::EVENT_SHOWN);
-
-    DCHECK(display_handle_);
-    display_handle_->Show(this);
+    if (SatisfiesSkipUIConstraints())
+      Pay();
   } else {
     journey_logger_.SetNotShown(
         JourneyLogger::NOT_SHOWN_REASON_NO_SUPPORTED_PAYMENT_METHOD);
@@ -285,10 +281,11 @@ void PaymentRequest::CanMakePayment() {
   if (observer_for_testing_)
     observer_for_testing_->OnCanMakePaymentCalled();
 
-  if (!delegate_->GetPrefService()->GetBoolean(kCanMakePaymentEnabled)) {
+  if (!delegate_->GetPrefService()->GetBoolean(kCanMakePaymentEnabled) ||
+      !state_) {
     CanMakePaymentCallback(/*can_make_payment=*/false);
   } else {
-    state()->CanMakePayment(
+    state_->CanMakePayment(
         base::BindOnce(&PaymentRequest::CanMakePaymentCallback,
                        weak_ptr_factory_.GetWeakPtr()));
   }
@@ -389,6 +386,10 @@ void PaymentRequest::HideIfNecessary() {
   display_handle_.reset();
 }
 
+void PaymentRequest::RecordDialogShownEventInJourneyLogger() {
+  journey_logger_.SetEventOccurred(JourneyLogger::EVENT_SHOWN);
+}
+
 bool PaymentRequest::IsIncognito() const {
   return delegate_->IsIncognito();
 }
@@ -415,10 +416,10 @@ void PaymentRequest::RecordFirstAbortReason(
 }
 
 void PaymentRequest::CanMakePaymentCallback(bool can_make_payment) {
-  if (CanMakePaymentQueryFactory::GetInstance()
-          ->GetForContext(web_contents_->GetBrowserContext())
-          ->CanQuery(top_level_origin_, frame_origin_,
-                     spec()->stringified_method_data())) {
+  if (!spec_ || CanMakePaymentQueryFactory::GetInstance()
+                    ->GetForContext(web_contents_->GetBrowserContext())
+                    ->CanQuery(top_level_origin_, frame_origin_,
+                               spec_->stringified_method_data())) {
     RespondToCanMakePaymentQuery(can_make_payment, false);
   } else if (OriginSecurityChecker::IsOriginLocalhostOrFile(frame_origin_)) {
     RespondToCanMakePaymentQuery(can_make_payment, true);

components/payments/content/payment_request.h

@@ -102,6 +102,9 @@ class PaymentRequest : public mojom::PaymentRequest,
   // Hide this Payment Request if it's already showing.
   void HideIfNecessary();
 
+  // Record the "dialog shown" event in the journey logger.
+  void RecordDialogShownEventInJourneyLogger();
+
   bool IsIncognito() const;
 
   // Returns true if this payment request supports skipping the Payment Sheet.

components/payments/content/payment_request_display_manager.h

@@ -31,8 +31,8 @@ class PaymentRequestDisplayManager : public KeyedService {
  public:
   class DisplayHandle {
    public:
-    explicit DisplayHandle(PaymentRequestDisplayManager* display_manager,
-                           ContentPaymentRequestDelegate* delegate);
+    DisplayHandle(PaymentRequestDisplayManager* display_manager,
+                  ContentPaymentRequestDelegate* delegate);
     ~DisplayHandle();
     void Show(PaymentRequest* request);
     // Attempt to display |url| inside the Payment Request dialog and run

components/payments/content/payment_request_state.cc

@@ -46,6 +46,8 @@ PaymentRequestState::PaymentRequestState(
       delegate_(delegate),
       personal_data_manager_(personal_data_manager),
       journey_logger_(journey_logger),
+      are_requested_methods_supported_(
+          !spec_->supported_card_networks().empty()),
       selected_shipping_profile_(nullptr),
       selected_shipping_option_error_profile_(nullptr),
       selected_contact_profile_(nullptr),
@@ -140,6 +142,7 @@ void PaymentRequestState::FinishedGetAllSWPaymentInstruments() {
   SetDefaultProfileSelections();
 
   get_all_instruments_finished_ = true;
+  are_requested_methods_supported_ |= !available_instruments_.empty();
   NotifyOnGetAllPaymentInstrumentsFinished();
 
   // Fullfill the pending CanMakePayment call.
@@ -208,8 +211,7 @@ void PaymentRequestState::CheckRequestedMethodsSupported(
     StatusCallback callback) {
   DCHECK(get_all_instruments_finished_);
 
-  std::move(callback).Run(!spec_->supported_card_networks().empty() ||
-                          !available_instruments_.empty());
+  std::move(callback).Run(are_requested_methods_supported_);
 }
 
 std::string PaymentRequestState::GetAuthenticatedEmail() const {

components/payments/content/payment_request_state.h

@@ -182,6 +182,13 @@ class PaymentRequestState : public PaymentResponseHelper::Delegate,
     return get_all_instruments_finished_;
   }
 
+  // Returns true after is_get_all_instruments_finished() is true and supported
+  // payment method are found. Should not be called before
+  // is_get_all_instruments_finished() is true.
+  bool are_requested_methods_supported() const {
+    return are_requested_methods_supported_;
+  }
+
   const std::string& GetApplicationLocale();
   autofill::PersonalDataManager* GetPersonalDataManager();
   autofill::RegionDataLoader* GetRegionDataLoader();
@@ -270,6 +277,7 @@ class PaymentRequestState : public PaymentResponseHelper::Delegate,
 
   StatusCallback can_make_payment_callback_;
   StatusCallback are_requested_methods_supported_callback_;
+  bool are_requested_methods_supported_;
 
   autofill::AutofillProfile* selected_shipping_profile_;
   autofill::AutofillProfile* selected_shipping_option_error_profile_;