[Payments] Record CanMakePayment metrics on Desktop

Plug in the JourneyLogger on the Desktop code and use it
to record CanMakePayment metrics.

BUG=716483

Review-Url: https://codereview.chromium.org/2851893002
Cr-Commit-Position: refs/heads/master@{#469447}

chrome/browser/payments/android/journey_logger_android.cc

@@ -94,7 +94,7 @@ void JourneyLoggerAndroid::SetEventOccurred(
     const base::android::JavaParamRef<jobject>& jcaller,
     jint jevent) {
   DCHECK_GE(jevent, 0);
-  DCHECK_LT(jevent, JourneyLogger::Event::EVENT_MAX);
+  DCHECK_LT(jevent, JourneyLogger::Event::EVENT_ENUM_MAX);
   journey_logger_.SetEventOccurred(static_cast<JourneyLogger::Event>(jevent));
 }
 

chrome/browser/payments/chrome_payment_request_delegate.cc

@@ -184,4 +184,8 @@ AddressNormalizer* ChromePaymentRequestDelegate::GetAddressNormalizer() {
   return &address_normalizer_;
 }
 
+ukm::UkmService* ChromePaymentRequestDelegate::GetUkmService() {
+  return g_browser_process->ukm_service();
+}
+
 }  // namespace payments

chrome/browser/payments/chrome_payment_request_delegate.h

@@ -40,6 +40,7 @@ class ChromePaymentRequestDelegate : public PaymentRequestDelegate {
           result_delegate) override;
   AddressNormalizer* GetAddressNormalizer() override;
   autofill::RegionDataLoader* GetRegionDataLoader() override;
+  ukm::UkmService* GetUkmService() override;
 
  protected:
   // Reference to the dialog so that we can satisfy calls to CloseDialog(). This

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

@@ -53,8 +53,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestNoShippingTest, OpenAndNavigateTo404) {
 
   ResetEventObserver(DialogEvent::DIALOG_CLOSED);
 
-  ui_test_utils::NavigateToURL(browser(),
-                               https_server()->GetURL("/non-existent.html"));
+  NavigateTo("/non-existent.html");
 
   WaitForObservedEvent();
 }
@@ -64,9 +63,7 @@ IN_PROC_BROWSER_TEST_F(PaymentRequestNoShippingTest, OpenAndNavigateToSame) {
 
   ResetEventObserver(DialogEvent::DIALOG_CLOSED);
 
-  ui_test_utils::NavigateToURL(
-      browser(),
-      https_server()->GetURL("/payment_request_no_shipping_test.html"));
+  NavigateTo("/payment_request_no_shipping_test.html");
 
   WaitForObservedEvent();
 }

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

@@ -59,14 +59,14 @@ PaymentRequestBrowserTestBase::PaymentRequestBrowserTestBase(
 PaymentRequestBrowserTestBase::~PaymentRequestBrowserTestBase() {}
 
 void PaymentRequestBrowserTestBase::SetUpOnMainThread() {
+  // Setup the https server.
   https_server_ = base::MakeUnique<net::EmbeddedTestServer>(
       net::EmbeddedTestServer::TYPE_HTTPS);
   ASSERT_TRUE(https_server_->InitializeAndListen());
   https_server_->ServeFilesFromSourceDirectory("chrome/test/data/payments");
   https_server_->StartAcceptingConnections();
 
-  GURL url = https_server()->GetURL(test_file_path_);
-  ui_test_utils::NavigateToURL(browser(), url);
+  NavigateTo(test_file_path_);
 
   // Starting now, PaymentRequest Mojo messages sent by the renderer will
   // create PaymentRequest objects via this test's CreatePaymentRequestForTest,
@@ -80,6 +80,10 @@ void PaymentRequestBrowserTestBase::SetUpOnMainThread() {
                  base::Unretained(this), web_contents));
 }
 
