[Payments] Selecting incomplete items will open editors

chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc

Index: chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
diff --git a/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc b/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
index 4ce0cbd8de428943195ddc351bc67287e2db45a0..e36e8fa8d3906e6b26584cf99b6e35e56eedf79e 100644
--- a/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
+++ b/chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
@@ -76,8 +76,9 @@ autofill::ServerFieldType GetFieldTypeFromString(const std::string& type) {
 ShippingAddressEditorViewController::ShippingAddressEditorViewController(
     PaymentRequestSpec* spec,
     PaymentRequestState* state,
-    PaymentRequestDialogView* dialog)
-    : EditorViewController(spec, state, dialog) {
+    PaymentRequestDialogView* dialog,
+    autofill::AutofillProfile* profile)
+    : EditorViewController(spec, state, dialog), profile_to_edit_(profile) {
   UpdateEditorFields();
 }
 
@@ -93,9 +94,18 @@ ShippingAddressEditorViewController::GetFieldDefinitions() {
   return editor_fields_;
 }
 
+base::string16 ShippingAddressEditorViewController::GetInitialValueForType(
+    autofill::ServerFieldType type) {
+  if (!profile_to_edit_)
+    return base::string16();
+
+  // TODO(crbug.com/709451): Use GetInfo() here.

[anthonyvd, 2017/04/07 20:08:09]

Why not use it already? I'm not sure what in that bug is preventing us from
doing it if we know it's the correct thing.

[Mathieu, 2017/04/09 00:35:45]

You're right, I can get the app_locale

+  return profile_to_edit_->GetRawInfo(type);
+}
+
 bool ShippingAddressEditorViewController::ValidateModelAndSave() {
+  // To validate the profile first, we use a temporary object.
   autofill::AutofillProfile profile;
-  profile.set_origin(autofill::kSettingsOrigin);
   for (const auto& field : text_fields()) {
     // Force a blur in case the value was left untouched.
     field.first->OnBlur();
@@ -121,8 +131,26 @@ bool ShippingAddressEditorViewController::ValidateModelAndSave() {
     }
   }
 
-  // Add the profile (will not add a duplicate).
-  state()->GetPersonalDataManager()->AddProfile(profile);
+  if (!profile_to_edit_) {
+    // Add the profile (will not add a duplicate).
+    profile.set_origin(autofill::kSettingsOrigin);

[anthonyvd, 2017/04/07 20:08:09]

Do we want a different origin for PaymentRequest?

[Mathieu, 2017/04/09 00:35:45]

I don't think so. "settings" means the profile has been verified by the user in
some way (it then becomes stronger than an inferred profile when merging with
others). Since this is an editor and the person has clicked Save, I think
keeping this origin is correct and equivalent to editing in chrome://settings.

+    state()->GetPersonalDataManager()->AddProfile(profile);
+  } else {
+    // Copy the temporary object's data to the object to be edited. Prefer this
+    // method to copying |profile| into |profile_to_edit_|, because the latter
+    // object needs to retain other properties (use count, use date, guid,
+    // etc.).
+    for (const auto& field : text_fields()) {
+      profile_to_edit_->SetRawInfo(field.second.type,

[anthonyvd, 2017/04/07 20:08:09]

If GetInfo is preferred over GetRawInfo above, shouldn't this be SetInfo in some
cases? 

[Mathieu, 2017/04/09 00:35:45]

Yeah I think we should play it safe and use GetInfo/SetInfo always.

+                                   profile.GetRawInfo(field.second.type));
+    }
+    for (const auto& field : comboboxes()) {
+      profile_to_edit_->SetRawInfo(field.second.type,
+                                   profile.GetRawInfo(field.second.type));
+    }
+    profile_to_edit_->set_origin(autofill::kSettingsOrigin);

[anthonyvd, 2017/04/07 20:08:09]

Same question re: origin.

[Mathieu, 2017/04/09 00:35:45]

Acknowledged.

+    state()->GetPersonalDataManager()->UpdateProfile(*profile_to_edit_);
+  }
 
   return true;
 }