[Autofill] Move country combo box model from chrome to component.

[Autofill] Move country combo box model from chrome to component.

BUG=None

TBR=stevenjb@chromium.org
For trivial changes in:
chrome\browser\extensions\api\autofill_private\autofill_util.cc
chrome\browser\ui\webui\options\autofill_options_handler.cc

Review-Url: https://codereview.chromium.org/2705773002
Cr-Commit-Position: refs/heads/master@{#452168}
Committed: https://chromium.googlesource.com/chromium/src/+/281505194df80512d41a0ef05d10c587e67c26df

[MAD, 2017-02-18 01:46:12]

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

[commit-bot: I haz the power, 2017-02-18 01:47:30]

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-02-18 03:49:20]

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

[commit-bot: I haz the power, 2017-02-18 03:49:21]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[MAD, 2017-02-20 16:39:30]

mad@chromium.org changed reviewers:
+ mathp@google.com

[Mathieu, 2017-02-21 00:30:42]

Description was changed from

==========
Move country combo box model from chrome to component.

BUG=None
==========

to

==========
Move country combo box model from chrome to component.

BUG=None
==========

[Mathieu, 2017-02-21 00:30:42]

mathp@chromium.org changed reviewers:
+ mathp@chromium.org
- mathp@google.com

[Mathieu, 2017-02-21 00:30:57]

Description was changed from

==========
Move country combo box model from chrome to component.

BUG=None
==========

to

==========
[Autofill] Move country combo box model from chrome to component.

BUG=None
==========

[Mathieu, 2017-02-21 00:57:20]

Thanks! 

One suggestion about the test, let me know if it's doable.

https://codereview.chromium.org/2705773002/diff/1/chrome/browser/ui/autofill/...
File chrome/browser/ui/autofill/country_combobox_model_unittest.cc (right):

https://codereview.chromium.org/2705773002/diff/1/chrome/browser/ui/autofill/...
chrome/browser/ui/autofill/country_combobox_model_unittest.cc:28: manager_.Init(
Do we need lines 28-31? If not, that means we can get rid of all the chrome/
dependencies and move this test next to the feature code.

https://codereview.chromium.org/2705773002/diff/1/chrome/browser/ui/autofill/...
chrome/browser/ui/autofill/country_combobox_model_unittest.cc:35:
g_browser_process->GetApplicationLocale());
It seems like we are testing "KR" - Korea, above. So we can pass "ko-KR" here I
guess...

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
File components/autofill/core/browser/country_combobox_model.cc (right):

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.cc:14: #include
"build/build_config.h"
unused?

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
File components/autofill/core/browser/country_combobox_model.h (right):

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.h:9: #include <set>
unused?

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.h:14: #include
"base/compiler_specific.h"
unused?

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.h:16: #include
"base/memory/scoped_vector.h"
unused?

[MAD, 2017-02-21 15:52:46]

Patchset #3 (id:40001) has been deleted

[MAD, 2017-02-21 15:56:18]

This would require more than a simple file move, but a test refactoring. This
would slow down the addition of the new UI. Your choice... ;-)

BYE
MAD

https://codereview.chromium.org/2705773002/diff/1/chrome/browser/ui/autofill/...
File chrome/browser/ui/autofill/country_combobox_model_unittest.cc (right):

https://codereview.chromium.org/2705773002/diff/1/chrome/browser/ui/autofill/...
chrome/browser/ui/autofill/country_combobox_model_unittest.cc:28: manager_.Init(
On 2017/02/21 00:57:19, Mathieu Perreault wrote:
> Do we need lines 28-31? If not, that means we can get rid of all the chrome/
> dependencies and move this test next to the feature code.

It could be replaced with a mock.

https://codereview.chromium.org/2705773002/diff/1/chrome/browser/ui/autofill/...
chrome/browser/ui/autofill/country_combobox_model_unittest.cc:35:
g_browser_process->GetApplicationLocale());
On 2017/02/21 00:57:19, Mathieu Perreault wrote:
> It seems like we are testing "KR" - Korea, above. So we can pass "ko-KR" here
I
> guess...