+void PaymentRequestBrowserTestBase::NavigateTo(const std::string& file_path) {
+  ui_test_utils::NavigateToURL(browser(), https_server()->GetURL(file_path));
+}
+
 void PaymentRequestBrowserTestBase::SetIncognito() {
   is_incognito_ = true;
 }
@@ -332,6 +336,12 @@ void PaymentRequestBrowserTestBase::ClickOnBackArrow() {
   ClickOnDialogViewAndWait(DialogViewID::BACK_BUTTON);
 }
 
+void PaymentRequestBrowserTestBase::ClickOnCancel() {
+  ResetEventObserver(DialogEvent::DIALOG_CLOSED);
+
+  ClickOnDialogViewAndWait(DialogViewID::CANCEL_BUTTON, false);
+}
+
 content::WebContents* PaymentRequestBrowserTestBase::GetActiveWebContents() {
   return browser()->tab_strip_model()->GetActiveWebContents();
 }

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

@@ -74,6 +74,8 @@ class PaymentRequestBrowserTestBase
 
   void SetUpOnMainThread() override;
 
+  void NavigateTo(const std::string& file_path);
+
   void SetIncognito();
   void SetInvalidSsl();
 
@@ -121,6 +123,7 @@ class PaymentRequestBrowserTestBase
   void OpenShippingAddressEditorScreen();
   void OpenContactInfoEditorScreen();
   void ClickOnBackArrow();
+  void ClickOnCancel();
 
   content::WebContents* GetActiveWebContents();
 

chrome/test/BUILD.gn

