Disable wallet in countries where it's not supported, take 2.

Disable wallet in countries where it's not supported, take 2.

Only disable it if there's no indication the user might be interested in US addresses. App locale is not taken into account because many international users set theirs to en-US.

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

[Dan Beam, 2014-01-02 22:10:38]

generally lg

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:754: DCHECK_EQ(2U,
country_code.size());
nit: check that country_code is valid (not just 2 chars)

  std::vector<std::string> all_country_codes;
  AutofillCountry::GetAvailableCountries(&all_country_codes);
  DCHECK(all_country_codes.find(country_code) != all_country_codes.end());

(and wrap with #ifndef NDEBUG or if (DCHECK_IS_ON()) if you want)

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:756: const
std::vector<AutofillProfile*>& profiles = web_profiles();
^ what about auxiliary profiles? (i.e. why not GetProfiles()?)

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:759:
country_codes.push_back(StringToLowerASCII(UTF16ToASCII(
^ all UTF conversion functions need to have base:: now

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.h (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.h:189: // profiles with
this address or if the user's system timezone is in the given
nit: with an address in |country_code|

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/test_personal_data_manager.h (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/test_personal_data_manager.h:38: virtual
std::string CountryCodeForCurrentTimezone() const OVERRIDE;
^ why not keep this as a reference?

[Evan Stade, 2014-01-02 22:23:15]

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:754: DCHECK_EQ(2U,
country_code.size());
On 2014/01/02 22:10:39, Dan Beam wrote:
> nit: check that country_code is valid (not just 2 chars)
> 
>   std::vector<std::string> all_country_codes;
>   AutofillCountry::GetAvailableCountries(&all_country_codes);
>   DCHECK(all_country_codes.find(country_code) != all_country_codes.end());
> 
> (and wrap with #ifndef NDEBUG or if (DCHECK_IS_ON()) if you want)

I think this is overkill, I was mainly just worried that an empty string might
match the empty timezone.

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:756: const
std::vector<AutofillProfile*>& profiles = web_profiles();
On 2014/01/02 22:10:39, Dan Beam wrote:
> ^ what about auxiliary profiles? (i.e. why not GetProfiles()?)

because that causes mac to hang sometimes

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:759:
country_codes.push_back(StringToLowerASCII(UTF16ToASCII(
On 2014/01/02 22:10:39, Dan Beam wrote:
> ^ all UTF conversion functions need to have base:: now

are you sure about that? These functions don't appear to be in the base
namespace.

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.h (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.h:189: // profiles with
this address or if the user's system timezone is in the given
On 2014/01/02 22:10:39, Dan Beam wrote:
> nit: with an address in |country_code|

Done.

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/test_personal_data_manager.h (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/test_personal_data_manager.h:38: virtual
std::string CountryCodeForCurrentTimezone() const OVERRIDE;
On 2014/01/02 22:10:39, Dan Beam wrote:
> ^ why not keep this as a reference?

because the real implementation isn't able to return a const ref

[Dan Beam, 2014-01-02 22:30:36]

lgtm but we should address auxiliary profiles if possible in the future

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:756: const
std::vector<AutofillProfile*>& profiles = web_profiles();
On 2014/01/02 22:23:16, Evan Stade wrote:
> On 2014/01/02 22:10:39, Dan Beam wrote:
> > ^ what about auxiliary profiles? (i.e. why not GetProfiles()?)
> 
> because that causes mac to hang sometimes

this means we're ignoring additional data sources on mac and android, right?

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:759:
country_codes.push_back(StringToLowerASCII(UTF16ToASCII(
On 2014/01/02 22:23:16, Evan Stade wrote:
> On 2014/01/02 22:10:39, Dan Beam wrote:
> > ^ all UTF conversion functions need to have base:: now
> 
> are you sure about that? These functions don't appear to be in the base
> namespace.

i'll let you and the compiler reconcile this.

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/test_personal_data_manager.h (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/test_personal_data_manager.h:38: virtual
std::string CountryCodeForCurrentTimezone() const OVERRIDE;
On 2014/01/02 22:23:16, Evan Stade wrote:
> On 2014/01/02 22:10:39, Dan Beam wrote:
> > ^ why not keep this as a reference?
> 
> because the real implementation isn't able to return a const ref

ah

[Dan Beam, 2014-01-02 22:35:27]

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:759:
country_codes.push_back(StringToLowerASCII(UTF16ToASCII(
On 2014/01/02 22:30:37, Dan Beam wrote:
> On 2014/01/02 22:23:16, Evan Stade wrote:
> > On 2014/01/02 22:10:39, Dan Beam wrote:
> > > ^ all UTF conversion functions need to have base:: now
> > 
> > are you sure about that? These functions don't appear to be in the base
> > namespace.
> 
> i'll let you and the compiler reconcile this.

nvm, yep, sorry, avi@ didn't change this

[Evan Stade, 2014-01-02 22:36:14]

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/106033007/diff/1/components/autofill/core/bro...
components/autofill/core/browser/personal_data_manager.cc:756: const
std::vector<AutofillProfile*>& profiles = web_profiles();
On 2014/01/02 22:30:37, Dan Beam wrote:
> On 2014/01/02 22:23:16, Evan Stade wrote:
> > On 2014/01/02 22:10:39, Dan Beam wrote:
> > > ^ what about auxiliary profiles? (i.e. why not GetProfiles()?)
> > 
> > because that causes mac to hang sometimes
> 
> this means we're ignoring additional data sources on mac and android, right?

yes, unfortunately.

[commit-bot: I haz the power, 2014-01-02 22:37:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/106033007/120001

[commit-bot: I haz the power, 2014-01-02 23:31:51]

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

[commit-bot: I haz the power, 2014-01-03 23:56:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/106033007/510001

[commit-bot: I haz the power, 2014-01-04 00:36:48]

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

[commit-bot: I haz the power, 2014-01-06 19:58:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/106033007/810001

[commit-bot: I haz the power, 2014-01-07 00:46:09]

Retried try job too often on win_rel for step(s) nacl_integration
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-08 01:29:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/106033007/810001

[commit-bot: I haz the power, 2014-01-08 04:54:19]

Retried try job too often on win_rel for step(s) nacl_integration
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-08 23:55:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/106033007/810001

[commit-bot: I haz the power, 2014-01-09 01:47:33]

Change committed as 243735