requestAutocomplete: Fill |form_structure_| from Online Wallet data (including

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 88 matching lines @@
 }
 
 // Returns true if |input| should be used for a site-requested |field|.
 bool DetailInputMatchesField(const DetailInput& input,
                              const AutofillField& field) {
   bool right_section = !input.section_suffix ||
       EndsWith(field.section(), input.section_suffix, false);
   return InputTypeMatchesFieldType(input, field) && right_section;
 }
 
+bool CreditCardType(AutofillFieldType type) {

[Ilya Sherman, 2013/03/14 21:59:08]

nit: IsCreditCardType

[Dan Beam, 2013/03/14 23:49:35]

Done.

+  return AutofillType(type).group() == AutofillType::CREDIT_CARD;
+}
+
+bool DetailInputMatchesCreditCard(const DetailInput& input,
+                                  const AutofillField& field) {
+  return DetailInputMatchesField(input, field) && CreditCardType(input.type);
+}
+
+bool DetailInputMatchesBilling(const DetailInput& input,
+                               const AutofillField& field) {
+  return DetailInputMatchesField(input, field) && !CreditCardType(input.type);
+}
+
 // Returns true if |input| should be used to fill a site-requested |field| which
 // is notated with a "shipping" tag, for use when the user has decided to use
 // the billing address as the shipping address.
 bool DetailInputMatchesShippingField(const DetailInput& input,
                                      const AutofillField& field) {
   if (input.section_suffix &&
       std::string(input.section_suffix) == "billing") {
     return InputTypeMatchesFieldType(input, field);
   }
 
@@ skipping 33 matching lines @@
   }
 }
 
 // Get billing info from |output| and put it into |card|, |cvc|, and |profile|.
 // These outparams are required because |card|/|profile| accept different types
 // of raw info, and CreditCard doesn't save CVCs.
 void GetBillingInfoFromOutputs(const DetailOutputMap& output,
                                CreditCard* card,
                                string16* cvc,
                                AutofillProfile* profile) {
+  DCHECK(card);
+  DCHECK(cvc);
+  DCHECK(profile);

[Ilya Sherman, 2013/03/14 21:59:08]

These don't seem very helpful, since you dereference the pointers within this
method.

[Dan Beam, 2013/03/14 23:49:35]

Done.

+
   for (DetailOutputMap::const_iterator it = output.begin();
        it != output.end(); ++it) {
     string16 trimmed;
     TrimWhitespace(it->second, TRIM_ALL, &trimmed);
 
     // Special case CVC as CreditCard just swallows it.
     if (it->first->type == CREDIT_CARD_VERIFICATION_CODE) {
-      *cvc = trimmed;
+      cvc->assign(trimmed);

[Ilya Sherman, 2013/03/14 21:59:08]

Just curious, why did you change this?

[Dan Beam, 2013/03/14 23:49:35]

seemed like this is the intended API, akin to saying .size() == 0 vs. .empty()

     } else {
       // Copy the credit card name to |profile| in addition to |card| as
       // wallet::Instrument requires a recipient name for its billing address.
       if (it->first->type == CREDIT_CARD_NAME)
         profile->SetRawInfo(NAME_FULL, trimmed);
 
-      if (AutofillType(it->first->type).group() == AutofillType::CREDIT_CARD)
+      if (CreditCardType(it->first->type))
         card->SetRawInfo(it->first->type, trimmed);
       else
         profile->SetRawInfo(it->first->type, trimmed);
     }
   }
 }
 
 // Returns the containing window for the given |web_contents|.  The containing
 // window might be a browser window for a Chrome tab, or it might be a shell
 // window for a platform app.
