Add application locale policy.

Adds the application locale police that allows to enforce the kApplicationLocale preference.

bug=44504
test=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=52936

[markusheintz_, 2010-07-19 13:03:43]

Please review asap!

THANKS.

[Nikita (slow), 2010-07-19 13:20:50]

http://codereview.chromium.org/2809058/diff/5001/6007
File chrome/browser/chromeos/login/language_switch_menu.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6007#newcode91
chrome/browser/chromeos/login/language_switch_menu.cc:91: // TODO(markusheintz):
If the preference is managed and can not be
I believe in that case we should not show language selection control at all or
gray it out with the tooltip explaining policy.

[markusheintz_, 2010-07-19 13:22:57]

http://codereview.chromium.org/2809058/diff/5001/6007
File chrome/browser/chromeos/login/language_switch_menu.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6007#newcode91
chrome/browser/chromeos/login/language_switch_menu.cc:91: // TODO(markusheintz):
If the preference is managed and can not be
I totally agree! Should have been more precise with me TODO comment.
On 2010/07/19 13:20:50, Nikita Kostylev wrote:
> I believe in that case we should not show language selection control at all or
> gray it out with the tooltip explaining policy.

[Pam (message me for reviews), 2010-07-19 14:07:49]

http://codereview.chromium.org/2809058/diff/5001/6001
File app/l10n_util.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6001#newcode384
app/l10n_util.cc:384: // First, try user prefs.
"User" prefs isn't entirely accurate now, since it covers command-line and
managed prefs too.

http://codereview.chromium.org/2809058/diff/5001/6001#newcode392
app/l10n_util.cc:392: // On ChromeOS, try user prefs. This restores the locale
used by the
Ditto "user"

http://codereview.chromium.org/2809058/diff/5001/6002
File chrome/app/chrome_dll_main.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6002#newcode727
chrome/app/chrome_dll_main.cc:727: // from the PrefService.
nits: "--lang" (lower-case); throught -> through, Therefor -> Therefore

Also, I don't quite understand this comment. Which line is it referring to, and
what needs to be changed in the future?

http://codereview.chromium.org/2809058/diff/5001/6003
File chrome/app/policy/windows/adm/en-US/chrome.adm (right):

http://codereview.chromium.org/2809058/diff/5001/6003#newcode171
chrome/app/policy/windows/adm/en-US/chrome.adm:171: Chrome will try falling back
to en-US. If the policy is not configured, Chrome will either use the preference
locale (if configured),\n\n
Rather than "try falling back to", how about simply "fall back to"? en-US should
never fail.

http://codereview.chromium.org/2809058/diff/5001/6004
File chrome/app/policy/windows/admx/chrome.admx (right):

http://codereview.chromium.org/2809058/diff/5001/6004#newcode7
chrome/app/policy/windows/admx/chrome.admx:7: <supersededAdm
fileName="D:\src\chromium\src\chrome\app\policy\windows\adm\en-US\chrome.adm" />
I don't understand these two changed lines. Surely we don't want hard-coded
development file paths checked into our tree. Or are they harmless?

http://codereview.chromium.org/2809058/diff/5001/6005
File chrome/app/policy/windows/admx/en-US/chrome.adml (right):

http://codereview.chromium.org/2809058/diff/5001/6005#newcode17
chrome/app/policy/windows/admx/en-US/chrome.adml:17: <string id="Chrome will try
falling back to en-US. If the policy is not configured, Chrome will either use
the preference locale (if configured),\n\n">Chrome will try falling back to
en-US. If the policy is not configured, Chrome will either use the preference
locale (if configured),
I don't know anything about ADML files, but this string id doesn't look right.

http://codereview.chromium.org/2809058/diff/5001/6006
File chrome/browser/browser_main_mac.mm (right):

http://codereview.chromium.org/2809058/diff/5001/6006#newcode50
chrome/browser/browser_main_mac.mm:50: // TODO(markuseintz): Read preferecence
pref::kApplicationLoacle in order
markuseintz -> markusheintz
preferecence -> preference
Loacle -> Locale
in order -> in order to

http://codereview.chromium.org/2809058/diff/5001/6007
File chrome/browser/chromeos/login/language_switch_menu.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6007#newcode92
chrome/browser/chromeos/login/language_switch_menu.cc:92: // changed by the user
a info msg should be displayed.
a info -> an info

