arc: Make Location Service manageable

arc: Make Location Service manageable.

Add policy declaration for using of Location Service

BUG= b/30469058
TEST=policy and browser tests extended.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001 (http://crrev.com/2225933002#ps20001)

Committed: https://crrev.com/75cf0e73ef617606f3e38c28d0b470f74ee821de
Cr-Commit-Position: refs/heads/master@{#412559}

[khmel, 2016-08-11 17:04:06]

Description was changed from

==========
arc: Suppress ToS and reflect admin policy in OptIn

If an account is managed and ARC++ is enabled, the account's admin
has to agree to the Terms of Service, so we do not need to show them
to the user.

The checkboxes on the welcome screen about Backup and Geolocation are
disabled if the policies ArcBackupRestoreEnabled and DefaultGeolocationSetting
already determine their value.

BUG= b/30469058
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001
(http://crrev.com/2225933002#ps20001)
==========

to

==========
arc: Make Location Service manageable.

Add policy declaration for using of Location Service

BUG= b/30469058
TEST=policy and browser tests extended.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001
(http://crrev.com/2225933002#ps20001)
==========

[khmel, 2016-08-11 17:11:31]

khmel@chromium.org changed reviewers:
+ bartfab@chromium.org, phweiss@chromium.org

[khmel, 2016-08-11 17:11:31]

Hi Bartosz,

This is continuation of the work started by Philipp in
https://codereview.chromium.org/2225933002/. I split original CL and this one
contains policy declaration. I left this code mostly unchanged, only fixed
styling and added one more browser test.

PTAL

[khmel, 2016-08-16 23:28:23]

On 2016/08/11 17:11:31, khmel wrote:
> Hi Bartosz,
> 
> This is continuation of the work started by Philipp in
> https://codereview.chromium.org/2225933002/. I split original CL and this one
> contains policy declaration. I left this code mostly unchanged, only fixed
> styling and added one more browser test.
> 
> PTAL

gentle ping

[bartfab (slow), 2016-08-17 14:52:07]

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
File chrome/browser/policy/default_geolocation_policy_handler.cc (right):

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:5: #include
"chrome/browser/policy/default_geolocation_policy_handler.h"
Nit: Add a blank line after this.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:21: const
base::Value* value = policies.GetValue(policy_name());
Nit 1: const pointer.
Nit 2: #include "base/values.h"

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:23: if (value &&
EnsureInRange(value, &value_in_range, NULL)
Nit: s/NULL/nullptr/

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:24: &&
value_in_range == 2) { // 2 = BlockGeolocation
Nit: There is a constant for that, CONTENT_SETTING_BLOCK in content_settings.h.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
File chrome/browser/policy/default_geolocation_policy_handler.h (right):

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.h:17: #if
defined(OS_CHROMEOS)
You should just exclude the files from the build on non-CrOS instead.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
File chrome/browser/policy/default_geolocation_policy_handler_unittest.cc
(right):

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler_unittest.cc:25:
TEST_F(DefaultGeolocationPolicyHandlerTest, AllowGeolocation) {
Nit: #include "testing/gtest/include/gtest/gtest.h"

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler_unittest.cc:31:
POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
Nit: #include "components/policy/core/common/policy_types.h"

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler_unittest.cc:33:
base::WrapUnique(new base::FundamentalValue(1)), nullptr);
Nit: #include "base/values.h"

https://codereview.chromium.org/2243513002/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/2243513002/diff/1/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:453:
'browser/policy/default_geolocation_policy_handler.cc',
Nit: Sort alphabetically.

[khmel, 2016-08-17 15:16:22]

Thanks Bartosz for your comments.

PTAL

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
File chrome/browser/policy/default_geolocation_policy_handler.cc (right):

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:5: #include
"chrome/browser/policy/default_geolocation_policy_handler.h"
On 2016/08/17 14:52:06, bartfab (slow) wrote:
> Nit: Add a blank line after this.

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:21: const
base::Value* value = policies.GetValue(policy_name());
On 2016/08/17 14:52:06, bartfab (slow) wrote:
> Nit 1: const pointer.
> Nit 2: #include "base/values.h"

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:23: if (value &&
EnsureInRange(value, &value_in_range, NULL)
On 2016/08/17 14:52:06, bartfab (slow) wrote:
> Nit: s/NULL/nullptr/

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.cc:24: &&
value_in_range == 2) { // 2 = BlockGeolocation
On 2016/08/17 14:52:06, bartfab (slow) wrote:
> Nit: There is a constant for that, CONTENT_SETTING_BLOCK in
content_settings.h.

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
File chrome/browser/policy/default_geolocation_policy_handler.h (right):

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler.h:17: #if
defined(OS_CHROMEOS)
On 2016/08/17 14:52:06, bartfab (slow) wrote:
> You should just exclude the files from the build on non-CrOS instead.

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
File chrome/browser/policy/default_geolocation_policy_handler_unittest.cc
(right):

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler_unittest.cc:25:
TEST_F(DefaultGeolocationPolicyHandlerTest, AllowGeolocation) {
On 2016/08/17 14:52:06, bartfab (slow) wrote:
> Nit: #include "testing/gtest/include/gtest/gtest.h"

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler_unittest.cc:31:
POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
On 2016/08/17 14:52:07, bartfab (slow) wrote:
> Nit: #include "components/policy/core/common/policy_types.h"

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/browser/policy/defau...
chrome/browser/policy/default_geolocation_policy_handler_unittest.cc:33:
base::WrapUnique(new base::FundamentalValue(1)), nullptr);
On 2016/08/17 14:52:07, bartfab (slow) wrote:
> Nit: #include "base/values.h"

Done.

https://codereview.chromium.org/2243513002/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/2243513002/diff/1/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:453:
'browser/policy/default_geolocation_policy_handler.cc',
On 2016/08/17 14:52:07, bartfab (slow) wrote:
> Nit: Sort alphabetically.

Done.

[bartfab (slow), 2016-08-17 15:37:12]

lgtm

https://codereview.chromium.org/2243513002/diff/20001/chrome/browser/policy/c...
File chrome/browser/policy/configuration_policy_handler_list_factory.cc (right):

https://codereview.chromium.org/2243513002/diff/20001/chrome/browser/policy/c...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:19: #include
"chrome/browser/policy/default_geolocation_policy_handler.h"
This #include needs to be #ifdef'd to CrOS only.

[khmel, 2016-08-17 15:40:08]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-08-17 15:40:09]

The patchset sent to the CQ was uploaded after l-g-t-m from bartfab@chromium.org
Link to the patchset: https://codereview.chromium.org/2243513002/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2016-08-17 15:40:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 16:45:34]

Description was changed from

==========
arc: Make Location Service manageable.

Add policy declaration for using of Location Service

BUG= b/30469058
TEST=policy and browser tests extended.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001
(http://crrev.com/2225933002#ps20001)
==========

to

==========
arc: Make Location Service manageable.

Add policy declaration for using of Location Service

BUG= b/30469058
TEST=policy and browser tests extended.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001
(http://crrev.com/2225933002#ps20001)
==========

[commit-bot: I haz the power, 2016-08-17 16:45:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-17 16:47:41]

Description was changed from

==========
arc: Make Location Service manageable.

Add policy declaration for using of Location Service

BUG= b/30469058
TEST=policy and browser tests extended.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001
(http://crrev.com/2225933002#ps20001)
==========

to

==========
arc: Make Location Service manageable.

Add policy declaration for using of Location Service

BUG= b/30469058
TEST=policy and browser tests extended.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

patch from issue 2225933002 at patchset 20001
(http://crrev.com/2225933002#ps20001)

Committed: https://crrev.com/75cf0e73ef617606f3e38c28d0b470f74ee821de
Cr-Commit-Position: refs/heads/master@{#412559}
==========

[commit-bot: I haz the power, 2016-08-17 16:47:41]

Patchset 3 (id:??) landed as
https://crrev.com/75cf0e73ef617606f3e38c28d0b470f74ee821de
Cr-Commit-Position: refs/heads/master@{#412559}