Collecting UMA metrics for the UnmaskPrompt.

chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.cc

Index: chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.cc
diff --git a/chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.cc b/chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.cc
index abd616f0cb3be6b12478acfb72beb9bfae2360c8..4918d90bcd5ed32241b65f0efbe235fd83f0e2ac 100644
--- a/chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.cc
+++ b/chrome/browser/ui/autofill/card_unmask_prompt_controller_impl.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/autofill/risk_util.h"
 #include "chrome/browser/ui/autofill/card_unmask_prompt_view.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/web_contents.h"
@@ -24,6 +25,9 @@ CardUnmaskPromptControllerImpl::CardUnmaskPromptControllerImpl(
     content::WebContents* web_contents)
     : web_contents_(web_contents),
       card_unmask_view_(nullptr),
+      unmasking_success_(false),
+      unmasking_initial_should_store_pan_(false),
+      unmasking_number_of_attempts_(0),
       weak_pointer_factory_(this) {
 }
 
@@ -32,6 +36,10 @@ CardUnmaskPromptControllerImpl::~CardUnmaskPromptControllerImpl() {
     card_unmask_view_->ControllerGone();
 }
 
+CardUnmaskPromptView* CardUnmaskPromptControllerImpl::CreateAndShowView() {
+  return CardUnmaskPromptView::CreateAndShow(this);
+}
+
 void CardUnmaskPromptControllerImpl::ShowPrompt(
     const CreditCard& card,
     base::WeakPtr<CardUnmaskDelegate> delegate) {
@@ -42,7 +50,11 @@ void CardUnmaskPromptControllerImpl::ShowPrompt(
   LoadRiskFingerprint();
   card_ = card;
   delegate_ = delegate;
-  card_unmask_view_ = CardUnmaskPromptView::CreateAndShow(this);
+  card_unmask_view_ = CreateAndShowView();
+  unmasking_success_ = false;
+  unmasking_number_of_attempts_ = 0;
+  unmasking_initial_should_store_pan_ = GetStoreLocallyStartState();
+  AutofillMetrics::LogUnmaskPromptEvent(AutofillMetrics::UNMASK_PROMPT_SHOWN);
 }
 
 void CardUnmaskPromptControllerImpl::OnVerificationResult(
@@ -53,8 +65,19 @@ void CardUnmaskPromptControllerImpl::OnVerificationResult(
   base::string16 error_message;
   bool allow_retry = true;
   switch (result) {
-    case AutofillClient::SUCCESS:
+    case AutofillClient::SUCCESS: {
+      unmasking_success_ = true;
+      AutofillMetrics::LogUnmaskPromptEvent(

[Evan Stade, 2015/03/18 21:29:07]

you can move this logging down to OnUnmaskDialogClosed so that (almost) all
logging is concentrated in one location

[Walter Cacau, 2015/03/18 23:50:05]

Done.

Also extracted that code to a method to make it clearer that it is only logging
code.

+          AutofillMetrics::UNMASK_PROMPT_UNMASKED_CARD);
+      bool final_should_store_pan =
+          user_prefs::UserPrefs::Get(web_contents_->GetBrowserContext())
+              ->GetBoolean(prefs::kAutofillWalletImportStorageCheckboxState);
+      if (final_should_store_pan) {
+        AutofillMetrics::LogUnmaskPromptEvent(
+            AutofillMetrics::UNMASK_PROMPT_SAVED_CARD_LOCALLY);
+      }
       break;
+    }
 
     case AutofillClient::TRY_AGAIN_FAILURE: {
       error_message = l10n_util::GetStringUTF16(
@@ -82,6 +105,34 @@ void CardUnmaskPromptControllerImpl::OnVerificationResult(
 
 void CardUnmaskPromptControllerImpl::OnUnmaskDialogClosed() {
   card_unmask_view_ = nullptr;
+  if (!unmasking_success_) {
+    if (unmasking_number_of_attempts_ > 0) {

[Evan Stade, 2015/03/18 01:47:59]

nit: use ternary operator

[Walter Cacau, 2015/03/18 17:04:54]

Done.

+      AutofillMetrics::LogUnmaskPromptEvent(
+          AutofillMetrics::UNMASK_PROMPT_CLOSED_FAILED_TO_UNMASK);
+    } else {
+      AutofillMetrics::LogUnmaskPromptEvent(
+          AutofillMetrics::UNMASK_PROMPT_CLOSED_NO_ATTEMPTS);
+    }
+  }
+  if (unmasking_number_of_attempts_ > 0) {
+    bool final_should_store_pan =
+        user_prefs::UserPrefs::Get(web_contents_->GetBrowserContext())
+            ->GetBoolean(prefs::kAutofillWalletImportStorageCheckboxState);

[Evan Stade, 2015/03/18 01:47:59]

final_should_store_pan needs to come from the param in OnUnmaskResponse. (We
don't really care if they uncheck the box and press cancel.)

[Walter Cacau, 2015/03/18 17:04:54]

Shouldn't we care? IMHO, it is changing chrome state and it would be nice to
track.

note: just opening the popup and closing will not trigger these events as there
was no attempt, but after the first attempt we will start recording because we
had a chance to save the pref.

[Evan Stade, 2015/03/18 21:29:07]

ok

+    if (unmasking_initial_should_store_pan_ && final_should_store_pan) {
+      AutofillMetrics::LogUnmaskPromptEvent(

[Evan Stade, 2015/03/18 01:47:59]

nit: pull out LogUnmaskPromptEvent so it looks like

if (foo && bar)
  event = AutofillMetrics::UNMASK_PROMPT_LOCAL_SAVE_DID_NOT_OPT_OUT;
else if...

[Walter Cacau, 2015/03/18 17:04:54]

Done.

+          AutofillMetrics::UNMASK_PROMPT_LOCAL_SAVE_DID_NOT_OPT_OUT);
+    } else if (!unmasking_initial_should_store_pan_
+               && !final_should_store_pan) {
+      AutofillMetrics::LogUnmaskPromptEvent(
+          AutofillMetrics::UNMASK_PROMPT_LOCAL_SAVE_DID_NOT_OPT_IN);
+    } else if (unmasking_initial_should_store_pan_ && !final_should_store_pan) {
+      AutofillMetrics::LogUnmaskPromptEvent(
+          AutofillMetrics::UNMASK_PROMPT_LOCAL_SAVE_DID_OPT_OUT);
+    } else {
+      AutofillMetrics::LogUnmaskPromptEvent(
+          AutofillMetrics::UNMASK_PROMPT_LOCAL_SAVE_DID_OPT_IN);
+    }
+  }
   delegate_->OnUnmaskPromptClosed();
 }
 
@@ -90,6 +141,7 @@ void CardUnmaskPromptControllerImpl::OnUnmaskResponse(
     const base::string16& exp_month,
     const base::string16& exp_year,
     bool should_store_pan) {
+  unmasking_number_of_attempts_++;
   card_unmask_view_->DisableAndWaitForVerification();
 
   DCHECK(!cvc.empty());