http://codereview.chromium.org/2809058/diff/5001/6007#newcode93
chrome/browser/chromeos/login/language_switch_menu.cc:93: if
(!prefs->IsManagedPreference(prefs::kApplicationLocale)) {
This condition isn't quite right, since an extension might be controlling the
pref. You want 
  if (prefs->IsUserModifiable())
from Mattias's pending patch. Making that a TODO is all right if you get through
review before he does, but please do follow up on it.

http://codereview.chromium.org/2809058/diff/5001/6009
File chrome/browser/configuration_policy_provider.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6009#newcode48
chrome/browser/configuration_policy_provider.cc:48: Value::TYPE_STRING,
"ApplicationLocale_Value" },
Does this name come from somewhere, or is it arbitrary? None of the others use
"_Value", so can this one just be "ApplicationLocale" to match?

http://codereview.chromium.org/2809058/diff/5001/6012
File chrome/browser/pref_service.h (right):

http://codereview.chromium.org/2809058/diff/5001/6012#newcode111
chrome/browser/pref_service.h:111: // Returns true if the preference for the
given preference name available and
is available

[markusheintz_, 2010-07-19 14:59:46]

http://codereview.chromium.org/2809058/diff/5001/6001
File app/l10n_util.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6001#newcode384
app/l10n_util.cc:384: // First, try user prefs.
On 2010/07/19 14:07:49, Pam wrote:
> "User" prefs isn't entirely accurate now, since it covers command-line and
> managed prefs too.

Done.

http://codereview.chromium.org/2809058/diff/5001/6001#newcode392
app/l10n_util.cc:392: // On ChromeOS, try user prefs. This restores the locale
used by the
On 2010/07/19 14:07:49, Pam wrote:
> Ditto "user"

I changed the comment. The preference value can either be set by a policy, the
cmd line flag --Lang or by the preference value stored  last time.

http://codereview.chromium.org/2809058/diff/5001/6002
File chrome/app/chrome_dll_main.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6002#newcode727
chrome/app/chrome_dll_main.cc:727: // from the PrefService.
On 2010/07/19 14:07:49, Pam wrote:
> nits: "--lang" (lower-case); throught -> through, Therefor -> Therefore
> 
> Also, I don't quite understand this comment. Which line is it referring to,
and
> what needs to be changed in the future?

Sry this I forgot to save in uploaded the unfinished comment.

http://codereview.chromium.org/2809058/diff/5001/6003
File chrome/app/policy/windows/adm/en-US/chrome.adm (right):

http://codereview.chromium.org/2809058/diff/5001/6003#newcode171
chrome/app/policy/windows/adm/en-US/chrome.adm:171: Chrome will try falling back
to en-US. If the policy is not configured, Chrome will either use the preference
locale (if configured),\n\n
On 2010/07/19 14:07:49, Pam wrote:
> Rather than "try falling back to", how about simply "fall back to"? en-US
should
> never fail.

Done.

http://codereview.chromium.org/2809058/diff/5001/6004
File chrome/app/policy/windows/admx/chrome.admx (right):

http://codereview.chromium.org/2809058/diff/5001/6004#newcode7
chrome/app/policy/windows/admx/chrome.admx:7: <supersededAdm
fileName="D:\src\chromium\src\chrome\app\policy\windows\adm\en-US\chrome.adm" />
On 2010/07/19 14:07:49, Pam wrote:
> I don't understand these two changed lines. Surely we don't want hard-coded
> development file paths checked into our tree. Or are they harmless?

These files are auto generated from the ADM file. I think we need to discuss if
we want to keep these file in this case.

http://codereview.chromium.org/2809058/diff/5001/6005
File chrome/app/policy/windows/admx/en-US/chrome.adml (right):

http://codereview.chromium.org/2809058/diff/5001/6005#newcode17
chrome/app/policy/windows/admx/en-US/chrome.adml:17: <string id="Chrome will try
falling back to en-US. If the policy is not configured, Chrome will either use
the preference locale (if configured),\n\n">Chrome will try falling back to
en-US. If the policy is not configured, Chrome will either use the preference
locale (if configured),
On 2010/07/19 14:07:49, Pam wrote:
> I don't know anything about ADML files, but this string id doesn't look right.