@@ skipping 401 matching lines @@
 
 scoped_ptr<DataModelWrapper> AutofillDialogControllerImpl::CreateWrapper(
     DialogSection section) {
   SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);
   std::string item_key = model->GetItemKeyForCheckedItem();
   scoped_ptr<DataModelWrapper> wrapper;
   if (item_key.empty())
     return wrapper.Pass();
 
   if (IsPayingWithWallet()) {
-    int index;
-    bool success = base::StringToInt(item_key, &index);
-    DCHECK(success);
+    if (did_submit_ && full_wallet_.get()) {
+      if (section == SECTION_CC_BILLING)
+        wrapper.reset(new FullWalletBillingWrapper(full_wallet_.get()));
+      else
+        wrapper.reset(new FullWalletShippingWrapper(full_wallet_.get()));
+    } else {
+      int index;
+      bool success = base::StringToInt(item_key, &index);
+      DCHECK(success);
 
-    if (section == SECTION_CC_BILLING) {
-      wrapper.reset(
-          new WalletInstrumentWrapper(wallet_items_->instruments()[index]));
-    } else {
-      wrapper.reset(
-          new WalletAddressWrapper(wallet_items_->addresses()[index]));
+      if (section == SECTION_CC_BILLING) {
+        wrapper.reset(
+            new WalletInstrumentWrapper(wallet_items_->instruments()[index]));
+      } else {
+        wrapper.reset(
+            new WalletAddressWrapper(wallet_items_->addresses()[index]));
+      }
     }

[Ilya Sherman, 2013/03/14 21:59:08]

nit: Some docs for the code flow logic here would be helpful.  It's not
completely obvious why "!did_submit_ || !full_wallet_.get()" is an interesting
case.

[Dan Beam, 2013/03/14 23:49:35]

Done.

   } else if (section == SECTION_CC) {
     CreditCard* card = GetManager()->GetCreditCardByGUID(item_key);
     DCHECK(card);
     wrapper.reset(new AutofillCreditCardWrapper(card));
   } else {
     // Calculate the variant by looking at how many items come from the same
     // FormGroup.
     size_t variant = 0;
     for (int i = model->checked_item() - 1; i >= 0; --i) {
       if (model->GetItemKeyAt(i) == item_key)
@@ skipping 557 matching lines @@
   if (popup_controller_)
     return popup_controller_->HandleKeyPressEvent(event);
 
   return false;
 }
 
 bool AutofillDialogControllerImpl::RequestingCreditCardInfo() const {
   DCHECK_GT(form_structure_.field_count(), 0U);
 
   for (size_t i = 0; i < form_structure_.field_count(); ++i) {
-    if (AutofillType(form_structure_.field(i)->type()).group() ==
-        AutofillType::CREDIT_CARD) {
+    if (CreditCardType(form_structure_.field(i)->type()))
       return true;
-    }
   }
 
   return false;
 }
 
 bool AutofillDialogControllerImpl::TransmissionWillBeSecure() const {
   return source_url_.SchemeIs(chrome::kHttpsScheme) &&
          !net::IsCertStatusError(ssl_status_.cert_status) &&
          !net::IsCertStatusMinorError(ssl_status_.cert_status);
 }
@@ skipping 123 matching lines @@
   return true;
 }
 
 void AutofillDialogControllerImpl::FillOutputForSectionWithComparator(
     DialogSection section,
     const InputFieldComparator& compare) {
   if (!SectionIsActive(section))
     return;
 
   if (!IsManuallyEditingSection(section)) {
-    if (section == SECTION_CC_BILLING) {
-      // TODO(dbeam): implement.
-    } else {
-      scoped_ptr<DataModelWrapper> model = CreateWrapper(section);
-      // Only fill in data that is associated with this section.
-      const DetailInputs& inputs = RequestedFieldsForSection(section);
-      model->FillFormStructure(inputs, compare, &form_structure_);
+    scoped_ptr<DataModelWrapper> model = CreateWrapper(section);
+    // Only fill in data that is associated with this section.
+    const DetailInputs& inputs = RequestedFieldsForSection(section);
+    model->FillFormStructure(inputs, compare, &form_structure_);
 
-      // CVC needs special-casing because the CreditCard class doesn't store
-      // or handle them.
-      if (section == SECTION_CC)
-        SetCvcResult(view_->GetCvc());
-    }
+    // CVC needs special-casing because the CreditCard class doesn't store
+    // or handle them.
+    if (section == SECTION_CC)

[Ilya Sherman, 2013/03/14 21:59:08]

Should this apply to SECTION_CC_BILLING as well?

[Dan Beam, 2013/03/14 23:49:35]

No, we need to get the CVC from |full_wallet_|; added comment.

+      SetCvcResult(view_->GetCvc());
   } else {
-    // The user manually input data.
+    // The user manually input data. If using Autofill, save the info as new or
+    // edited data. Always fill local data into |form_structure_|.
     DetailOutputMap output;
     view_->GetUserInput(section, &output);
 
-    if (IsPayingWithWallet()) {
-      // TODO(dbeam): implement.
+    if (section == SECTION_CC_BILLING) {
+      CreditCard card;
+      string16 cvc;
+      AutofillProfile profile;
+      GetBillingInfoFromOutputs(output, &card, &cvc, &profile);
+
+      // |card| and |profile| can't be asked for the wrong types without
+      // DCHECK()'ing, so we split the filling into CC and non-CC.
+      FillFormStructureForSection(card, 0, section,
+                                  base::Bind(DetailInputMatchesCreditCard));
+      FillFormStructureForSection(profile, 0, section,
+                                  base::Bind(DetailInputMatchesBilling));
+      SetCvcResult(cvc);
+    } else if (section == SECTION_CC) {
+      CreditCard card;
+      FillFormGroupFromOutputs(output, &card);
+
+      if (ShouldSaveDetailsLocally())
+        GetManager()->SaveImportedCreditCard(card);
+
+      FillFormStructureForSection(card, 0, section, compare);
+
+      // Again, CVC needs special-casing. Fill it in directly from |output|.
+      SetCvcResult(GetValueForType(output, CREDIT_CARD_VERIFICATION_CODE));
     } else {

[Ilya Sherman, 2013/03/14 21:59:08]

Can you DCHECK that the section is the one you expect here?

[Dan Beam, 2013/03/14 23:49:35]

This hunk of the diff was reverted and now it'd be SECTION_EMAIL,
SECTION_BILLING, or SECTION_SHIPPING. Is DCHECK()'ing that useful?

-      // Save the info as new or edited data and fill it into |form_structure_|.
-      if (section == SECTION_CC) {
-        CreditCard card;
-        FillFormGroupFromOutputs(output, &card);
+      AutofillProfile profile;
+      FillFormGroupFromOutputs(output, &profile);
 
-        if (view_->SaveDetailsLocally())
-          GetManager()->SaveImportedCreditCard(card);
+      // TODO(estade): we should probably edit the existing profile in the
+      // cases where the input data is based on an existing profile (user
+      // clicked "Edit" or autofill popup filled in the form).
+      if (ShouldSaveDetailsLocally())
+        GetManager()->SaveImportedProfile(profile);
 
-        FillFormStructureForSection(card, 0, section, compare);
-
-        // Again, CVC needs special-casing. Fill it in directly from |output|.
-        SetCvcResult(GetValueForType(output, CREDIT_CARD_VERIFICATION_CODE));
-      } else {
-        AutofillProfile profile;
-        FillFormGroupFromOutputs(output, &profile);
-
-        // TODO(estade): we should probably edit the existing profile in the
-        // cases where the input data is based on an existing profile (user
-        // clicked "Edit" or autofill popup filled in the form).
-        if (view_->SaveDetailsLocally())
-          GetManager()->SaveImportedProfile(profile);
-
-        FillFormStructureForSection(profile, 0, section, compare);
-      }
+      FillFormStructureForSection(profile, 0, section, compare);
     }
   }
 }
 
 void AutofillDialogControllerImpl::FillOutputForSection(DialogSection section) {
   FillOutputForSectionWithComparator(section,
                                      base::Bind(DetailInputMatchesField));
 }
 
 void AutofillDialogControllerImpl::FillFormStructureForSection(
@@ skipping 103 matching lines @@
   NOTIMPLEMENTED();
 }
 
 bool AutofillDialogControllerImpl::IsManuallyEditingSection(
     DialogSection section) {
   return section_editing_state_[section] ||
          SuggestionsMenuModelForSection(section)->
              GetItemKeyForCheckedItem().empty();
 }
 
-bool AutofillDialogControllerImpl::UseBillingForShipping() {
+bool AutofillDialogControllerImpl::ShouldUseBillingForShipping() {
   // If the user is editing or inputting data, ask the view.
   if (IsManuallyEditingSection(SECTION_SHIPPING))
     return view_->UseBillingForShipping();
 
   // Otherwise, the checkbox should be hidden so its state is irrelevant.
   // Always use the shipping suggestion model.
   return false;
 }
 
+bool AutofillDialogControllerImpl::ShouldSaveDetailsLocally() {
+  return !IsPayingWithWallet() && view_->SaveDetailsLocally();

[Ilya Sherman, 2013/03/14 21:59:08]

nit: Please include a comment explaining why we need more than just the second
check.  This code looks a little surprising as it is.

[Dan Beam, 2013/03/14 23:49:35]

Done.

+}
+
 void AutofillDialogControllerImpl::SubmitWithWallet() {
   // TODO(dbeam): disallow interacting with the dialog while submitting.
 
   active_instrument_id_.clear();
   active_address_id_.clear();
 
   if (!section_editing_state_[SECTION_CC_BILLING]) {
     SuggestionsMenuModel* billing =
         SuggestionsMenuModelForSection(SECTION_CC_BILLING);
     if (!billing->GetItemKeyForCheckedItem().empty() &&
         billing->checked_item() <
             static_cast<int>(wallet_items_->instruments().size())) {
       const wallet::WalletItems::MaskedInstrument* active_instrument =
           wallet_items_->instruments()[billing->checked_item()];
       active_instrument_id_ = active_instrument->object_id();
 
       // TODO(dbeam): does re-using instrument address IDs work?
-      if (UseBillingForShipping())
+      if (ShouldUseBillingForShipping())
         active_address_id_ = active_instrument->address().object_id();
     }
   }
 
   if (!section_editing_state_[SECTION_SHIPPING] && active_address_id_.empty()) {
     SuggestionsMenuModel* shipping =
         SuggestionsMenuModelForSection(SECTION_SHIPPING);
     if (!shipping->GetItemKeyForCheckedItem().empty() &&
         shipping->checked_item() <
             static_cast<int>(wallet_items_->addresses().size())) {
@@ skipping 20 matching lines @@
     CreditCard card;
     AutofillProfile profile;
     string16 cvc;
     GetBillingInfoFromOutputs(output, &card, &cvc, &profile);
 
     new_instrument.reset(new wallet::Instrument(card, cvc, profile));
   }
 
   scoped_ptr<wallet::Address> new_address;
   if (active_address_id_.empty()) {
-    if (UseBillingForShipping() && new_instrument.get()) {
+    if (ShouldUseBillingForShipping() && new_instrument.get()) {
       new_address.reset(new wallet::Address(new_instrument->address()));
     } else {
       DetailOutputMap output;
       view_->GetUserInput(SECTION_SHIPPING, &output);
 
       AutofillProfile profile;
       FillFormGroupFromOutputs(output, &profile);
 
       new_address.reset(new wallet::Address(profile));
     }
@@ skipping 30 matching lines @@
       wallet_items_->google_transaction_id(),
       dialog_type_,
       std::vector<wallet::WalletClient::RiskCapability>()));
 }
 
 void AutofillDialogControllerImpl::FinishSubmit() {
   FillOutputForSection(SECTION_EMAIL);
   FillOutputForSection(SECTION_CC);
   FillOutputForSection(SECTION_BILLING);
   FillOutputForSection(SECTION_CC_BILLING);
-  if (UseBillingForShipping()) {
+  if (ShouldUseBillingForShipping()) {
     FillOutputForSectionWithComparator(
         SECTION_BILLING,
         base::Bind(DetailInputMatchesShippingField));
     FillOutputForSectionWithComparator(
         SECTION_CC,
         base::Bind(DetailInputMatchesShippingField));
   } else {
     FillOutputForSection(SECTION_SHIPPING);
   }
   callback_.Run(&form_structure_);
   callback_ = base::Callback<void(const FormStructure*)>();
 
   if (dialog_type_ == DIALOG_TYPE_REQUEST_AUTOCOMPLETE) {
     // This may delete us.
     Hide();
   }
 }
 
 }  // namespace autofill