Change the behavior of the [X] Save details to Wallet checkbox notification to:

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

 // Copyright 2013 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 <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 235 matching lines @@
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_email_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_cc_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_billing_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_cc_billing_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(suggested_shipping_(this)),
       section_showing_popup_(SECTION_BILLING),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
       metric_logger_(metric_logger),
       initial_user_state_(AutofillMetrics::DIALOG_USER_STATE_UNKNOWN),
       dialog_type_(dialog_type),
+      is_first_run_(!profile_->GetPrefs()->HasPrefPath(
+          prefs::kAutofillDialogPayWithoutWallet)),
       is_submitting_(false),
       autocheckout_is_running_(false),
       had_autocheckout_error_(false) {
   // TODO(estade): remove duplicates from |form|?
   DCHECK(!callback_.is_null());
+  if (is_first_run_) {
+    profile_->GetPrefs()->SetBoolean(prefs::kAutofillDialogPayWithoutWallet,
+                                     kPayWithoutWalletDefault);
+  }
 }
 
 AutofillDialogControllerImpl::~AutofillDialogControllerImpl() {
   if (popup_controller_)
     popup_controller_->Hide();
 
   metric_logger_.LogDialogInitialUserState(dialog_type_, initial_user_state_);
 }
 
 // static
@@ skipping 115 matching lines @@
   // fields. First we must figure out what the "right" fields are.
   view_.reset(CreateView());
   view_->Show();
   GetManager()->AddObserver(this);
 
   // Try to see if the user is already signed-in.
   // If signed-in, fetch the user's Wallet data.
   // Otherwise, see if the user could be signed in passively.
   // TODO(aruslan): UMA metrics for sign-in.
   if (account_chooser_model_.WalletIsSelected())
-    StartFetchingWalletItems();
+    GetWalletItems();

[Dan Beam, 2013/04/05 02:50:08]

this matches the other calls we have (e.g. GetFullWallet())

[aruslan, 2013/04/05 03:45:35]

Ack. Here and in GetWalletItems below the codereview shows weird indentation, I
guess it's just a glitch.

 }
 
 void AutofillDialogControllerImpl::Hide() {
   if (view_)
     view_->Hide();
 }
 
 void AutofillDialogControllerImpl::UpdateProgressBar(double value) {
   view_->UpdateProgressBar(value);
 }
@@ skipping 127 matching lines @@
   return wallet_items_.get() != NULL &&
          !wallet_items_->instruments().empty() &&
          !wallet_items_->addresses().empty();
 }
 
 bool AutofillDialogControllerImpl::IsSubmitPausedOn(
     wallet::RequiredAction required_action) const {
   return full_wallet_ && full_wallet_->HasRequiredAction(required_action);
 }
 
-void AutofillDialogControllerImpl::StartFetchingWalletItems() {
+void AutofillDialogControllerImpl::GetWalletItems() {
   // TODO(dbeam): Add Risk capabilites once the UI supports risk challenges.
   GetWalletClient()->GetWalletItems(
-      source_url_,
-      std::vector<wallet::WalletClient::RiskCapability>());
+      source_url_, std::vector<wallet::WalletClient::RiskCapability>());
 }
 
 void AutofillDialogControllerImpl::OnWalletOrSigninUpdate() {
-  if (wallet_items_.get()) {

[Dan Beam, 2013/04/05 02:50:08]

this needed to work without |wallet_items_|

[aruslan, 2013/04/05 03:45:35]

Yup, it's already like this in another CL.
In REQUIRES_RESPONSE case below please break instead of not reached.

-    DCHECK(!signin_helper_.get());
-    switch (SignedInState()) {
-      case SIGNED_IN:
-        // Start fetching the user name if we don't know it yet.
-        if (current_username_.empty()) {
-          signin_helper_.reset(new wallet::WalletSigninHelper(
-              this,
-              profile_->GetRequestContext()));
-          signin_helper_->StartUserNameFetch();
-        }
-        break;
+  switch (SignedInState()) {
+    case SIGNED_IN:
+      // Start fetching the user name if we don't know it yet.
+      if (current_username_.empty()) {
+        signin_helper_.reset(new wallet::WalletSigninHelper(
+            this, profile_->GetRequestContext()));
+        signin_helper_->StartUserNameFetch();
+      }
+      break;
 
-      case REQUIRES_SIGN_IN:
-        // TODO(aruslan): automatic sign-in?
-        break;
+    case REQUIRES_SIGN_IN:
+      // TODO(aruslan): automatic sign-in?
+      break;
 
-      case REQUIRES_PASSIVE_SIGN_IN:
-        // Attempt to passively sign in the user.
-        current_username_.clear();
-        signin_helper_.reset(new wallet::WalletSigninHelper(
-            this,
-            profile_->GetRequestContext()));
-        signin_helper_->StartPassiveSignin();
-        break;
+    case REQUIRES_PASSIVE_SIGN_IN:
+      // Attempt to passively sign in the user.
+      current_username_.clear();
+      signin_helper_.reset(new wallet::WalletSigninHelper(
+          this, profile_->GetRequestContext()));
+      signin_helper_->StartPassiveSignin();
+      break;
 
-      case REQUIRES_RESPONSE:
-        NOTREACHED();
-    }
+    case REQUIRES_RESPONSE:
+      NOTREACHED();
   }
 
   GenerateSuggestionsModels();
   view_->ModelChanged();
   view_->UpdateAccountChooser();
   view_->UpdateNotificationArea();
 
   // On the first successful response, compute the initial user state metric.
   if (initial_user_state_ == AutofillMetrics::DIALOG_USER_STATE_UNKNOWN)
     initial_user_state_ = GetInitialUserState();
@@ skipping 95 matching lines @@
     return ui::ResourceBundle::GetSharedInstance().GetImageNamed(
         IDR_WALLET_ICON);
   }
 
   gfx::Image icon;
   account_chooser_model_.GetIconAt(account_chooser_model_.checked_item(),
                                    &icon);
   return icon;
 }
 