See ADMX file reply. Same here. This is a auto-generated file.

http://codereview.chromium.org/2809058/diff/5001/6006
File chrome/browser/browser_main_mac.mm (right):

http://codereview.chromium.org/2809058/diff/5001/6006#newcode50
chrome/browser/browser_main_mac.mm:50: // TODO(markuseintz): Read preferecence
pref::kApplicationLoacle in order
On 2010/07/19 14:07:49, Pam wrote:
> markuseintz -> markusheintz
> preferecence -> preference
> Loacle -> Locale
> in order -> in order to

Done.

http://codereview.chromium.org/2809058/diff/5001/6007
File chrome/browser/chromeos/login/language_switch_menu.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6007#newcode92
chrome/browser/chromeos/login/language_switch_menu.cc:92: // changed by the user
a info msg should be displayed.
On 2010/07/19 14:07:49, Pam wrote:
> a info -> an info

Done.

http://codereview.chromium.org/2809058/diff/5001/6007#newcode93
chrome/browser/chromeos/login/language_switch_menu.cc:93: if
(!prefs->IsManagedPreference(prefs::kApplicationLocale)) {
On 2010/07/19 14:07:49, Pam wrote:
> This condition isn't quite right, since an extension might be controlling the
> pref. You want 
>   if (prefs->IsUserModifiable())
> from Mattias's pending patch. Making that a TODO is all right if you get
through
> review before he does, but please do follow up on it.

Done.

http://codereview.chromium.org/2809058/diff/5001/6009
File chrome/browser/configuration_policy_provider.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6009#newcode48
chrome/browser/configuration_policy_provider.cc:48: Value::TYPE_STRING,
"ApplicationLocale_Value" },
On 2010/07/19 14:07:49, Pam wrote:
> Does this name come from somewhere, or is it arbitrary? None of the others use
> "_Value", so can this one just be "ApplicationLocale" to match?

There is a name collision with the registry keys defined int he ADM template. I
will see if I can rename this.
In the ADM file I already have a ApplicationLocaleValue key for the win registry

http://codereview.chromium.org/2809058/diff/5001/6012
File chrome/browser/pref_service.h (right):

http://codereview.chromium.org/2809058/diff/5001/6012#newcode111
chrome/browser/pref_service.h:111: // Returns true if the preference for the
given preference name available and
On 2010/07/19 14:07:49, Pam wrote:
> is available

Done.

[markusheintz_, 2010-07-19 15:15:28]

http://codereview.chromium.org/2809058/diff/5001/6009
File chrome/browser/configuration_policy_provider.cc (right):

http://codereview.chromium.org/2809058/diff/5001/6009#newcode48
chrome/browser/configuration_policy_provider.cc:48: Value::TYPE_STRING,
"ApplicationLocale_Value" },
On 2010/07/19 14:59:47, markusheintz1 wrote:
> On 2010/07/19 14:07:49, Pam wrote:
> > Does this name come from somewhere, or is it arbitrary? None of the others
use
> > "_Value", so can this one just be "ApplicationLocale" to match?
> 
> There is a name collision with the registry keys defined int he ADM template.
I
> will see if I can rename this.
> In the ADM file I already have a ApplicationLocaleValue key for the win
registry

s/ApplicationLocale_Value/ApplicationLocaleValue/

[danno, 2010-07-19 15:35:46]

Policy stuff LGTM.

[Pam (message me for reviews), 2010-07-19 15:58:37]

In the log message, please change police -> policy.

Danno, do you know what's going on with that ADML file?

- Pam

http://codereview.chromium.org/2809058/diff/5001/6004
File chrome/app/policy/windows/admx/chrome.admx (right):

http://codereview.chromium.org/2809058/diff/5001/6004#newcode7
chrome/app/policy/windows/admx/chrome.admx:7: <supersededAdm
fileName="D:\src\chromium\src\chrome\app\policy\windows\adm\en-US\chrome.adm" />
> These files are auto generated from the ADM file. I think we need to discuss
if
> we want to keep these file in this case.

I have no idea. But the previous hard-coded path doesn't seem to have broken
anything, so I assume until proven otherwise that this one won't either.

http://codereview.chromium.org/2809058/diff/5001/6005
File chrome/app/policy/windows/admx/en-US/chrome.adml (right):