It's a bit weird that it works with the browser process app locale here and a
hard coded country above... I could dig a bit deeper to understand better.

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
File components/autofill/core/browser/country_combobox_model.cc (right):

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.cc:14: #include
"build/build_config.h"
On 2017/02/21 00:57:19, Mathieu Perreault wrote:
> unused?

Done.

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
File components/autofill/core/browser/country_combobox_model.h (right):

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.h:9: #include <set>
On 2017/02/21 00:57:20, Mathieu Perreault wrote:
> unused?

Done.

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.h:14: #include
"base/compiler_specific.h"
On 2017/02/21 00:57:20, Mathieu Perreault wrote:
> unused?

Done.

https://codereview.chromium.org/2705773002/diff/1/components/autofill/core/br...
components/autofill/core/browser/country_combobox_model.h:16: #include
"base/memory/scoped_vector.h"
On 2017/02/21 00:57:19, Mathieu Perreault wrote:
> unused?

Done.

[Mathieu, 2017-02-21 17:37:05]

Hi MAD, sorry for the delay.

Agreed it's tricky when you have no prior knowledge of Autofill code. Luckily it
wasn't so bad, so I wrote a draft CL which removes the chrome/ dependencies, can
you integrate it?

https://codereview.chromium.org/2708143002

[MAD, 2017-02-22 16:52:22]

Patchset #4 (id:80001) has been deleted

[MAD, 2017-02-22 16:53:58]

How about this?
(there was still a content dependency left, but it wasn't used :-).

BYE
MAD

[Mathieu, 2017-02-22 17:28:39]

Yes! LGTM

[MAD, 2017-02-22 17:46:31]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2017-02-22 17:47:09]

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-02-22 17:55:43]

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

[commit-bot: I haz the power, 2017-02-22 17:55:44]

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

[MAD, 2017-02-22 18:05:24]

mad@chromium.org changed reviewers:
+ stevenjb@chromium.org

[MAD, 2017-02-22 18:05:25]

Oups, was missing OWNERs review for:

chrome\browser\extensions\api\autofill_private\autofill_util.cc   
chrome\browser\ui\webui\options\autofill_options_handler.cc

The changes are quite simple, so I'll add a TBR to the description, unless you
disagree...

[MAD, 2017-02-22 18:06:06]

Description was changed from

==========
[Autofill] Move country combo box model from chrome to component.

BUG=None
==========

to

==========
[Autofill] Move country combo box model from chrome to component.

BUG=None

TBR=stevenjb@chromium.org
For trivial changes in:
chrome\browser\extensions\api\autofill_private\autofill_util.cc
chrome\browser\ui\webui\options\autofill_options_handler.cc
==========

[MAD, 2017-02-22 18:06:11]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2017-02-22 18:06:49]

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

[stevenjb, 2017-02-22 18:09:17]

autofill_options_handler lgtm

[commit-bot: I haz the power, 2017-02-22 18:36:07]

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

[commit-bot: I haz the power, 2017-02-22 18:36:09]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[MAD, 2017-02-22 19:00:57]

The CQ bit was checked by mad@chromium.org

[commit-bot: I haz the power, 2017-02-22 19:01:32]

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-02-22 19:53:44]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1487790057602390,
"parent_rev": "68d02518d39937443244001624fa500e00aad408", "commit_rev":
"281505194df80512d41a0ef05d10c587e67c26df"}

[commit-bot: I haz the power, 2017-02-22 19:54:29]

Description was changed from

==========
[Autofill] Move country combo box model from chrome to component.

BUG=None

TBR=stevenjb@chromium.org
For trivial changes in:
chrome\browser\extensions\api\autofill_private\autofill_util.cc
chrome\browser\ui\webui\options\autofill_options_handler.cc
==========

to

==========
[Autofill] Move country combo box model from chrome to component.

BUG=None

TBR=stevenjb@chromium.org
For trivial changes in:
chrome\browser\extensions\api\autofill_private\autofill_util.cc
chrome\browser\ui\webui\options\autofill_options_handler.cc

Review-Url: https://codereview.chromium.org/2705773002
Cr-Commit-Position: refs/heads/master@{#452168}
Committed:
https://chromium.googlesource.com/chromium/src/+/281505194df80512d41a0ef05d10...
==========

[commit-bot: I haz the power, 2017-02-22 19:54:30]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/281505194df80512d41a0ef05d10...