Skip to content

Commit

Permalink
Add ArcEnabled policy implementation
Browse files Browse the repository at this point in the history
Hide "ARC OptIn" control from Chrome:Settings for enterprise users,
map ArcEnabled policy to ArcEnabled pref.

BUG=582440

Review URL: https://codereview.chromium.org/1684063002

Cr-Commit-Position: refs/heads/master@{#382266}
  • Loading branch information
pbond authored and Commit bot committed Mar 21, 2016
1 parent adc7b8b commit 33cc17d
Show file tree
Hide file tree
Showing 20 changed files with 251 additions and 73 deletions.
45 changes: 21 additions & 24 deletions chrome/browser/chromeos/arc/arc_auth_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,24 +202,17 @@ void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
profile_, GURL(site_url));
CHECK(storage_partition_);

// In case UI is disabled we assume that ARC is opted-in.
if (!IsOptInVerificationDisabled()) {
pref_change_registrar_.Init(profile_->GetPrefs());
pref_change_registrar_.Add(
prefs::kArcEnabled,
base::Bind(&ArcAuthService::OnOptInPreferenceChanged,
base::Unretained(this)));
if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
OnOptInPreferenceChanged();
} else {
if (!disable_ui_for_testing && profile_->IsNewProfile()) {
PrefServiceSyncableFromProfile(profile_)->AddObserver(this);
OnIsSyncingChanged();
}
}
pref_change_registrar_.Init(profile_->GetPrefs());
pref_change_registrar_.Add(
prefs::kArcEnabled, base::Bind(&ArcAuthService::OnOptInPreferenceChanged,
base::Unretained(this)));
if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
OnOptInPreferenceChanged();
} else {
auth_code_.clear();
StartArc();
if (!disable_ui_for_testing && profile_->IsNewProfile()) {
PrefServiceSyncableFromProfile(profile_)->AddObserver(this);
OnIsSyncingChanged();
}
}
}

Expand Down Expand Up @@ -298,16 +291,20 @@ void ArcAuthService::OnOptInPreferenceChanged() {

if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
if (state_ != State::ACTIVE) {
CloseUI();
auth_code_.clear();

if (!profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn)) {
// Need pre-fetch auth code and show OptIn UI if needed.
initial_opt_in_ = true;
SetState(State::FETCHING_CODE);
FetchAuthCode();
if (!IsOptInVerificationDisabled()) {
CloseUI();
if (!profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn)) {
// Need pre-fetch auth code and show OptIn UI if needed.
initial_opt_in_ = true;
SetState(State::FETCHING_CODE);
FetchAuthCode();
} else {
// Ready to start Arc.
StartArc();
}
} else {
// Ready to start Arc.
StartArc();
}
}
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/policy/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include_rules = [
"+chrome",
"+chromeos",
"+components/arc/test",
"+components/drive/drive_pref_names.h",
"+components/user_manager",
"+content/public/browser",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,9 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
{ key::kUnifiedDesktopEnabledByDefault,
prefs::kUnifiedDesktopEnabledByDefault,
base::Value::TYPE_BOOLEAN },
{ key::kArcEnabled,
prefs::kArcEnabled,
base::Value::TYPE_BOOLEAN },
#endif // defined(OS_CHROMEOS)

// Metrics reporting is controlled by a platform specific policy for ChromeOS
Expand Down
92 changes: 92 additions & 0 deletions chrome/browser/policy/policy_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,20 @@
#include "ash/shell.h"
#include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
#include "chrome/browser/chromeos/accessibility/magnification_manager.h"
#include "chrome/browser/chromeos/arc/arc_auth_service.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/ash/chrome_screenshot_grabber.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
#include "chromeos/audio/cras_audio_handler.h"
#include "chromeos/chromeos_switches.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_session_manager_client.h"
#include "chromeos/dbus/session_manager_client.h"
#include "components/arc/arc_bridge_service.h"
#include "components/arc/arc_bridge_service_impl.h"
#include "components/arc/arc_service_manager.h"
#include "components/arc/test/fake_arc_bridge_bootstrap.h"
#include "components/arc/test/fake_arc_bridge_instance.h"
#include "ui/chromeos/accessibility_types.h"
#include "ui/keyboard/keyboard_util.h"
#include "ui/snapshot/screenshot_grabber.h"
Expand Down Expand Up @@ -3991,6 +4002,87 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, UnifiedDesktopEnabledByDefault) {
UpdateProviderPolicy(policies);
EXPECT_FALSE(display_manager->unified_desktop_enabled());
}

