Add ArcEnabled policy implementation

Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
TEST=browser_tests --gtest_filter=ArcPolicyTest*

TBR=jochen@chromium.org,dbeam@chromium.org,tnagel@chromium.org,hashimoto@chromium.org

Committed: https://crrev.com/e40ccc1743bbf43418970bc259e394ca99cb708d
Cr-Commit-Position: refs/heads/master@{#384912}

[Polina Bondarenko, 2016-03-23 15:02:45]

The CQ bit was checked by pbond@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-23 15:02:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1829703002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1829703002/1

[commit-bot: I haz the power, 2016-03-23 15:12:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-23 15:12:30]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Polina Bondarenko, 2016-03-23 15:35:41]

Patchset #2 (id:20001) has been deleted

[Polina Bondarenko, 2016-03-23 15:43:19]

Description was changed from

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
==========

to

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440

TBR=bartfab@chromium.org,jochen@chromium.org,dbeam@chromium.org,tnagel@chromi...
==========

[Polina Bondarenko, 2016-03-23 15:46:48]

pbond@chromium.org changed reviewers:
+ hidehiko@chromium.org

[Polina Bondarenko, 2016-03-23 15:46:49]

Hi,
PTAL.
The first patch is an initial reverted CL
https://codereview.chromium.org/1684063002/

I removed any changes from arc_auth_service.cc from the second patch, thus
HWTest should work now.

I added everyone from the previous CL review list to TBR, since there is a small
fix, that affects only ARC.

[hidehiko, 2016-03-24 07:17:17]

On 2016/03/23 15:46:49, Polina Bondarenko wrote:
> Hi,
> PTAL.
> The first patch is an initial reverted CL
> https://codereview.chromium.org/1684063002/
> 
> I removed any changes from arc_auth_service.cc from the second patch, thus
> HWTest should work now.
> 
> I added everyone from the previous CL review list to TBR, since there is a
small
> fix, that affects only ARC.

Could you add TEST= line in your CL description?
Could you run trybots?
Also, as you updated the chrome/browser/policy, it's better to have a code
review from its expert.

[Polina Bondarenko, 2016-03-24 10:09:54]

The CQ bit was checked by pbond@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-24 10:10:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1829703002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1829703002/40001

[Polina Bondarenko, 2016-03-24 10:14:20]

Description was changed from

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440

TBR=bartfab@chromium.org,jochen@chromium.org,dbeam@chromium.org,tnagel@chromi...
==========

to

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
TEST=browser_tests --gtest_filter=ArcPolicyTest*

TBR=jochen@chromium.org,dbeam@chromium.org,tnagel@chromium.org,hashimoto@chro...
==========

[Polina Bondarenko, 2016-03-24 10:18:01]

pbond@chromium.org changed reviewers:
+ bartfab@chromium.org

[Polina Bondarenko, 2016-03-24 10:18:03]

bartfab@chromium.org: Please review changes in 

policy_browsertests.cc

The initial commit has been reverted due to HWTest failure. It happened because
I made changes in arc_auth_service OptInVerificationDIsabled checks for testing.

[commit-bot: I haz the power, 2016-03-24 11:16:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-24 11:16:47]

Dry run: This issue passed the CQ dry run.

[bartfab (slow), 2016-04-01 10:41:19]

lgtm

https://codereview.chromium.org/1829703002/diff/40001/chrome/browser/policy/p...
File chrome/browser/policy/policy_browsertest.cc (right):

https://codereview.chromium.org/1829703002/diff/40001/chrome/browser/policy/p...
chrome/browser/policy/policy_browsertest.cc:199: #include
"chromeos/chromeos_switches.h"
Nit: No longer used.

https://codereview.chromium.org/1829703002/diff/40001/components/arc/arc_serv...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1829703002/diff/40001/components/arc/arc_serv...
components/arc/arc_service_manager.cc:28: ArcBridgeService*
g_arc_bridge_service_for_testing = nullptr;
Nit: #include "components/arc/arc_bridge_service.h"

[Polina Bondarenko, 2016-04-01 11:04:56]

The CQ bit was checked by pbond@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 11:05:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1829703002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1829703002/80001

[Polina Bondarenko, 2016-04-01 11:05:10]

Fixed comments.

https://codereview.chromium.org/1829703002/diff/40001/chrome/browser/policy/p...
File chrome/browser/policy/policy_browsertest.cc (right):

https://codereview.chromium.org/1829703002/diff/40001/chrome/browser/policy/p...
chrome/browser/policy/policy_browsertest.cc:199: #include
"chromeos/chromeos_switches.h"
On 2016/04/01 10:41:18, bartfab (slow) wrote:
> Nit: No longer used.

Done.

https://codereview.chromium.org/1829703002/diff/40001/components/arc/arc_serv...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1829703002/diff/40001/components/arc/arc_serv...
components/arc/arc_service_manager.cc:28: ArcBridgeService*
g_arc_bridge_service_for_testing = nullptr;
On 2016/04/01 10:41:18, bartfab (slow) wrote:
> Nit: #include "components/arc/arc_bridge_service.h"

Done.

[commit-bot: I haz the power, 2016-04-01 12:32:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-01 12:32:29]

Dry run: 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_...)

