[Payments] When saving an edited address in editor, clear data first.

[Payments] When saving an edited address in editor, clear data first.

The original bug was that some information was wrongfully being retained
in the edited profile when saved, because some fields were absent in the
newly selected country (say you edited from US to Albania, which has no
states). We fix this by clearing all the address fields from the profile
being edited, before replacing with the values in the editor.

BUG=723610
TEST=browser_tests

Review-Url: https://codereview.chromium.org/2972973002
Cr-Commit-Position: refs/heads/master@{#484756}
Committed: https://chromium.googlesource.com/chromium/src/+/ee7829e4607ec6e4b27efbfdf4212c8e447bde13

[Mathieu, 2017-07-06 02:45:32]

The CQ bit was checked by mathp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 02:45:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 03:42:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 03:42:47]

Dry run: This issue passed the CQ dry run.

[Mathieu, 2017-07-06 12:56:40]

Description was changed from

==========
[Payments] When saving an edited address in editor, clear data first.

The original bug was that some information was wrongfully being retained
in the edited profile when saved, because some fields were absent in the
newly selected country (say you edited from US to Albania, which has no
states). We fix this by clearing all the address fields from the profile
being edited, before replacing with the values in the editor.

BUG=723610
TEST=browser_tests
==========

to

==========
[Payments] When saving an edited address in editor, clear data first.

The original bug was that some information was wrongfully being retained
in the edited profile when saved, because some fields were absent in the
newly selected country (say you edited from US to Albania, which has no
states). We fix this by clearing all the address fields from the profile
being edited, before replacing with the values in the editor.

BUG=723610
TEST=browser_tests
==========

[Mathieu, 2017-07-06 12:56:41]

mathp@chromium.org changed reviewers:
+ sebsg@chromium.org

[sebsg, 2017-07-06 14:55:19]

LGTM % some comments

Nice find!

https://codereview.chromium.org/2972973002/diff/1/chrome/browser/ui/views/pay...
File
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc
(right):

https://codereview.chromium.org/2972973002/diff/1/chrome/browser/ui/views/pay...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc:40:

Nit: remove line?

https://codereview.chromium.org/2972973002/diff/1/chrome/browser/ui/views/pay...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc:724:
profile->GetRawInfo(autofill::ADDRESS_HOME_STATE));
Can you also set some metadata like use count and data and make sure it's not
reset?

Thanks!

[please use gerrit instead, 2017-07-06 15:03:06]

FYI, that's how we do this on Android:

https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...

[please use gerrit instead, 2017-07-06 15:03:48]

rouslan@chromium.org changed reviewers:
+ rouslan@chromium.org

[please use gerrit instead, 2017-07-06 15:03:48]

Moe: Can you verify iOS behavior?

[Mathieu, 2017-07-06 15:14:47]

On 2017/07/06 15:03:48, rouslan wrote:
> Moe: Can you verify iOS behavior?

Does Android have an equivalent browsertest? It's tricky to get right. For
example a France profile has CEDEX, and changing the country to US may leave
CEDEX lingering in the DB.

[please use gerrit instead, 2017-07-06 15:18:49]

On 2017/07/06 15:14:47, Mathieu wrote:
> On 2017/07/06 15:03:48, rouslan wrote:
> > Moe: Can you verify iOS behavior?
> 
> Does Android have an equivalent browsertest? It's tricky to get right. For
> example a France profile has CEDEX, and changing the country to US may leave
> CEDEX lingering in the DB.

Unfortunately, address editor is under-tested on Android. The closest thing we
have is testChangeCountryAddAddressAndPay():

https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu...

[Mathieu, 2017-07-06 15:43:43]

Thanks

https://codereview.chromium.org/2972973002/diff/1/chrome/browser/ui/views/pay...
File
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc
(right):

https://codereview.chromium.org/2972973002/diff/1/chrome/browser/ui/views/pay...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc:40:

On 2017/07/06 14:55:19, sebsg wrote:
> Nit: remove line?

Done.

https://codereview.chromium.org/2972973002/diff/1/chrome/browser/ui/views/pay...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc:724:
profile->GetRawInfo(autofill::ADDRESS_HOME_STATE));
On 2017/07/06 14:55:18, sebsg wrote:
> Can you also set some metadata like use count and data and make sure it's not
> reset?
> 
> Thanks!

Yes!

[Mathieu, 2017-07-06 15:44:19]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2017-07-06 15:44:20]

The patchset sent to the CQ was uploaded after l-g-t-m from sebsg@chromium.org
Link to the patchset: https://codereview.chromium.org/2972973002/#ps20001
(title: "addressed comments")

[commit-bot: I haz the power, 2017-07-06 15:44:37]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 16:48:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 16:48:45]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mathieu, 2017-07-06 21:19:12]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2017-07-06 21:19:31]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 22:30:40]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1499375952085750,
"parent_rev": "7603dc313cc97e1a1674d63c72dcd9b8c4fb755a", "commit_rev":
"ee7829e4607ec6e4b27efbfdf4212c8e447bde13"}

[commit-bot: I haz the power, 2017-07-06 22:31:03]

Description was changed from

==========
[Payments] When saving an edited address in editor, clear data first.

The original bug was that some information was wrongfully being retained
in the edited profile when saved, because some fields were absent in the
newly selected country (say you edited from US to Albania, which has no
states). We fix this by clearing all the address fields from the profile
being edited, before replacing with the values in the editor.

BUG=723610
TEST=browser_tests
==========

to

==========
[Payments] When saving an edited address in editor, clear data first.

The original bug was that some information was wrongfully being retained
in the edited profile when saved, because some fields were absent in the
newly selected country (say you edited from US to Albania, which has no
states). We fix this by clearing all the address fields from the profile
being edited, before replacing with the values in the editor.

BUG=723610
TEST=browser_tests

Review-Url: https://codereview.chromium.org/2972973002
Cr-Commit-Position: refs/heads/master@{#484756}
Committed:
https://chromium.googlesource.com/chromium/src/+/ee7829e4607ec6e4b27efbfdf421...
==========

[commit-bot: I haz the power, 2017-07-06 22:31:05]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/ee7829e4607ec6e4b27efbfdf421...