class ArcPolicyTest : public PolicyTest {
public:
ArcPolicyTest() {}
~ArcPolicyTest() override {}

protected:
void SetUpTest() {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
chromeos::switches::kDisableArcOptInVerification);
arc::ArcAuthService::DisableUIForTesting();

arc::ArcServiceManager::Get()->OnPrimaryUserProfilePrepared(
multi_user_util::GetAccountIdFromProfile(browser()->profile()));
arc::ArcAuthService::Get()->OnPrimaryUserProfilePrepared(
browser()->profile());
}

void TearDownTest() {
arc::ArcAuthService::Get()->Shutdown();
}

void SetUpInProcessBrowserTestFixture() override {
PolicyTest::SetUpInProcessBrowserTestFixture();
fake_session_manager_client_ = new chromeos::FakeSessionManagerClient;
fake_session_manager_client_->set_arc_available(true);
chromeos::DBusThreadManager::GetSetterForTesting()->SetSessionManagerClient(
scoped_ptr<chromeos::SessionManagerClient>(
fake_session_manager_client_));

fake_arc_bridge_instance_.reset(new arc::FakeArcBridgeInstance);
arc::ArcServiceManager::SetArcBridgeServiceForTesting(make_scoped_ptr(
new arc::ArcBridgeServiceImpl(make_scoped_ptr(
new arc::FakeArcBridgeBootstrap(
fake_arc_bridge_instance_.get())))));
}

private:
chromeos::FakeSessionManagerClient *fake_session_manager_client_;
scoped_ptr<arc::FakeArcBridgeInstance> fake_arc_bridge_instance_;

DISALLOW_COPY_AND_ASSIGN(ArcPolicyTest);
};