+bool AutofillDialogControllerImpl::GetPayWithoutWallet() const {
+  return profile_->GetPrefs()->GetBoolean(
+      prefs::kAutofillDialogPayWithoutWallet);
+}
+
+void AutofillDialogControllerImpl::SetPayWithoutWallet(
+    bool pay_without_wallet) {
+  profile_->GetPrefs()->SetBoolean(prefs::kAutofillDialogPayWithoutWallet,
+                                   pay_without_wallet);
+}

[Dan Beam, 2013/04/05 02:50:08]

^ this isn't strictly necessary to do in the controller, but I'm worried that
all the views will need to subscribe to pref updates themselves rather than
saying:

  void AutofillDialogControllerImpl::PrefChanged(const std::string& pref) {
    if (pref == prefs::kAutofillDialogPayWithoutWallet && view_) {
      view_->UpdateNotificationArea();
      view_->UpdateAccountChooser();
    }
  }

(which I can wire up / add if we want)

[aruslan, 2013/04/05 03:45:35]

Currently, the account chooser is wired up to trigger AcoountChoiceChanged. I'm
happy to see this gone.

+
 string16 AutofillDialogControllerImpl::LabelForSection(DialogSection section)
     const {
   switch (section) {
     case SECTION_EMAIL:
       return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_EMAIL);
     case SECTION_CC:
       return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_CC);
     case SECTION_BILLING:
       return l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECTION_BILLING);
     case SECTION_CC_BILLING:
@@ skipping 376 matching lines @@
   if (autocheckout_is_running_ || had_autocheckout_error_) {
     AutofillMetrics::AutocheckoutCompletionStatus metric =
         autocheckout_is_running_ ?
             AutofillMetrics::AUTOCHECKOUT_SUCCEEDED :
             AutofillMetrics::AUTOCHECKOUT_FAILED;
     metric_logger_.LogAutocheckoutDuration(
         base::Time::Now() - autocheckout_started_timestamp_,
         metric);
   }
 
