Making credit card number un-editable in Wallet mode.

Making credit card number un-editable in Wallet mode.

Also adds a way of generically making any textfield or combobox disabled if necessary.

R=estade@chromium.org
BUG=180147
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202847

[Dan Beam, 2013-05-24 05:45:29]

estade@: before I write tests, do you like this design?

[Evan Stade, 2013-05-24 20:13:58]

On 2013/05/24 05:45:29, Dan Beam wrote:
> estade@: before I write tests, do you like this design?

why not make it (enabled/disabled state) part of the DetailInput?

[Dan Beam, 2013-05-25 00:28:11]

estade@: ptal now

[Evan Stade, 2013-05-25 00:41:19]

just nits

https://codereview.chromium.org/15979002/diff/11010/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/autofill_dialog_types.h (right):

https://codereview.chromium.org/15979002/diff/11010/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/autofill_dialog_types.h:39: // Whether the input
should be enabled (e.g. editable for textfields).
I would just call the field editable.

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
(right):

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:91:
message_loop_runner_(runner) {
{} on same line

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:447: ++it)
{
one more space of indent

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/views/a...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/views/a...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:958: if
(!it->second->textfield()->enabled())
This isn't necessary, and I'd just as soon leave it out for now.

https://codereview.chromium.org/15979002/diff/14001/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/15979002/diff/14001/components/autofill.gypi#...
components/autofill.gypi:39: 'target_name': 'autofill_test',
autofill_test_util?

[Dan Beam, 2013-05-25 02:36:57]

had to massage some stuff

https://codereview.chromium.org/15979002/diff/11010/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/autofill_dialog_types.h (right):

https://codereview.chromium.org/15979002/diff/11010/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/autofill_dialog_types.h:39: // Whether the input
should be enabled (e.g. editable for textfields).
On 2013/05/25 00:41:19, Evan Stade wrote:
> I would just call the field editable.

Done.

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
(right):

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:91:
message_loop_runner_(runner) {
On 2013/05/25 00:41:19, Evan Stade wrote:
> {} on same line

Done.

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc:447: ++it)
{
On 2013/05/25 00:41:19, Evan Stade wrote:
> one more space of indent

removed

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/views/a...
File chrome/browser/ui/views/autofill/autofill_dialog_views.cc (right):

https://codereview.chromium.org/15979002/diff/14001/chrome/browser/ui/views/a...
chrome/browser/ui/views/autofill/autofill_dialog_views.cc:958: if
(!it->second->textfield()->enabled())
On 2013/05/25 00:41:19, Evan Stade wrote:
> This isn't necessary, and I'd just as soon leave it out for now.

Done.

https://codereview.chromium.org/15979002/diff/14001/components/autofill.gypi
File components/autofill.gypi (right):

https://codereview.chromium.org/15979002/diff/14001/components/autofill.gypi#...
components/autofill.gypi:39: 'target_name': 'autofill_test',
On 2013/05/25 00:41:19, Evan Stade wrote:
> autofill_test_util?

Done.

[Evan Stade, 2013-05-28 16:11:45]

lgtm. +joi for gyp changes

[Jói, 2013-05-28 18:56:50]

gyp LGTM

[commit-bot: I haz the power, 2013-05-28 19:45:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/15979002/22001

[commit-bot: I haz the power, 2013-05-28 19:45:45]

Failed to apply patch for
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
  Hunk #1 succeeded at 778 (offset 19 lines).
  Hunk #2 succeeded at 1084 (offset 19 lines).
  Hunk #3 FAILED at 2271.
  1 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc.rej

Patch:       chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
Index: chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
index
f4679ab0fcc76c4fd0f6734d275125ccae5baa3d..ab499adaa55b63523ce35f444c9e9b3f9f653226
100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -759,23 +759,27 @@ void
AutofillDialogControllerImpl::EnsureLegalDocumentsText() {
 
 void AutofillDialogControllerImpl::PrepareDetailInputsForSection(
     DialogSection section) {
-  // Reset all previously entered data and stop editing |section|.
-  DetailInputs* inputs = MutableRequestedFieldsForSection(section);
-  for (size_t i = 0; i < inputs->size(); ++i) {
-    (*inputs)[i].initial_value.clear();
-  }
   section_editing_state_[section] = false;
+  scoped_ptr<DataModelWrapper> wrapper = CreateWrapper(section);
 
   // If the chosen item in |model| yields an empty suggestion text, it is
   // invalid. In this case, show the editing UI with invalid fields
highlighted.
   SuggestionsMenuModel* model = SuggestionsMenuModelForSection(section);
   if (IsASuggestionItemKey(model->GetItemKeyForCheckedItem()) &&
       SuggestionTextForSection(section).empty()) {
-    scoped_ptr<DataModelWrapper> wrapper = CreateWrapper(section);
-    wrapper->FillInputs(MutableRequestedFieldsForSection(section));
     section_editing_state_[section] = true;
   }
 
+  // Reset all previously entered data and stop editing |section|.
+  DetailInputs* inputs = MutableRequestedFieldsForSection(section);
+  for (DetailInputs::iterator it = inputs->begin(); it != inputs->end(); ++it)
{
+    it->initial_value.clear();
+    it->editable = InputIsEditable(*it, section);
+  }
+
+  if (wrapper && section_editing_state_[section])
+    wrapper->FillInputs(inputs);
+
   if (view_)
     view_->UpdateSection(section);
 }
@@ -1061,8 +1065,14 @@ bool AutofillDialogControllerImpl::EditEnabledForSection(
 void AutofillDialogControllerImpl::EditClickedForSection(
     DialogSection section) {
   scoped_ptr<DataModelWrapper> model = CreateWrapper(section);
-  model->FillInputs(MutableRequestedFieldsForSection(section));
   section_editing_state_[section] = true;
+
+  DetailInputs* inputs = MutableRequestedFieldsForSection(section);
+  for (DetailInputs::iterator it = inputs->begin(); it != inputs->end(); ++it)
{
+    it->editable = InputIsEditable(*it, section);
+  }
+  model->FillInputs(inputs);
+
   view_->UpdateSection(section);
 
   GetMetricLogger().LogDialogUiEvent(
@@ -2261,6 +2271,20 @@ bool
AutofillDialogControllerImpl::IsManuallyEditingAnySection() const {
   return false;
 }
 
+bool AutofillDialogControllerImpl::InputIsEditable(
+    const DetailInput& input,
+    DialogSection section) const {
+  if (input.type != CREDIT_CARD_NUMBER || !IsPayingWithWallet())
+    return true;
+
+  std::map<DialogSection, bool>::const_iterator it =
+      section_editing_state_.find(section);
+  if (it != section_editing_state_.end() && it->second)
+    return false;
+
+  return true;
+}
+
 bool AutofillDialogControllerImpl::InputIsValid(AutofillFieldType type,
                                                 const string16& value) const {
   switch (AutofillType::GetEquivalentFieldType(type)) {

[commit-bot: I haz the power, 2013-05-28 21:13:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/15979002/30002

[commit-bot: I haz the power, 2013-05-28 21:37:17]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-05-28 22:30:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/15979002/30002

[commit-bot: I haz the power, 2013-05-29 09:00:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/15979002/60001

[commit-bot: I haz the power, 2013-05-29 11:48:31]

Change committed as 202847