[Autofill] Add user type metrics.

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/autofill/autofill_dialog_controller_impl.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 1012 matching lines @@
 }
 
 void AutofillDialogControllerImpl::OnDidGetFullWallet(
     scoped_ptr<wallet::FullWallet> full_wallet) {
   NOTIMPLEMENTED();
   WalletRequestCompleted(true);
 }
 
 void AutofillDialogControllerImpl::OnDidGetWalletItems(
     scoped_ptr<wallet::WalletItems> wallet_items) {
+  if (!wallet_items_ && wallet_items) {

[Dan Beam, 2013/03/12 02:06:01]

wallet_items will always be non-NULL here

+    // On the first successful response, log any relevant user type metrics.
+    // Consider a user to be an Autofill user if the user has any credit cards
+    // or addresses saved.  Check that the item count is greater than 1 because
+    // an "empty" menu still has the "add new" menu item.
+    bool has_autofill_profiles =
+        suggested_cc_.GetItemCount() > 1 ||
+        suggested_billing_.GetItemCount() > 1;
+    AutofillMetrics::DialogUserTypeMetric user_type;
+    if (SignedInState() == SIGNED_IN) {
+      if (!wallet_items->instruments().empty())
+        user_type = AutofillMetrics::DIALOG_WALLET_USER_SIGNED_IN;

[Dan Beam, 2013/03/12 02:06:01]

why are we not logging whether a user has both autofill and wallet?

+      else if (has_autofill_profiles)
+        user_type = AutofillMetrics::DIALOG_AUTOFILL_USER_SIGNED_IN;
+      else
+        user_type = AutofillMetrics::DIALOG_NEW_USER_SIGNED_IN;

[Dan Beam, 2013/03/12 02:06:01]

I think this is a little strange to assume a user is new just because they have
no autofill profiles...

+    } else {
+      if (has_autofill_profiles)
+        user_type = AutofillMetrics::DIALOG_AUTOFILL_USER_NOT_SIGNED_IN;
+      else
+        user_type = AutofillMetrics::DIALOG_NEW_USER_NOT_SIGNED_IN;
+    }
+    metric_logger_.LogDialogUserType(dialog_type_, user_type);

[Dan Beam, 2013/03/12 02:06:01]

if I were writing this it'd go something like:

  class AutofillDialogControllerImpl {
   private:
    enum DialogUserType {
      NOT_SIGNED_IN = 0,
      HAS_AUTOFILL = 1,
      SIGNED_IN = 1 << 2,
      HAS_WALLET = 1 << 3,
    };
    void CheckForAutofillProfiles();
    int user_input_;
  };

  AutofillDialogControllerImpl::AutofillDialogControllerImpl()
      : user_input_(NOT_SIGNED_IN) {
    CheckForAutofillProfiles();
  }

  AutofillDialogControllerImpl::~AutofillDialogControllerImpl() {
    metric_logger_.LogDialogUserType(dialog_type_, user_type_);
  }

  AutofillDialogControllerImpl::OnDidGetWalletItems() {
    if (SignedInState() == SIGNED_IN)
      user_type_ |= SIGNED_IN;
    if (HasCompleteWallet())
      user_type_ |= HAS_WALLET;
  }

  AutofillDialogControllerImpl::OnPersonalDataChanged() {
    CheckForAutofillProfiles();
  }

  AutofillDialogControllerImpl::CheckForAutofillProfiles() {
    if (GetManager()->GetProfiles().size() > 0)
      user_type_ |= HAS_AUTOFILL;
  }

and align the UMA buckets accordingly (i.e. 3 == signed in and autofill, 13 ==
signed in, has wallet, has autofill) -- does this seem reasonable?

it certainly seems simpler and less error prone (except the bitwise math) but
it's my code so I'm obviously biased :P.

+  }
+
   bool items_changed = !wallet_items_ || *wallet_items != *wallet_items_;
   wallet_items_ = wallet_items.Pass();
   WalletRequestCompleted(true);
 
   if (items_changed) {
     GenerateSuggestionsModels();
     view_->ModelChanged();
     view_->UpdateAccountChooser();
     view_->UpdateNotificationArea();
   }
@@ skipping 393 matching lines @@
   // If the user is editing or inputting data, ask the view.
   if (item_key.empty() || section_editing_state_[SECTION_SHIPPING])
     return view_->UseBillingForShipping();
 
   // Otherwise, the checkbox should be hidden so its state is irrelevant.
   // Always use the shipping suggestion model.
   return false;
 }
 
 }  // namespace autofill