-  // Reset the view so that updates to the pref aren't processed.
-  view_.reset();
-  // On a successful submit, if the user manually selected "pay without wallet",
-  // stop trying to pay with Wallet on future runs of the dialog.
-  bool manually_selected_pay_without_wallet =
-      !account_chooser_model_.WalletIsSelected() &&
-      !account_chooser_model_.had_wallet_error();
-  profile_->GetPrefs()->SetBoolean(prefs::kAutofillDialogPayWithoutWallet,
-                                   manually_selected_pay_without_wallet);
-
   delete this;
 }
 
 std::vector<DialogNotification>
     AutofillDialogControllerImpl::CurrentNotifications() const {
   std::vector<DialogNotification> notifications;
 
-  if (account_chooser_model_.WalletIsSelected()) {
-    if (SignedInState() == SIGNED_IN) {
-      // On first run with a complete wallet profile, show a notification
-      // explaining where this data came from.
-      if (IsFirstRun() && HasCompleteWallet()) {
+  if (account_chooser_model_.had_wallet_error()) {
+    // TODO(dbeam): pass along the Wallet error or remove from the translation.
+    // TODO(dbeam): figure out a way to dismiss this error after a while.
+    notifications.push_back(DialogNotification(
+        DialogNotification::WALLET_ERROR,
+        l10n_util::GetStringFUTF16(IDS_AUTOFILL_DIALOG_COMPLETE_WITHOUT_WALLET,
+                                   ASCIIToUTF16("Oops, [Wallet-Error]."))));
+  } else {
+    if (is_first_run_) {
+      if (SignedInState() == SIGNED_IN) {
+        if (HasCompleteWallet()) {
+          // First run, signed in, has a complete Google Wallet.
+          notifications.push_back(DialogNotification(
+              DialogNotification::EXPLANATORY_MESSAGE,
+              l10n_util::GetStringUTF16(
+                  IDS_AUTOFILL_DIALOG_DETAILS_FROM_WALLET)));
+        } else {
+          // First run, signed in, has an incomplete (or no) Google Wallet.
+          notifications.push_back(DialogNotification(
+              DialogNotification::WALLET_USAGE_CONFIRMATION,
+              l10n_util::GetStringUTF16(
+                  IDS_AUTOFILL_DIALOG_SAVE_DETAILS_IN_WALLET)));
+        }
+      } else {
+        // First run, not signed in, wallet promo.
         notifications.push_back(DialogNotification(
-            DialogNotification::EXPLANATORY_MESSAGE,
+            DialogNotification::WALLET_SIGNIN_PROMO,
             l10n_util::GetStringUTF16(
-                IDS_AUTOFILL_DIALOG_DETAILS_FROM_WALLET)));
-      } else {
-        notifications.push_back(DialogNotification(
-            DialogNotification::WALLET_USAGE_CONFIRMATION,
-            l10n_util::GetStringUTF16(
-                IDS_AUTOFILL_DIALOG_SAVE_DETAILS_IN_WALLET)));
+                IDS_AUTOFILL_DIALOG_SIGN_IN_AND_SAVE_DETAILS)));
       }
-    } else if (IsFirstRun()) {
-      // If the user is not signed in, show an upsell notification on first run.
+    } else if (SignedInState() == SIGNED_IN && !HasCompleteWallet()) {
+      // After first run, signed in.
       notifications.push_back(DialogNotification(
-          DialogNotification::WALLET_SIGNIN_PROMO,
+          DialogNotification::WALLET_USAGE_CONFIRMATION,
           l10n_util::GetStringUTF16(
-              IDS_AUTOFILL_DIALOG_SIGN_IN_AND_SAVE_DETAILS)));
+              IDS_AUTOFILL_DIALOG_SAVE_DETAILS_IN_WALLET)));
+    } else {
+      // If the user isn't signed in and it's after the first run, no promo.
     }
   }
 
   if (RequestingCreditCardInfo() && !TransmissionWillBeSecure()) {
     notifications.push_back(DialogNotification(
         DialogNotification::SECURITY_WARNING,
         l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_SECURITY_WARNING)));
   }
 
   if (!invoked_from_same_origin_) {
     notifications.push_back(DialogNotification(
         DialogNotification::SECURITY_WARNING,
         l10n_util::GetStringFUTF16(IDS_AUTOFILL_DIALOG_SITE_WARNING,
                                    UTF8ToUTF16(source_url_.host()))));
   }
 
   if (IsSubmitPausedOn(wallet::VERIFY_CVV)) {
     notifications.push_back(DialogNotification(
             DialogNotification::REQUIRED_ACTION,
             l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_VERIFY_CVV)));
   }
 
   if (had_autocheckout_error_) {
     notifications.push_back(DialogNotification(
         DialogNotification::AUTOCHECKOUT_ERROR,
         l10n_util::GetStringUTF16(IDS_AUTOFILL_DIALOG_AUTOCHECKOUT_ERROR)));
   }
 
-  if (account_chooser_model_.had_wallet_error()) {
-    // TODO(dbeam): pass along the Wallet error or remove from the translation.
-    // TODO(dbeam): figure out a way to dismiss this error after a while.
-    notifications.push_back(DialogNotification(
-        DialogNotification::WALLET_ERROR,
-        l10n_util::GetStringFUTF16(
-            IDS_AUTOFILL_DIALOG_COMPLETE_WITHOUT_WALLET,
-            ASCIIToUTF16("Oops, [Wallet-Error]."))));
-  }
-
   return notifications;
 }
 
 void AutofillDialogControllerImpl::StartSignInFlow() {
   DCHECK(registrar_.IsEmpty());
 
   content::Source<content::NavigationController> source(view_->ShowSignIn());
   registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED, source);
 }
 