[bartfab (slow), 2016-04-01 14:50:56]

Still LGTM

[hidehiko, 2016-04-01 16:03:25]

On 2016/04/01 14:50:56, bartfab (slow) wrote:
> Still LGTM

The latest test looks failing. Could you take a look?

[Polina Bondarenko, 2016-04-01 21:15:31]

On 2016/04/01 16:03:25, hidehiko wrote:
> On 2016/04/01 14:50:56, bartfab (slow) wrote:
> > Still LGTM
> 
> The latest test looks failing. Could you take a look?

Yes, ArcAuthService became a SyncablePrefObserver after the last rebase and
isn't notified about ArcEnabled changes when ArcEnabled is managed by policy.

[Polina Bondarenko, 2016-04-01 21:18:04]

On 2016/04/01 21:15:31, Polina Bondarenko wrote:
> On 2016/04/01 16:03:25, hidehiko wrote:
> > On 2016/04/01 14:50:56, bartfab (slow) wrote:
> > > Still LGTM
> > 
> > The latest test looks failing. Could you take a look?
> 
> Yes, ArcAuthService became a SyncablePrefObserver after the last rebase and
> isn't notified about ArcEnabled changes when ArcEnabled is managed by policy.

*SyncedPrefObserver

[Polina Bondarenko, 2016-04-04 11:40:27]

The CQ bit was checked by pbond@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-04 11:40:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1829703002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1829703002/100001

[Polina Bondarenko, 2016-04-04 11:46:15]

On 2016/04/04 11:40:36, commit-bot: I haz the power wrote:
> Dry run: CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1829703002/100001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1829703002/100001

khmel@ fixed the problem: https://codereview.chromium.org/1852843003/

Agreed to merge it into one CL.

PTAL

[bartfab (slow), 2016-04-04 11:57:03]

https://codereview.chromium.org/1829703002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1829703002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service.cc:230:
base::Bind(&ArcAuthService::OnOptInPreferenceChanged,
Nit: #include "base/bind.h"

https://codereview.chromium.org/1829703002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service.cc:231: base::Unretained(this)));
Nit: #include "base/bind_helpers.h"

[Polina Bondarenko, 2016-04-04 12:03:21]

Fixed comments.

https://codereview.chromium.org/1829703002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1829703002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service.cc:230:
base::Bind(&ArcAuthService::OnOptInPreferenceChanged,
On 2016/04/04 11:57:03, bartfab (slow) wrote:
> Nit: #include "base/bind.h"

Done.

https://codereview.chromium.org/1829703002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service.cc:231: base::Unretained(this)));
On 2016/04/04 11:57:03, bartfab (slow) wrote:
> Nit: #include "base/bind_helpers.h"

Done.

[Polina Bondarenko, 2016-04-04 12:03:33]

The CQ bit was checked by pbond@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-04 12:03:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1829703002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1829703002/120001

[bartfab (slow), 2016-04-04 12:51:44]

lgtm

[commit-bot: I haz the power, 2016-04-04 13:18:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-04 13:18:12]

Dry run: This issue passed the CQ dry run.

[Polina Bondarenko, 2016-04-04 14:41:13]

Hi hidehiko@, PTAL at changes in arc_auth_service* files.

[hidehiko, 2016-04-04 15:18:26]

lgtm

lgtm

[hidehiko, 2016-04-04 15:18:26]

lgtm

[Polina Bondarenko, 2016-04-04 15:23:49]

On 2016/04/04 15:18:26, hidehiko wrote:
> lgtm

Thanks for reviewing this CL!

[Polina Bondarenko, 2016-04-04 15:23:56]

The CQ bit was checked by pbond@chromium.org

[commit-bot: I haz the power, 2016-04-04 15:24:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1829703002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1829703002/120001

[commit-bot: I haz the power, 2016-04-04 15:32:47]

Description was changed from

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
TEST=browser_tests --gtest_filter=ArcPolicyTest*

TBR=jochen@chromium.org,dbeam@chromium.org,tnagel@chromium.org,hashimoto@chro...
==========

to

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
TEST=browser_tests --gtest_filter=ArcPolicyTest*

TBR=jochen@chromium.org,dbeam@chromium.org,tnagel@chromium.org,hashimoto@chro...
==========

[commit-bot: I haz the power, 2016-04-04 15:32:49]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-04-04 15:33:56]

Description was changed from

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
TEST=browser_tests --gtest_filter=ArcPolicyTest*

TBR=jochen@chromium.org,dbeam@chromium.org,tnagel@chromium.org,hashimoto@chro...
==========

to

==========
Add ArcEnabled policy implementation

Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440
TEST=browser_tests --gtest_filter=ArcPolicyTest*

TBR=jochen@chromium.org,dbeam@chromium.org,tnagel@chromium.org,hashimoto@chro...

Committed: https://crrev.com/e40ccc1743bbf43418970bc259e394ca99cb708d
Cr-Commit-Position: refs/heads/master@{#384912}
==========

[commit-bot: I haz the power, 2016-04-04 15:33:58]

Patchset 6 (id:??) landed as
https://crrev.com/e40ccc1743bbf43418970bc259e394ca99cb708d
Cr-Commit-Position: refs/heads/master@{#384912}