Remove application locale cache in autofill code.

Remove application locale cache in autofill code (AutofillCountry::ApplicationLOcale).

This is in preparation for removing content::GetContentClient calls outside of content.

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

[Ilya Sherman, 2013-04-04 04:27:38]

https://codereview.chromium.org/13488009/diff/4004/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right):

https://codereview.chromium.org/13488009/diff/4004/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:145:
(*inputs)[j].type, g_browser_process->GetApplicationLocale());
nit: Please continue to cache this outside of the loop.  It's probably a minor
performance difference, but still, not much reason to call the method
repeatedly.  (This comment also applies to all of the other loops this change
affects.)

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

https://codereview.chromium.org/13488009/diff/4004/chrome/browser/webdata/aut...
chrome/browser/webdata/autofill_profile_syncable_service.cc:557: merge_from,
g_browser_process->GetApplicationLocale());
I'm pretty sure that AutofillProfileSyncableService::MergeProfile() is called on
the DB thread, from which it isn't safe to access g_browser_process.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
File components/autofill/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:701: for
(std::vector<AutofillProfile>::iterator i = profiles->begin();
nit: Please use "it" for the iterator name.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:707: }
nit: No need for curly braces.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:766: i !=
credit_cards->end();) {
nit: Please use "it" for the iterator name.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:771: }
nit: No need for curly braces.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
File components/autofill/browser/phone_number_i18n.cc (right):

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:152: std::string const& region,
Optional nit: As long as you're here, could you fix this to be "const
std::string&"?

[Ilya Sherman, 2013-04-04 04:36:13]

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
File components/autofill/browser/phone_number_i18n.cc (right):

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:158: if
(!ParsePhoneNumber(value, SanitizeRegion(region, app_locale),
Looking through the call sites, it looks like this call to SanitizeRegion can be
replaced with a DCHECK_EQ(2U, region.size());

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:182: SanitizeRegion(region,
app_locale),
Likewise, this one can be replaced by a DCHECK.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:197: const std::string region =
SanitizeRegion(raw_region, app_locale);
This one is still needed, though, due to callsites like the one in FindByPhone()
in autofill_profile.cc.  That pulls the country code from the profile, which
fetches it from its Address, but the Address might not have a country code set.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:232: :
region_(SanitizeRegion(region, app_locale)),
Likewise, this one can be replaced by a DCHECK.

[jam, 2013-04-04 17:58:12]

https://codereview.chromium.org/13488009/diff/4004/chrome/browser/ui/autofill...
File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right):

https://codereview.chromium.org/13488009/diff/4004/chrome/browser/ui/autofill...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:145:
(*inputs)[j].type, g_browser_process->GetApplicationLocale());
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> nit: Please continue to cache this outside of the loop.  It's probably a minor
> performance difference, but still, not much reason to call the method
> repeatedly.  (This comment also applies to all of the other loops this change
> affects.)

Done.

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

https://codereview.chromium.org/13488009/diff/4004/chrome/browser/webdata/aut...
chrome/browser/webdata/autofill_profile_syncable_service.cc:557: merge_from,
g_browser_process->GetApplicationLocale());
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> I'm pretty sure that AutofillProfileSyncableService::MergeProfile() is called
on
> the DB thread, from which it isn't safe to access g_browser_process.

thanks, fixed.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
File components/autofill/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:701: for
(std::vector<AutofillProfile>::iterator i = profiles->begin();
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> nit: Please use "it" for the iterator name.

Done.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:701: for
(std::vector<AutofillProfile>::iterator i = profiles->begin();
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> nit: Please use "it" for the iterator name.

Done.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:707: }
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> nit: No need for curly braces.

yeah, i prefer having them when there's an else. the style guide doesn't set a
rule for this

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:766: i !=
credit_cards->end();) {
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> nit: Please use "it" for the iterator name.

Done.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
File components/autofill/browser/phone_number_i18n.cc (right):

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:152: std::string const& region,
On 2013/04/04 04:27:38, Ilya Sherman wrote:
> Optional nit: As long as you're here, could you fix this to be "const
> std::string&"?

Done.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:158: if
(!ParsePhoneNumber(value, SanitizeRegion(region, app_locale),
On 2013/04/04 04:36:14, Ilya Sherman wrote:
> Looking through the call sites, it looks like this call to SanitizeRegion can
be
> replaced with a DCHECK_EQ(2U, region.size());

Done.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:182: SanitizeRegion(region,
app_locale),
On 2013/04/04 04:36:14, Ilya Sherman wrote:
> Likewise, this one can be replaced by a DCHECK.

Done.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/phone_number_i18n.cc:232: :
region_(SanitizeRegion(region, app_locale)),
On 2013/04/04 04:36:14, Ilya Sherman wrote:
> Likewise, this one can be replaced by a DCHECK.

Done.

[Ilya Sherman, 2013-04-04 23:00:39]

LGTM, thanks.

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
File components/autofill/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/13488009/diff/4004/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:707: }
On 2013/04/04 17:58:12, jam wrote:
> On 2013/04/04 04:27:38, Ilya Sherman wrote:
> > nit: No need for curly braces.
> 
> yeah, i prefer having them when there's an else. the style guide doesn't set a
> rule for this

The style guide might not set a rule for this, but Autofill code pretty
consistently omits them.