Add a group policy controlling which sites can install extensions.

Add a group policy controlling which sites can install extensions.

BUG=55584
TBR=jhawkins@chromium.org

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

[Aaron Boodman, 2012-06-07 05:43:32]

dmazzoni: Please review policy stuff
yoz: Please review extensiony stuff

[Aaron Boodman, 2012-06-07 17:12:17]

whoops - s/dmazzoni/mnissler/

[Yoyo Zhou, 2012-06-07 19:19:15]

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/c...
File chrome/browser/extensions/crx_installer.h (left):

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/c...
chrome/browser/extensions/crx_installer.h:150: void
set_allow_off_store_install(bool val) { allow_off_store_install_ = val; }
You need to change all the other callers of this (in browsertests).

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_prefs.h (right):

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_prefs.h:436: URLPatternSet
GetAllowedInstallSites();
Should have some sort of comment on what this is. Also, I would move this up to
group it with the other getters for specific prefs.

http://codereview.chromium.org/10542048/diff/2001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

http://codereview.chromium.org/10542048/diff/2001/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:1345: // Also, when off-store-extension installs are
disabled, these sites are exempt.
Should this say "this is controlled by the administrator" like the two below?

[Aaron Boodman, 2012-06-07 22:37:11]

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/c...
File chrome/browser/extensions/crx_installer.h (left):

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/c...
chrome/browser/extensions/crx_installer.h:150: void
set_allow_off_store_install(bool val) { allow_off_store_install_ = val; }
On 2012/06/07 19:19:15, Yoyo Zhou wrote:
> You need to change all the other callers of this (in browsertests).

Fussy, fussy.

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_prefs.h (right):

http://codereview.chromium.org/10542048/diff/2001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_prefs.h:436: URLPatternSet
GetAllowedInstallSites();
On 2012/06/07 19:19:15, Yoyo Zhou wrote:
> Should have some sort of comment on what this is. Also, I would move this up
to
> group it with the other getters for specific prefs.

Done.

http://codereview.chromium.org/10542048/diff/2001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

http://codereview.chromium.org/10542048/diff/2001/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:1345: // Also, when off-store-extension installs are
disabled, these sites are exempt.
On 2012/06/07 19:19:15, Yoyo Zhou wrote:
> Should this say "this is controlled by the administrator" like the two below?

The ones below aren't strictly true. I mean anyone can set the pref, if they
edit the file. I don't think it makes sense to mention the administrator here...
that's a separate system. I'll just remove it from the other two.

[Mattias Nissler (ping if slow), 2012-06-08 09:38:22]

LGTM with nits.

http://codereview.chromium.org/10542048/diff/7001/chrome/app/policy/policy_te...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/10542048/diff/7001/chrome/app/policy/policy_te...
chrome/app/policy/policy_templates.json:1121: ExtensionInstallBlacklist takes
precedence over this policy. That is, an extension on the blacklist won't be
installed, even if it happens from a site on this list.''',
Please add a note on what happens if the policy is left unconfigured.

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/extensions/c...
File chrome/browser/extensions/crx_installer.h (right):

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/extensions/c...
chrome/browser/extensions/crx_installer.h:61: OffStoreInstallAllowedBecausePref,
nit: I'd rename this to OffStoreInstallAllowedByPref, up to you.

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/extensions/c...
chrome/browser/extensions/crx_installer.h:63: NumOffStoreInstallAllowReasons
Style guide says "Though the Google C++ Style Guide says to use kConstantNaming
for enums, Chrome still uses MACRO_STYLE naming for enums for consistency with
our existing code.". These enum constants don't follow either style.

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_prefs.cc (right):

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_prefs.cc:1910: if (!list)
This NULL check is not necessary as pref service will always give you a value.
If the pref is missing, we'll trigger the NOTREACHED() inside PrefService and
crashing is the correct course of action.

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/policy/confi...
File chrome/browser/policy/configuration_policy_handler_list.cc (right):

http://codereview.chromium.org/10542048/diff/7001/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list.cc:339: 
drop the newline here so all the extension handlers are in one block.

http://codereview.chromium.org/10542048/diff/7001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

http://codereview.chromium.org/10542048/diff/7001/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:1353: // conatin "*" meaning all extensions. This
list should not be confused with the
nit: s/conatin/contain/

[Yoyo Zhou, 2012-06-08 18:08:42]

extensions LGTM

[commit-bot: I haz the power, 2012-06-09 01:42:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/10542048/11002

[commit-bot: I haz the power, 2012-06-09 01:43:02]

Presubmit check for 10542048-11002 failed and returned exit status 1.

Running presubmit commit checks ...
Finished checking
/mnt/data/b/commit-queue/workdir/chromium/chrome/app/policy/policy_templates.json.
0 errors, 0 warnings.

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui/webui

Presubmit checks took 4.7s to calculate.

[commit-bot: I haz the power, 2012-06-09 01:46:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/10542048/11002

[commit-bot: I haz the power, 2012-06-09 06:06:00]

Change committed as 141355