@@ -1985,6 +1985,7 @@ test("browser_tests") {
         "../browser/ui/views/payments/payment_request_browsertest_base.cc",
         "../browser/ui/views/payments/payment_request_browsertest_base.h",
         "../browser/ui/views/payments/payment_request_can_make_payment_browsertest.cc",
+        "../browser/ui/views/payments/payment_request_can_make_payment_metrics_browsertest.cc",
         "../browser/ui/views/payments/payment_request_payment_app_browsertest.cc",
         "../browser/ui/views/payments/payment_request_payment_response_browsertest.cc",
         "../browser/ui/views/payments/payment_sheet_view_controller_browsertest.cc",

chrome/test/data/payments/can_make_payment_metrics.js

@@ -7,12 +7,14 @@
 /* global PaymentRequest:false */
 /* global print:false */
 
+var request;
+
 /**
  * Do not query CanMakePayment before showing the Payment Request.
  */
 function noQueryShow() {  // eslint-disable-line no-unused-vars
   try {
-    var request = new PaymentRequest(
+    request = new PaymentRequest(
         [{supportedMethods: ['https://bobpay.com', 'visa']}],
         {total: {label: 'Total', amount: {currency: 'USD', value: '5.00'}}});
     request.show()
@@ -34,7 +36,7 @@ function noQueryShow() {  // eslint-disable-line no-unused-vars
  */
 function queryShow() {  // eslint-disable-line no-unused-vars
   try {
-    var request = new PaymentRequest(
+    request = new PaymentRequest(
         [{supportedMethods: ['https://bobpay.com', 'visa']}],
         {total: {label: 'Total', amount: {currency: 'USD', value: '5.00'}}});
     request.canMakePayment()
@@ -59,7 +61,7 @@ function queryShow() {  // eslint-disable-line no-unused-vars
  */
 function queryNoShow() {  // eslint-disable-line no-unused-vars
   try {
-    var request = new PaymentRequest(
+    request = new PaymentRequest(
         [{supportedMethods: ['https://bobpay.com', 'visa']}],
         {total: {label: 'Total', amount: {currency: 'USD', value: '5.00'}}});
     request.canMakePayment()
@@ -69,3 +71,18 @@ function queryNoShow() {  // eslint-disable-line no-unused-vars
     print(error.message);
   }
 }
+
+/**
+ * Aborts the PaymentRequest UI.
+ */
+function abort() {  // eslint-disable-line no-unused-vars
+  try {
+    request.abort().then(() => {
+      print('Aborted');
+    }).catch(() => {
+      print('Cannot abort');
+    });
+  } catch (error) {
+    print(error.message);
+  }
+}

chrome/test/data/payments/payment_request_can_make_payment_metrics_test.html

@@ -15,6 +15,7 @@ found in the LICENSE file.
 <button onclick="noQueryShow()" id="noQueryShow">No Query Show Test</button><br>
 <button onclick="queryShow()" id="queryShow">Query Show Test</button><br>
 <button onclick="queryNoShow()" id="queryNoShow">Query No Show Test</button><br>
+<button onclick="abort()" id="abort">Abort</button><br>
 <pre id="result"></pre>
 <script src="util.js"></script>
 <script src="can_make_payment_metrics.js"></script>

components/payments/content/payment_request.cc

@@ -26,7 +26,10 @@ PaymentRequest::PaymentRequest(
       delegate_(std::move(delegate)),
       manager_(manager),
       binding_(this, std::move(request)),
-      observer_for_testing_(observer_for_testing) {
+      observer_for_testing_(observer_for_testing),
+      journey_logger_(delegate_->IsIncognito(),
+                      web_contents_->GetLastCommittedURL(),
+                      delegate_->GetUkmService()) {
   // OnConnectionTerminated will be called when the Mojo pipe is closed. This
   // will happen as a result of many renderer-side events (both successful and
   // erroneous in nature).
@@ -104,6 +107,7 @@ void PaymentRequest::Show() {
     return;
   }
 
+  journey_logger_.SetShowCalled();
   delegate_->ShowDialog(this);
 }
 
@@ -121,6 +125,9 @@ void PaymentRequest::Abort() {
   // The API user has decided to abort. We return a successful abort message to
   // the renderer, which closes the Mojo message pipe, which triggers
   // PaymentRequest::OnConnectionTerminated, which destroys this object.
+  // TODO(crbug.com/716546): Add a merchant abort metric,
+  journey_logger_.RecordJourneyStatsHistograms(
+      JourneyLogger::COMPLETION_STATUS_OTHER_ABORTED);
   if (client_.is_bound())
     client_->OnAbort(true /* aborted_successfully */);
 }
@@ -132,6 +139,8 @@ void PaymentRequest::Complete(mojom::PaymentComplete result) {
   if (result != mojom::PaymentComplete::SUCCESS) {
     delegate_->ShowErrorMessage();
   } else {
+    journey_logger_.RecordJourneyStatsHistograms(
+        JourneyLogger::COMPLETION_STATUS_COMPLETED);
     // When the renderer closes the connection,
     // PaymentRequest::OnConnectionTerminated will be called.
     client_->OnComplete();
@@ -145,6 +154,8 @@ void PaymentRequest::CanMakePayment() {
       delegate_->IsIncognito() || state()->CanMakePayment()
           ? mojom::CanMakePaymentQueryResult::CAN_MAKE_PAYMENT
           : mojom::CanMakePaymentQueryResult::CANNOT_MAKE_PAYMENT);
+  journey_logger_.SetCanMakePaymentValue(delegate_->IsIncognito() ||
+                                         state()->CanMakePayment());
   if (observer_for_testing_)
     observer_for_testing_->OnCanMakePaymentCalled();
 }
@@ -170,6 +181,9 @@ void PaymentRequest::UserCancelled() {
   if (!client_.is_bound())
     return;
 
+  journey_logger_.RecordJourneyStatsHistograms(
+      JourneyLogger::COMPLETION_STATUS_USER_ABORTED);
+
   // This sends an error to the renderer, which informs the API user.
   client_->OnError(mojom::PaymentErrorReason::USER_CANCEL);
 

components/payments/content/payment_request.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "components/payments/content/payment_request_spec.h"
 #include "components/payments/content/payment_request_state.h"
+#include "components/payments/core/journey_logger.h"
 #include "components/payments/core/payment_request_delegate.h"
 #include "components/payments/mojom/payment_request.mojom.h"
 #include "mojo/public/cpp/bindings/binding.h"
@@ -102,6 +103,8 @@ class PaymentRequest : public mojom::PaymentRequest,
   // May be null, must outlive this object.
   ObserverForTest* observer_for_testing_;
 
+  JourneyLogger journey_logger_;
+
   DISALLOW_COPY_AND_ASSIGN(PaymentRequest);
 };
 

components/payments/core/autofill_payment_instrument_unittest.cc

@@ -47,6 +47,97 @@ class FakePaymentInstrumentDelegate : public PaymentInstrument::Delegate {
   bool on_instrument_details_error_called_ = false;
 };
 
+class FakeAddressNormalizer : public AddressNormalizer {
+ public:
+  FakeAddressNormalizer() {}
+
+  void LoadRulesForRegion(const std::string& region_code) override {}
+
+  bool AreRulesLoadedForRegion(const std::string& region_code) override {
+    return true;
+  }
+
+  void StartAddressNormalization(
+      const autofill::AutofillProfile& profile,
+      const std::string& region_code,
+      int timeout_seconds,
+      AddressNormalizer::Delegate* requester) override {
+    profile_ = profile;
+    requester_ = requester;
+  }
+
+  void OnAddressValidationRulesLoaded(const std::string& region_code,
+                                      bool success) override {}
+
+  void CompleteAddressNormalization() {
+    requester_->OnAddressNormalized(profile_);
+  }
+
+ private:
+  autofill::AutofillProfile profile_;
+  AddressNormalizer::Delegate* requester_;
+};
+
+class FakePaymentRequestDelegate : public PaymentRequestDelegate {
+ public:
+  FakePaymentRequestDelegate()
+      : locale_("en-US"), last_committed_url_("https://shop.com") {}
+  void ShowDialog(PaymentRequest* request) override {}
+
+  void CloseDialog() override {}
+
+  void ShowErrorMessage() override {}
+
+  autofill::PersonalDataManager* GetPersonalDataManager() override {
+    return nullptr;
+  }
+
+  const std::string& GetApplicationLocale() const override { return locale_; }
+
+  bool IsIncognito() const override { return false; }
+
+  bool IsSslCertificateValid() override { return true; }
+
+  const GURL& GetLastCommittedURL() const override {
+    return last_committed_url_;
+  }
+
+  void DoFullCardRequest(
+      const autofill::CreditCard& credit_card,
+      base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
+          result_delegate) override {
+    full_card_request_card_ = credit_card;
+    full_card_result_delegate_ = result_delegate;
+  }
+
+  AddressNormalizer* GetAddressNormalizer() override {
+    return &address_normalizer_;
+  }
+
+  FakeAddressNormalizer* GetTestAddressNormalizer() {
+    return &address_normalizer_;
+  }
+
+  void CompleteFullCardRequest() {
+    full_card_result_delegate_->OnFullCardRequestSucceeded(
+        full_card_request_card_, base::ASCIIToUTF16("123"));
+  }
+
+  autofill::RegionDataLoader* GetRegionDataLoader() override { return nullptr; }
+
+  ukm::UkmService* GetUkmService() override { return nullptr; }
+
+ private:
+  std::string locale_;
+  const GURL last_committed_url_;
+  FakeAddressNormalizer address_normalizer_;
+
+  autofill::CreditCard full_card_request_card_;
+  base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
+      full_card_result_delegate_;
+  DISALLOW_COPY_AND_ASSIGN(FakePaymentRequestDelegate);
+};
+
 }  // namespace
 
 class AutofillPaymentInstrumentTest : public testing::Test {

components/payments/core/journey_logger.cc

@@ -67,10 +67,7 @@ std::string GetHistogramNameSuffix(
 JourneyLogger::JourneyLogger(bool is_incognito,
                              const GURL& url,
                              ukm::UkmService* ukm_service)
-    : was_can_make_payments_used_(false),
-      could_make_payment_(false),
-      was_show_called_(false),
-      is_incognito_(is_incognito),
+    : is_incognito_(is_incognito),
       events_(EVENT_INITIATED),
       url_(url),
       ukm_service_(ukm_service) {}
@@ -113,6 +110,9 @@ void JourneyLogger::SetEventOccurred(Event event) {
 
 void JourneyLogger::RecordJourneyStatsHistograms(
     CompletionStatus completion_status) {
+  DCHECK(!has_recorded_);
+  has_recorded_ = true;
+
   RecordSectionSpecificStats(completion_status);
 
   // Record the CanMakePayment metrics based on whether the transaction was

components/payments/core/journey_logger.h

@@ -66,7 +66,7 @@ class JourneyLogger {
     EVENT_PAY_CLICKED = 1 << 1,
     EVENT_RECEIVED_INSTRUMENT_DETAILS = 1 << 2,
     EVENT_SKIPPED_SHOW = 1 << 3,
-    EVENT_MAX = 16,
+    EVENT_ENUM_MAX = 16,
   };
 
   // Used to mesure the impact of the CanMakePayment return value on whether the
@@ -157,9 +157,10 @@ class JourneyLogger {
   void RecordUrlKeyedMetrics(CompletionStatus completion_status);
 
   SectionStats sections_[NUMBER_OF_SECTIONS];
-  bool was_can_make_payments_used_;
-  bool could_make_payment_;
-  bool was_show_called_;
+  bool has_recorded_ = false;
+  bool was_can_make_payments_used_ = false;
+  bool could_make_payment_ = false;
+  bool was_show_called_ = false;
   bool is_incognito_;
 
   // Accumulates the many events that have happened during the Payment Request.

components/payments/core/payment_request_delegate.h

@@ -19,6 +19,10 @@ class PersonalDataManager;
 class RegionDataLoader;
 }  // namespace autofill
 
+namespace ukm {
+class UkmService;
+}  // namespace ukm
+
 namespace payments {
 
 class AddressNormalizer;
@@ -67,6 +71,9 @@ class PaymentRequestDelegate {
 
   // Creates a new region data loader that will self delete, or a test mock.
   virtual autofill::RegionDataLoader* GetRegionDataLoader() = 0;
+
+  // Returns a pointer to the UKM service.
+  virtual ukm::UkmService* GetUkmService() = 0;
 };
 
 }  // namespace payments

components/payments/core/test_payment_request_delegate.cc

@@ -59,6 +59,10 @@ autofill::RegionDataLoader* TestPaymentRequestDelegate::GetRegionDataLoader() {
   return nullptr;
 }
 
+ukm::UkmService* TestPaymentRequestDelegate::GetUkmService() {
+  return nullptr;
+}
+
 TestAddressNormalizer* TestPaymentRequestDelegate::test_address_normalizer() {
   return &address_normalizer_;
 }

components/payments/core/test_payment_request_delegate.h

@@ -31,6 +31,7 @@ class TestPaymentRequestDelegate : public PaymentRequestDelegate {
           result_delegate) override;
   AddressNormalizer* GetAddressNormalizer() override;
   autofill::RegionDataLoader* GetRegionDataLoader() override;
+  ukm::UkmService* GetUkmService() override;
 
   TestAddressNormalizer* test_address_normalizer();
   void DelayFullCardRequestCompletion();
@@ -42,11 +43,10 @@ class TestPaymentRequestDelegate : public PaymentRequestDelegate {
   const GURL last_committed_url_;
   TestAddressNormalizer address_normalizer_;
 
+  bool instantaneous_full_card_request_result_ = true;
   autofill::CreditCard full_card_request_card_;
   base::WeakPtr<autofill::payments::FullCardRequest::ResultDelegate>
       full_card_result_delegate_;
-
-  bool instantaneous_full_card_request_result_ = true;
   DISALLOW_COPY_AND_ASSIGN(TestPaymentRequestDelegate);
 };