[Autofill] Sync Autofill profiles' origins.

[Autofill] Sync Autofill profiles' origins.

BUG=170401
TEST=unit tests
R=estade@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207909

[Evan Stade, 2013-06-11 21:38:44]

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
File chrome/browser/webdata/autofill_profile_syncable_service.h (right):

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
chrome/browser/webdata/autofill_profile_syncable_service.h:109: // For
unit-tests.
nit: why the hyphen

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
File chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
(right):

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc:141:
.WillOnce(DoAll(CopyData(&profiles_from_web_db), Return(true)));
I think this crosses the line of acceptably simple use of gmock (where that line
is exactly is up to jam and ben, but this seems a fair bit more complex than
just EXPECT_CALL)

[Ilya Sherman, 2013-06-11 21:53:42]

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
File chrome/browser/webdata/autofill_profile_syncable_service.h (right):

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
chrome/browser/webdata/autofill_profile_syncable_service.h:109: // For
unit-tests.
On 2013/06/11 21:38:44, Evan Stade wrote:
> nit: why the hyphen

Done.

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
File chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
(right):

https://codereview.chromium.org/16024018/diff/1/chrome/browser/webdata/autofi...
chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc:141:
.WillOnce(DoAll(CopyData(&profiles_from_web_db), Return(true)));
On 2013/06/11 21:38:44, Evan Stade wrote:
> I think this crosses the line of acceptably simple use of gmock (where that
line
> is exactly is up to jam and ben, but this seems a fair bit more complex than
> just EXPECT_CALL)

This is pre-existing gmock code that I've simply moved so that it can be shared
by multiple tests.  I agree that it's pretty complex, but I don't think it's
appropriate to refactor it as part of this CL.  If you feel strongly about it, I
can spin up a separate CL specifically focused on refactoring this.  (It's also
not UI code, so Ben might not care as much, FWIW.)

[Evan Stade, 2013-06-12 22:12:24]

ok, lgtm

https://codereview.chromium.org/16024018/diff/6001/chrome/browser/webdata/aut...
File chrome/browser/webdata/autofill_profile_syncable_service.cc (right):

https://codereview.chromium.org/16024018/diff/6001/chrome/browser/webdata/aut...
chrome/browser/webdata/autofill_profile_syncable_service.cc:467: i !=
profile_map->end(); ++i) {
nit: more readable if you create a local variable for |i->second|

[Ilya Sherman, 2013-06-13 04:19:31]

+Nicolas for Sync OWNERS review.

Evan, please note that I've also added a test that Nicolas had previously asked
me to include, and moved a bit of code from
AutofillProfileSyncableService::MergeProfile() into
AutofillProfile::OverwriteWithOrAddTo(), to match what's done in [
https://codereview.chromium.org/16034018/ ].

https://codereview.chromium.org/16024018/diff/6001/chrome/browser/webdata/aut...
File chrome/browser/webdata/autofill_profile_syncable_service.cc (right):

https://codereview.chromium.org/16024018/diff/6001/chrome/browser/webdata/aut...
chrome/browser/webdata/autofill_profile_syncable_service.cc:467: i !=
profile_map->end(); ++i) {
On 2013/06/12 22:12:24, Evan Stade wrote:
> nit: more readable if you create a local variable for |i->second|

Done.

[Ilya Sherman, 2013-06-14 23:51:52]

+Tim and Fred as Sync owners, since Nicolas is OOO.

[Nicolas Zea, 2013-06-19 21:49:12]

sync LGTM

https://codereview.chromium.org/16024018/diff/36001/sync/protocol/autofill_sp...
File sync/protocol/autofill_specifics.proto (left):

https://codereview.chromium.org/16024018/diff/36001/sync/protocol/autofill_sp...
sync/protocol/autofill_specifics.proto:65: 
nit: shouldn't this still be there? (I thought we opted for newline at end of
file)

[Ilya Sherman, 2013-06-19 21:54:44]

https://codereview.chromium.org/16024018/diff/36001/sync/protocol/autofill_sp...
File sync/protocol/autofill_specifics.proto (left):

https://codereview.chromium.org/16024018/diff/36001/sync/protocol/autofill_sp...
sync/protocol/autofill_specifics.proto:65: 
On 2013/06/19 21:49:13, Nicolas Zea wrote:
> nit: shouldn't this still be there? (I thought we opted for newline at end of
> file)

There were two newlines at the end of this file; now there's just one.

[commit-bot: I haz the power, 2013-06-20 01:43:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/isherman@chromium.org/16024018/36001

[commit-bot: I haz the power, 2013-06-20 09:40:09]

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-06-21 03:51:34]

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

[commit-bot: I haz the power, 2013-06-21 05:20:04]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2013-06-21 06:11:27]

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

[commit-bot: I haz the power, 2013-06-21 13:48:37]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
cacheinvalidation_unittests, cc_unittests, check_deps, chrome_frame_net_tests,
chrome_frame_tests, chrome_frame_unittests, chromedriver2_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
gpu_unittests, installer_util_unittests, ipc_tests, jingle_unittests,
media_unittests, mini_installer_test, nacl_integration, net_unittests,
ppapi_unittests, printing_unittests, remoting_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-21 20:04:40]

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

[commit-bot: I haz the power, 2013-06-21 20:05:15]

Change committed as 207909