@@ skipping 120 matching lines @@
 void AutofillDialogControllerImpl::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   DCHECK_EQ(type, content::NOTIFICATION_NAV_ENTRY_COMMITTED);
   content::LoadCommittedDetails* load_details =
       content::Details<content::LoadCommittedDetails>(details).ptr();
   if (wallet::IsSignInContinueUrl(load_details->entry->GetVirtualURL())) {
     EndSignInFlow();
     if (account_chooser_model_.WalletIsSelected())
-      StartFetchingWalletItems();
+      GetWalletItems();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // SuggestionsMenuModelDelegate implementation.
 
 void AutofillDialogControllerImpl::SuggestionItemSelected(
     const SuggestionsMenuModel& model) {
   const DialogSection section = SectionForSuggestionsMenuModel(model);
   EditCancelledForSection(section);
@@ skipping 40 matching lines @@
   GenerateSuggestionsModels();
   view_->ModelChanged();
   view_->UpdateNotificationArea();
   view_->UpdateButtonStrip();
 }
 
 void AutofillDialogControllerImpl::OnPassiveSigninSuccess(
     const std::string& username) {
   current_username_ = username;
   signin_helper_.reset();
-  wallet_items_.reset();
-  StartFetchingWalletItems();
+  GetWalletItems();
 }
 
 void AutofillDialogControllerImpl::OnUserNameFetchSuccess(
     const std::string& username) {
   current_username_ = username;
   signin_helper_.reset();
   OnWalletOrSigninUpdate();
 }
 
 void AutofillDialogControllerImpl::OnAutomaticSigninSuccess(
@@ skipping 90 matching lines @@
 void AutofillDialogControllerImpl::OnPersonalDataChanged() {
   HidePopup();
   GenerateSuggestionsModels();
   view_->ModelChanged();
 }
 
 void AutofillDialogControllerImpl::AccountChoiceChanged() {
   if (!view_)
     return;
 
-  // Whenever the user changes the current account, the Wallet data should be
-  // cleared. If the user has chosen a Wallet account, an attempt to fetch
-  // the Wallet data is made to see if the user is still signed in.
-  // This will trigger a passive sign-in if required.
-  // TODO(aruslan): integrate an automatic sign-in.
-  wallet_items_.reset();
-  if (account_chooser_model_.WalletIsSelected())
-    StartFetchingWalletItems();
+  if (account_chooser_model_.WalletIsSelected() && !wallet_items_)

[Dan Beam, 2013/04/05 02:50:08]

^ we could probably re-fetch if we want...

[aruslan, 2013/04/05 03:45:35]

Without the account chooser CL this doesn't matter (but I would refetch anyway).
In the account chooser CL it initiates refetch if we returned to the one that we
were using, otherwise it initiates the sign-in sequence.

+    GetWalletItems();
 
   GenerateSuggestionsModels();
   view_->ModelChanged();
   view_->UpdateAccountChooser();
   view_->UpdateNotificationArea();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
 bool AutofillDialogControllerImpl::HandleKeyPressEventInInput(
@@ skipping 50 matching lines @@
   full_wallet_.reset();
 }
 
 void AutofillDialogControllerImpl::OnWalletSigninError() {
   signin_helper_.reset();
   current_username_.clear();
   account_chooser_model_.SetHadWalletSigninError();
   GetWalletClient()->CancelPendingRequests();
 }
 
-bool AutofillDialogControllerImpl::IsFirstRun() const {
-  PrefService* prefs = profile_->GetPrefs();
-  return !prefs->HasPrefPath(prefs::kAutofillDialogPayWithoutWallet);
-}
-
 void AutofillDialogControllerImpl::GenerateSuggestionsModels() {
   suggested_email_.Reset();
   suggested_cc_.Reset();
   suggested_billing_.Reset();
   suggested_cc_billing_.Reset();
   suggested_shipping_.Reset();
 
   if (IsPayingWithWallet()) {
     // TODO(estade): fill in the email address.
 
@@ skipping 440 matching lines @@
         AutofillMetrics::DIALOG_USER_SIGNED_IN_NO_WALLET_NO_AUTOFILL;
   }
 
   // Has Wallet items.
   return has_autofill_profiles ?
       AutofillMetrics::DIALOG_USER_SIGNED_IN_HAS_WALLET_HAS_AUTOFILL :
       AutofillMetrics::DIALOG_USER_SIGNED_IN_HAS_WALLET_NO_AUTOFILL;
 }
 
 }  // namespace autofill