Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(338)

Issue 1684063002: Add ArcEnabled policy implementation (Closed)

Created:
4 years, 10 months ago by Polina Bondarenko
Modified:
4 years, 8 months ago
CC:
chromium-reviews, dbeam+watch-options_chromium.org, michaelpg+watch-options_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@26869593
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add ArcEnabled policy implementation Hide "ARC OptIn" control from Chrome:Settings for enterprise users, map ArcEnabled policy to ArcEnabled pref. BUG=582440 Committed: https://crrev.com/33cc17d626fd54020cacc43e1cf2c08298ae8d52 Cr-Commit-Position: refs/heads/master@{#382266}

Patch Set 1 : #

Total comments: 3

Patch Set 2 : Disable android app section checkbox for managed profiles #

Total comments: 8

Patch Set 3 : andorid->android #

Total comments: 12

Patch Set 4 : Added browser test. #

Total comments: 55

Patch Set 5 : Fixed comments. #

Total comments: 11

Patch Set 6 : Fixed comments. #

Total comments: 6

Patch Set 7 : Inject ArcBridgeService for testing. #

Total comments: 2

Patch Set 8 : Rebase. #

Patch Set 9 : Fixed indent. #

Patch Set 10 : Fixed build issue. #

Patch Set 11 : Fixed non-chromeos and gn build errors. #

Patch Set 12 : Fixed GuestModeOptionsUIBrowserTest.testSections #

Unified diffs Side-by-side diffs Delta from patch set Stats (+251 lines, -73 lines) Patch
M chrome/browser/chromeos/arc/arc_auth_service.cc View 1 2 3 4 5 6 7 2 chunks +21 lines, -24 lines 0 comments Download
M chrome/browser/policy/DEPS View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/policy/configuration_policy_handler_list_factory.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/policy/policy_browsertest.cc View 1 2 3 4 5 6 7 2 chunks +92 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/browser_options.html View 1 2 3 4 5 6 7 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/resources/options/browser_options.js View 1 2 3 4 5 6 7 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/options/chromeos/guest_mode_options_browsertest.js View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/test/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/test/data/policy/policy_test_cases.json View 1 2 3 4 5 6 7 1 chunk +4 lines, -1 line 0 comments Download
M chromeos/dbus/fake_session_manager_client.h View 1 2 3 4 5 6 7 2 chunks +4 lines, -0 lines 0 comments Download
M chromeos/dbus/fake_session_manager_client.cc View 1 2 3 4 5 6 7 2 chunks +8 lines, -8 lines 0 comments Download
M components/arc.gypi View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M components/arc/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M components/arc/arc_bridge_bootstrap.cc View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M components/arc/arc_bridge_service_unittest.cc View 1 2 3 4 5 6 7 2 chunks +1 line, -33 lines 0 comments Download
M components/arc/arc_service_manager.h View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M components/arc/arc_service_manager.cc View 1 2 3 4 5 6 7 3 chunks +24 lines, -3 lines 0 comments Download
A components/arc/test/fake_arc_bridge_bootstrap.h View 1 2 3 4 1 chunk +34 lines, -0 lines 0 comments Download
A components/arc/test/fake_arc_bridge_bootstrap.cc View 1 2 3 4 5 6 7 8 1 chunk +33 lines, -0 lines 0 comments Download

Messages

