[Payments] Record CanMakePayment metrics on Desktop.

components/payments/content/payment_request.cc

Index: components/payments/content/payment_request.cc
diff --git a/components/payments/content/payment_request.cc b/components/payments/content/payment_request.cc
index 2cc645f189121f621636186a3c7f0213e6d0f42d..9194bd711178d20e0f446da01c29964ab85a3555 100644
--- a/components/payments/content/payment_request.cc
+++ b/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,7 @@ void PaymentRequest::CanMakePayment() {
       delegate_->IsIncognito() || state()->CanMakePayment()
           ? mojom::CanMakePaymentQueryResult::CAN_MAKE_PAYMENT
           : mojom::CanMakePaymentQueryResult::CANNOT_MAKE_PAYMENT);
+  journey_logger_.SetCanMakePaymentValue(state()->CanMakePayment());

[Mathieu, 2017/05/02 20:56:47]

I think we should log the value that is sent back to the merchant, which takes
into account the incognito state. What do you think?

[sebsg, 2017/05/02 22:15:08]

Right, it's more safe if the code changes, but we currently don't log
CanMakePayment metrics if the user is in incognito, so the behavior is the same.
Thanks for checking :)

   if (observer_for_testing_)
     observer_for_testing_->OnCanMakePaymentCalled();
 }
@@ -170,6 +180,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);
 
@@ -185,6 +198,8 @@ void PaymentRequest::OnConnectionTerminated() {
   // has decided to close the pipe for various reasons (see all uses of
   // PaymentRequest::clearResolversAndCloseMojoConnection() in Blink). We close
   // the binding and the dialog, and ask to be deleted.
+  journey_logger_.RecordJourneyStatsHistograms(
+      JourneyLogger::COMPLETION_STATUS_OTHER_ABORTED);

[Mathieu, 2017/05/02 20:56:47]

This method can get called because of many reasons, some of them positive (for
example, as a result of line 146). Therefore, I don't think this call is right.
Let me know if you need more details about when this might be called. 

[sebsg, 2017/05/02 22:15:08]

Yeah I would like if you could give me more info. I'm not sure I understand what
can be a positive reason that is not a payment completion. Thanks!

[Mathieu, 2017/05/03 02:22:49]

Line 33-37 gives a possible way we could know of the reason. But we can't always
assume it's an abort. I would remove this logging statement. It feels wrong
especially since on line 142 we log a different completion status, and we will
still end up on this line. 

[sebsg, 2017/05/03 17:10:11]

Thanks, will look at this right after this CL

   client_.reset();
   binding_.Close();
   delegate_->CloseDialog();