Allow device policy code to be optionally included.

Allow device policy code to be optionally included.

BUG=None
TEST=Existing


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

[John Knottenbelt, 2011-05-31 15:36:30]

Please could everyone review the gypi files, and specifically:

mnissler: chrome/browser/policy/ and chrome/browser/prefs
csilv: chrome/browser/ui/webui/options/
jcivelli: chrome/test/

Many thanks

John

[csilv, 2011-05-31 16:07:12]

LGTM for:
chrome/browser/ui/webui/options/

[Mattias Nissler (ping if slow), 2011-05-31 16:11:40]

prefs and policy LGTM if you address my comments and pending trybots.

http://codereview.chromium.org/7096013/diff/1020/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7096013/diff/1020/build/common.gypi#newcode143
build/common.gypi:143: 'device_policy%': 1,
The term "device policy" actually only refers to a subset of the policy
functionality. Please rename to "configuration_policy" or just "policy".

http://codereview.chromium.org/7096013/diff/1020/chrome/browser/policy/config...
File chrome/browser/policy/configuration_policy_pref_store.cc (right):

http://codereview.chromium.org/7096013/diff/1020/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_pref_store.cc:306:
ConfigurationPolicyPrefStore* ConfigurationPolicyPrefStore::Create(
Can you please move this definition down so it matches the corresponding order
in the header? You may also want to adjust the definition order for the
constructor you made private.

[Jay Civelli, 2011-05-31 16:32:41]

LGTM for chrome/test/testing_browser_process.cc

[Torne, 2011-06-01 10:45:36]

LGTM with nitpicking

http://codereview.chromium.org/7096013/diff/8001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7096013/diff/8001/build/common.gypi#newcode142
build/common.gypi:142: # Device policy is enabled by default. Set to 0 to
disable.
Change comment to match variable name below

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_browser.gypi#n...
chrome/chrome_browser.gypi:3578: ['include',
'^browser/ui/webui/options/options_managed_banner_handler_stub.cc'],
This line shouldn't be needed, it's already included by default. Also, I suggest
sorting by pathname (so the two browser/policy paths are adjacent)..

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:1506: 'browser/prefs/proxy_policy_unittest.cc',
Add in alphabetical order

[John Knottenbelt, 2011-06-01 13:13:00]

http://codereview.chromium.org/7096013/diff/1020/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7096013/diff/1020/build/common.gypi#newcode143
build/common.gypi:143: 'device_policy%': 1,
Done. Renamed to configuration_policy.
On 2011/05/31 16:11:40, Mattias Nissler wrote:
> The term "device policy" actually only refers to a subset of the policy
> functionality. Please rename to "configuration_policy" or just "policy".

http://codereview.chromium.org/7096013/diff/1020/chrome/browser/policy/config...
File chrome/browser/policy/configuration_policy_pref_store.cc (right):

http://codereview.chromium.org/7096013/diff/1020/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_pref_store.cc:306:
ConfigurationPolicyPrefStore* ConfigurationPolicyPrefStore::Create(
On 2011/05/31 16:11:40, Mattias Nissler wrote:
> Can you please move this definition down so it matches the corresponding order
> in the header? You may also want to adjust the definition order for the
> constructor you made private.

Done.

http://codereview.chromium.org/7096013/diff/8001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7096013/diff/8001/build/common.gypi#newcode142
build/common.gypi:142: # Device policy is enabled by default. Set to 0 to
disable.
On 2011/06/01 10:45:36, Torne wrote:
> Change comment to match variable name below

Done.

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_browser.gypi#n...
chrome/chrome_browser.gypi:3578: ['include',
'^browser/ui/webui/options/options_managed_banner_handler_stub.cc'],
On 2011/06/01 10:45:36, Torne wrote:
> This line shouldn't be needed, it's already included by default. Also, I
suggest
> sorting by pathname (so the two browser/policy paths are adjacent)..

Done.

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/7096013/diff/8001/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:1506: 'browser/prefs/proxy_policy_unittest.cc',
On 2011/06/01 10:45:36, Torne wrote:
> Add in alphabetical order

Done.

[commit-bot: I haz the power, 2011-06-01 13:56:27]

Change committed as 87468