// Test ArcEnabled policy.
IN_PROC_BROWSER_TEST_F(ArcPolicyTest, ArcEnabled) {
SetUpTest();

const PrefService* const pref = browser()->profile()->GetPrefs();
const arc::ArcBridgeService* const arc_bridge_service
= arc::ArcBridgeService::Get();

// ARC is switched off by default.
EXPECT_EQ(arc::ArcBridgeService::State::STOPPED, arc_bridge_service->state());
EXPECT_FALSE(pref->GetBoolean(prefs::kArcEnabled));

// Enable ARC.
PolicyMap policies;
policies.Set(key::kArcEnabled,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::FundamentalValue(true),
nullptr);
UpdateProviderPolicy(policies);
EXPECT_TRUE(pref->GetBoolean(prefs::kArcEnabled));
EXPECT_EQ(arc::ArcBridgeService::State::READY, arc_bridge_service->state());

// Disable ARC.
policies.Set(key::kArcEnabled,
POLICY_LEVEL_MANDATORY,
POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
new base::FundamentalValue(false),
nullptr);
UpdateProviderPolicy(policies);
EXPECT_FALSE(pref->GetBoolean(prefs::kArcEnabled));
EXPECT_EQ(arc::ArcBridgeService::State::STOPPED, arc_bridge_service->state());

TearDownTest();
}
#endif // defined(OS_CHROMEOS)

} // namespace policy
4 changes: 3 additions & 1 deletion chrome/browser/resources/options/browser_options.html
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,16 @@ <h3 i18n-content="sectionTitleSearch"></h3>
</div>
</section>
<if expr="chromeos">
<section id="andorid-apps-section" guest-visibility="hidden">
<section id="android-apps-section" guest-visibility="hidden">
<h3 i18n-content="androidAppsTitle"></h3>
<div class="checkbox controlled-setting-with-label">
<label>
<input id="android-apps-enabled" pref="arc.enabled"
metric="Options_AndroidApps" type="checkbox">
<span>
<span i18n-content="androidAppsEnabled"></span>
<span class="controlled-setting-indicator"
pref="arc.enabled"></span>
</span>
</label>
</div>
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/resources/options/browser_options.js
Original file line number Diff line number Diff line change
Expand Up @@ -2327,10 +2327,11 @@ cr.define('options', function() {
};

/**
* Hides Android Apps settings when they are not available (ChromeOS only).
* Hides Android Apps settings when they are not available.
* (Chrome OS only).
*/
BrowserOptions.hideAndroidAppsSection = function() {
var section = $('andorid-apps-section');
var section = $('android-apps-section');
if (section)
section.hidden = true;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ GuestModeOptionsUIBrowserTest.prototype = {
TEST_F('GuestModeOptionsUIBrowserTest', 'testSections', function() {
this.expectHidden($('startup-section'));
this.expectHidden($('appearance-section'));
this.expectHidden($('andorid-apps-section'));
this.expectHidden($('android-apps-section'));
this.expectHidden($('sync-users-section'));
this.expectHidden($('easy-unlock-section'));
this.expectHidden($('reset-profile-settings-section'));
Expand Down
7 changes: 7 additions & 0 deletions chrome/chrome_tests.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -2432,6 +2432,13 @@
}],
['configuration_policy==1', {
'sources': [ '<@(chrome_browser_tests_policy_sources)' ],
'conditions': [
['chromeos==1', {
'dependencies': [
'../components/components.gyp:arc_test_support',
]
}]
]
}],
['enable_web_speech==1', {
'sources': [ '<@(chrome_browser_tests_speech_sources)' ],
Expand Down
3 changes: 3 additions & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,9 @@ if (!is_android) {
chrome_tests_gypi_values.chrome_browser_tests_policy_sources,
".",
"//chrome")
if (is_chromeos) {
deps += [ "//components/arc:arc_test_support" ]
}
}
if (enable_web_speech) {
sources += rebase_path(
Expand Down
5 changes: 4 additions & 1 deletion chrome/test/data/policy/policy_test_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -2436,7 +2436,10 @@
"ArcEnabled": {
"os": ["chromeos"],
"can_be_recommended": false,
"test_policy": { "ArcEnabled": false }
"test_policy": { "ArcEnabled": false },
"pref_mappings": [
{ "pref": "arc.enabled" }
]
},

"ArcApplicationPolicy": {
Expand Down
16 changes: 8 additions & 8 deletions chromeos/dbus/fake_session_manager_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ namespace chromeos {
FakeSessionManagerClient::FakeSessionManagerClient()
: start_device_wipe_call_count_(0),
notify_lock_screen_shown_call_count_(0),
notify_lock_screen_dismissed_call_count_(0) {
}
notify_lock_screen_dismissed_call_count_(0),
arc_available_(false) {}

FakeSessionManagerClient::~FakeSessionManagerClient() {
}
Expand Down Expand Up @@ -153,19 +153,19 @@ void FakeSessionManagerClient::GetServerBackedStateKeys(

void FakeSessionManagerClient::CheckArcAvailability(
const ArcCallback& callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
base::Bind(callback, false));
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, arc_available_));
}

void FakeSessionManagerClient::StartArcInstance(const std::string& socket_path,
const ArcCallback& callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
base::Bind(callback, false));
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, arc_available_));
}

void FakeSessionManagerClient::StopArcInstance(const ArcCallback& callback) {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
base::Bind(callback, false));
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(callback, arc_available_));
}

const std::string& FakeSessionManagerClient::device_policy() const {
Expand Down
4 changes: 4 additions & 0 deletions chromeos/dbus/fake_session_manager_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ class FakeSessionManagerClient : public SessionManagerClient {
return notify_lock_screen_dismissed_call_count_;
}

void set_arc_available(bool available) { arc_available_ = available; }

private:
std::string device_policy_;
std::map<cryptohome::Identification, std::string> user_policies_;
Expand All @@ -117,6 +119,8 @@ class FakeSessionManagerClient : public SessionManagerClient {
int notify_lock_screen_shown_call_count_;
int notify_lock_screen_dismissed_call_count_;

bool arc_available_;

DISALLOW_COPY_AND_ASSIGN(FakeSessionManagerClient);
};

Expand Down
2 changes: 2 additions & 0 deletions components/arc.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@
'sources': [
'arc/test/fake_app_instance.cc',
'arc/test/fake_app_instance.h',
'arc/test/fake_arc_bridge_bootstrap.cc',
'arc/test/fake_arc_bridge_bootstrap.h',
'arc/test/fake_arc_bridge_instance.cc',
'arc/test/fake_arc_bridge_instance.h',
'arc/test/fake_arc_bridge_service.cc',
Expand Down
2 changes: 2 additions & 0 deletions components/arc/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ static_library("arc_test_support") {
sources = [
"test/fake_app_instance.cc",
"test/fake_app_instance.h",
"test/fake_arc_bridge_bootstrap.cc",
"test/fake_arc_bridge_bootstrap.h",
"test/fake_arc_bridge_instance.cc",
"test/fake_arc_bridge_instance.h",
"test/fake_arc_bridge_service.cc",
Expand Down
1 change: 1 addition & 0 deletions components/arc/arc_bridge_bootstrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ void ArcBridgeBootstrapImpl::SetState(State state) {
} // namespace

ArcBridgeBootstrap::ArcBridgeBootstrap() {}

ArcBridgeBootstrap::~ArcBridgeBootstrap() {}

// static
Expand Down
Loading

0 comments on commit 33cc17d

Please sign in to comment.