http://codereview.chromium.org/2809058/diff/5001/6005#newcode17
chrome/app/policy/windows/admx/en-US/chrome.adml:17: <string id="Chrome will try
falling back to en-US. If the policy is not configured, Chrome will either use
the preference locale (if configured),\n\n">Chrome will try falling back to
en-US. If the policy is not configured, Chrome will either use the preference
locale (if configured),
On 2010/07/19 14:59:47, markusheintz1 wrote:
> On 2010/07/19 14:07:49, Pam wrote:
> > I don't know anything about ADML files, but this string id doesn't look
right.
> 
> See ADMX file reply. Same here. This is a auto-generated file.

In this case it sure looks like something is wrong with the source that it's
being generated from. It's putting the text in the string IDs as well as in the
strings themselves, and splitting the strings into pieces. That doesn't happen
for any of the other strings I see.

[markusheintz_, 2010-07-19 16:04:03]

Updated ADM template, and ADMX/ADML files.

[markusheintz_, 2010-07-19 16:06:42]

http://codereview.chromium.org/2809058/diff/5001/6004
File chrome/app/policy/windows/admx/chrome.admx (right):

http://codereview.chromium.org/2809058/diff/5001/6004#newcode7
chrome/app/policy/windows/admx/chrome.admx:7: <supersededAdm
fileName="D:\src\chromium\src\chrome\app\policy\windows\adm\en-US\chrome.adm" />
On 2010/07/19 15:58:37, Pam wrote:
> > These files are auto generated from the ADM file. I think we need to discuss
> if
> > we want to keep these file in this case.
> 
> I have no idea. But the previous hard-coded path doesn't seem to have broken
> anything, so I assume until proven otherwise that this one won't either.

Danno mentioned that the path entries don't matter. So I can ignore them.

http://codereview.chromium.org/2809058/diff/5001/6005
File chrome/app/policy/windows/admx/en-US/chrome.adml (right):

http://codereview.chromium.org/2809058/diff/5001/6005#newcode17
chrome/app/policy/windows/admx/en-US/chrome.adml:17: <string id="Chrome will try
falling back to en-US. If the policy is not configured, Chrome will either use
the preference locale (if configured),\n\n">Chrome will try falling back to
en-US. If the policy is not configured, Chrome will either use the preference
locale (if configured),
On 2010/07/19 15:58:37, Pam wrote:
> On 2010/07/19 14:59:47, markusheintz1 wrote:
> > On 2010/07/19 14:07:49, Pam wrote:
> > > I don't know anything about ADML files, but this string id doesn't look
> right.
> > 
> > See ADMX file reply. Same here. This is a auto-generated file.
> 
> In this case it sure looks like something is wrong with the source that it's
> being generated from. It's putting the text in the string IDs as well as in
the
> strings themselves, and splitting the strings into pieces. That doesn't happen
> for any of the other strings I see.
> 

Fixed ADM file.

[danno, 2010-07-19 16:10:01]

http://codereview.chromium.org/2809058/diff/5001/6004
File chrome/app/policy/windows/admx/chrome.admx (right):

http://codereview.chromium.org/2809058/diff/5001/6004#newcode7
chrome/app/policy/windows/admx/chrome.admx:7: <supersededAdm
fileName="D:\src\chromium\src\chrome\app\policy\windows\adm\en-US\chrome.adm" />
On 2010/07/19 15:58:37, Pam wrote:
> > These files are auto generated from the ADM file. I think we need to discuss
> if
> > we want to keep these file in this case.
> 
> I have no idea. But the previous hard-coded path doesn't seem to have broken
> anything, so I assume until proven otherwise that this one won't either.

AFAIK, this is an informational "comment" that indicates how the admx file was
generated. We definitely need the ADM files, since ADMX doesn't work with
Windows Server 2003.

[Pam (message me for reviews), 2010-07-19 16:17:08]

LGTM with below nit fixed.

- Pam

http://codereview.chromium.org/2809058/diff/32002/33004
File chrome/app/policy/windows/adm/en-US/chrome.adm (right):

http://codereview.chromium.org/2809058/diff/32002/33004#newcode172
chrome/app/policy/windows/adm/en-US/chrome.adm:172: the system locale or the
fallback locale en-US."
These need spaces at the ends, so we don't get
configured,Chrome
supportedChrome
configured,the