Total messages: 70 (28 generated)
Polina Bondarenko
Hi Thiemo, PTAL
4 years, 10 months ago (2016-02-10 12:52:36 UTC) #3
Thiemo Nagel
https://codereview.chromium.org/1684063002/diff/20001/chrome/browser/ui/webui/options/browser_options_handler.cc File chrome/browser/ui/webui/options/browser_options_handler.cc (right): https://codereview.chromium.org/1684063002/diff/20001/chrome/browser/ui/webui/options/browser_options_handler.cc#newcode1051 chrome/browser/ui/webui/options/browser_options_handler.cc:1051: if (policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile) Wouldn't that hide the ARC checkbox for ...
4 years, 10 months ago (2016-02-12 17:32:08 UTC) #5
Polina Bondarenko
https://codereview.chromium.org/1684063002/diff/20001/chrome/browser/ui/webui/options/browser_options_handler.cc File chrome/browser/ui/webui/options/browser_options_handler.cc (right): https://codereview.chromium.org/1684063002/diff/20001/chrome/browser/ui/webui/options/browser_options_handler.cc#newcode1051 chrome/browser/ui/webui/options/browser_options_handler.cc:1051: if (policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile) On 2016/02/12 17:32:08, Thiemo Nagel wrote: > ...
4 years, 10 months ago (2016-02-12 17:45:00 UTC) #6
Thiemo Nagel
https://codereview.chromium.org/1684063002/diff/20001/chrome/browser/ui/webui/options/browser_options_handler.cc File chrome/browser/ui/webui/options/browser_options_handler.cc (right): https://codereview.chromium.org/1684063002/diff/20001/chrome/browser/ui/webui/options/browser_options_handler.cc#newcode1051 chrome/browser/ui/webui/options/browser_options_handler.cc:1051: if (policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile) > Do you mean that if ARC ...
4 years, 10 months ago (2016-02-12 17:53:32 UTC) #7
Polina Bondarenko
Disabled Android apps section for managed profiles.
4 years, 10 months ago (2016-02-16 08:15:33 UTC) #9
Thiemo Nagel
https://codereview.chromium.org/1684063002/diff/60001/chrome/browser/resources/options/browser_options.html File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/1684063002/diff/60001/chrome/browser/resources/options/browser_options.html#newcode225 chrome/browser/resources/options/browser_options.html:225: <section id="andorid-apps-section" guest-visibility="hidden"> "andorid" doesn't seem right. https://codereview.chromium.org/1684063002/diff/60001/chrome/browser/resources/options/browser_options.js File ...
4 years, 10 months ago (2016-02-16 10:37:17 UTC) #10
Polina Bondarenko
https://codereview.chromium.org/1684063002/diff/60001/chrome/browser/resources/options/browser_options.html File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/1684063002/diff/60001/chrome/browser/resources/options/browser_options.html#newcode225 chrome/browser/resources/options/browser_options.html:225: <section id="andorid-apps-section" guest-visibility="hidden"> On 2016/02/16 10:37:16, Thiemo Nagel wrote: ...
4 years, 10 months ago (2016-02-16 14:05:02 UTC) #12
Polina Bondarenko
bartfab@chromium.org: Please review changes.
4 years, 10 months ago (2016-02-16 14:05:53 UTC) #14
bartfab (slow)
https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc#newcode3947 chrome/browser/policy/policy_browsertest.cc:3947: IN_PROC_BROWSER_TEST_F(PolicyTest, ArcEnabled) { This test duplicates what policy_test_cases.json does ...
4 years, 10 months ago (2016-02-16 15:23:01 UTC) #15
Polina Bondarenko
https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc#newcode3947 chrome/browser/policy/policy_browsertest.cc:3947: IN_PROC_BROWSER_TEST_F(PolicyTest, ArcEnabled) { On 2016/02/16 15:23:00, bartfab (slow) wrote: ...
4 years, 10 months ago (2016-02-16 15:41:13 UTC) #16
bartfab (slow)
https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc#newcode3947 chrome/browser/policy/policy_browsertest.cc:3947: IN_PROC_BROWSER_TEST_F(PolicyTest, ArcEnabled) { On 2016/02/16 15:41:13, Polina Bondarenko wrote: ...
4 years, 10 months ago (2016-02-16 15:44:22 UTC) #17
Polina Bondarenko
Fixed comments, added browser test. https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/policy/policy_browsertest.cc#newcode3948 chrome/browser/policy/policy_browsertest.cc:3948: PrefService* pref = browser()->profile()->GetPrefs(); ...
4 years, 10 months ago (2016-02-25 15:28:31 UTC) #19
bartfab (slow)
https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/resources/options/browser_options.js File chrome/browser/resources/options/browser_options.js (right): https://codereview.chromium.org/1684063002/diff/100001/chrome/browser/resources/options/browser_options.js#newcode2323 chrome/browser/resources/options/browser_options.js:2323: * Disables Android Apps checkbox for managed profiles. (ChromeOS ...
4 years, 9 months ago (2016-03-02 14:45:41 UTC) #20
Polina Bondarenko
https://codereview.chromium.org/1684063002/diff/140001/chrome/browser/DEPS File chrome/browser/DEPS (right): https://codereview.chromium.org/1684063002/diff/140001/chrome/browser/DEPS#newcode14 chrome/browser/DEPS:14: "+components/arc/test", On 2016/03/02 14:45:40, bartfab (slow) wrote: > Does ...
4 years, 9 months ago (2016-03-06 20:22:34 UTC) #22
bartfab (slow)
https://codereview.chromium.org/1684063002/diff/140001/chromeos/dbus/fake_session_manager_client.cc File chromeos/dbus/fake_session_manager_client.cc (right): https://codereview.chromium.org/1684063002/diff/140001/chromeos/dbus/fake_session_manager_client.cc#newcode20 chromeos/dbus/fake_session_manager_client.cc:20: arc_available_(false) {} On 2016/03/06 20:22:33, Polina Bondarenko wrote: > ...
4 years, 9 months ago (2016-03-07 13:44:04 UTC) #23
Polina Bondarenko
https://codereview.chromium.org/1684063002/diff/180001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/180001/chrome/browser/policy/policy_browsertest.cc#newcode3984 chrome/browser/policy/policy_browsertest.cc:3984: arc::ArcBridgeBootstrap::SetBootstrapForTesting(make_scoped_ptr( On 2016/03/07 13:44:04, bartfab (slow) wrote: > This ...
4 years, 9 months ago (2016-03-09 17:59:15 UTC) #24
bartfab (slow)
https://codereview.chromium.org/1684063002/diff/180001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/180001/chrome/browser/policy/policy_browsertest.cc#newcode3984 chrome/browser/policy/policy_browsertest.cc:3984: arc::ArcBridgeBootstrap::SetBootstrapForTesting(make_scoped_ptr( On 2016/03/09 17:59:15, Polina Bondarenko wrote: > On ...
4 years, 9 months ago (2016-03-10 16:13:08 UTC) #25
bartfab (slow)
lgtm
4 years, 9 months ago (2016-03-10 16:15:44 UTC) #26
Polina Bondarenko
Hi, I've added implementation of ArcEnabled policy, which enables or disables ARC for enterprise users. ...
4 years, 9 months ago (2016-03-10 16:27:00 UTC) #28
Dan Beam
lgtm
4 years, 9 months ago (2016-03-14 21:18:57 UTC) #29
hidehiko
https://codereview.chromium.org/1684063002/diff/200001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/200001/chrome/browser/policy/policy_browsertest.cc#newcode3984 chrome/browser/policy/policy_browsertest.cc:3984: arc::ArcBridgeBootstrap::SetBootstrapForTesting(make_scoped_ptr( Can we inject ArcBridgeService instead of Bootstrap? https://codereview.chromium.org/1684063002/diff/200001/chrome/browser/policy/policy_browsertest.cc#newcode3987 ...
4 years, 9 months ago (2016-03-15 07:01:53 UTC) #30
Polina Bondarenko
Fixed comments. Added injection of ArcBridgeService for testing. https://codereview.chromium.org/1684063002/diff/200001/chrome/browser/policy/policy_browsertest.cc File chrome/browser/policy/policy_browsertest.cc (right): https://codereview.chromium.org/1684063002/diff/200001/chrome/browser/policy/policy_browsertest.cc#newcode3984 chrome/browser/policy/policy_browsertest.cc:3984: arc::ArcBridgeBootstrap::SetBootstrapForTesting(make_scoped_ptr( ...
4 years, 9 months ago (2016-03-15 16:16:00 UTC) #32
Polina Bondarenko
hidehiko@ and hashimoto@ ping:)
4 years, 9 months ago (2016-03-16 16:20:35 UTC) #33
Polina Bondarenko
hidehiko@ and hashimoto@ ping:)
4 years, 9 months ago (2016-03-16 16:20:36 UTC) #34
hidehiko
Sorry for being late. https://codereview.chromium.org/1684063002/diff/240001/components/arc/arc_service_manager.cc File components/arc/arc_service_manager.cc (right): https://codereview.chromium.org/1684063002/diff/240001/components/arc/arc_service_manager.cc#newcode26 components/arc/arc_service_manager.cc:26: ArcBridgeService* g_arc_bridge_service_for_testing = nullptr; Could ...
4 years, 9 months ago (2016-03-16 16:45:36 UTC) #35
hashimoto
chromeos/dbus lgtm Sorry for being late.
4 years, 9 months ago (2016-03-17 05:32:14 UTC) #37
Polina Bondarenko
Fixed hidehiko@'s comment: ArcBridgeService injection. https://codereview.chromium.org/1684063002/diff/240001/components/arc/arc_service_manager.cc File components/arc/arc_service_manager.cc (right): https://codereview.chromium.org/1684063002/diff/240001/components/arc/arc_service_manager.cc#newcode26 components/arc/arc_service_manager.cc:26: ArcBridgeService* g_arc_bridge_service_for_testing = nullptr; ...
4 years, 9 months ago (2016-03-17 13:40:28 UTC) #38
Polina Bondarenko
jochen@chromium.org: Please review changes in components/arc.gypi
4 years, 9 months ago (2016-03-17 13:46:51 UTC) #40
hidehiko
Ah you're right we accidentally create two ArcServiceManager in this way. Hmm, but it worries ...
4 years, 9 months ago (2016-03-18 07:08:27 UTC) #41
jochen (gone - plz use gerrit)
lgtm
4 years, 9 months ago (2016-03-18 15:31:58 UTC) #43
Polina Bondarenko
Thanks everybody for reviewing!
4 years, 9 months ago (2016-03-18 16:07:45 UTC) #44
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1684063002/320001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1684063002/320001
4 years, 9 months ago (2016-03-18 16:08:01 UTC) #47
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/158584)
4 years, 9 months ago (2016-03-18 16:12:44 UTC) #49
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1684063002/340001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1684063002/340001
4 years, 9 months ago (2016-03-18 16:52:53 UTC) #52
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/197405)
4 years, 9 months ago (2016-03-18 16:57:24 UTC) #54
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1684063002/350001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1684063002/350001
4 years, 9 months ago (2016-03-18 18:26:56 UTC) #57
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/183514)
4 years, 9 months ago (2016-03-18 19:49:29 UTC) #59
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1684063002/370001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1684063002/370001
4 years, 9 months ago (2016-03-19 08:41:24 UTC) #61
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 9 months ago (2016-03-19 11:13:45 UTC) #63
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1684063002/370001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1684063002/370001
4 years, 9 months ago (2016-03-21 09:47:25 UTC) #66
commit-bot: I haz the power
Committed patchset #12 (id:370001)
4 years, 9 months ago (2016-03-21 10:57:13 UTC) #68
commit-bot: I haz the power
4 years, 9 months ago (2016-03-21 10:58:37 UTC) #70
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/33cc17d626fd54020cacc43e1cf2c08298ae8d52
Cr-Commit-Position: refs/heads/master@{#382266}

Powered by Google App Engine
This is Rietveld 408576698