Commit 58757fb2 authored by Polina Bondarenko's avatar Polina Bondarenko Committed by Chromium LUCI CQ

arc: Add disabling data snapshot feature by policy.

Add OnSnapshotsDisabled() processing to ArcDataSnapshotdManager.
All available snapshots must be removed and ongoing flows stopped
(except when MGS is already running with loaded snapshot) once the
feature is switched off.

BUG=b:170187468
TEST=components_unittests

Change-Id: I9f2c64817589639407f4294f04b077fea85aefea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2558403
Commit-Queue: Polina Bondarenko <pbond@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Reviewed-by: default avatarOleksandr Peletskyi <peletskyi@chromium.org>
Auto-Submit: Polina Bondarenko <pbond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835614}
parent bed773e1
...@@ -4504,6 +4504,7 @@ static_library("browser") { ...@@ -4504,6 +4504,7 @@ static_library("browser") {
"//chromeos/ui/vector_icons", "//chromeos/ui/vector_icons",
"//components/arc", "//components/arc",
"//components/arc:arc_base", "//components/arc:arc_base",
"//components/arc/enterprise",
"//components/arc/mojom", "//components/arc/mojom",
"//components/full_restore", "//components/full_restore",
"//components/metrics/structured", "//components/metrics/structured",
......
...@@ -33,6 +33,8 @@ ...@@ -33,6 +33,8 @@
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/geolocation/simple_geolocation_provider.h" #include "chromeos/geolocation/simple_geolocation_provider.h"
#include "chromeos/timezone/timezone_resolver.h" #include "chromeos/timezone/timezone_resolver.h"
#include "components/arc/enterprise/arc_data_snapshotd_manager.h"
#include "components/arc/enterprise/snapshot_hours_policy_service.h"
#include "components/keep_alive_registry/keep_alive_types.h" #include "components/keep_alive_registry/keep_alive_types.h"
#include "components/keep_alive_registry/scoped_keep_alive.h" #include "components/keep_alive_registry/scoped_keep_alive.h"
#include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h" #include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
...@@ -177,9 +179,19 @@ void BrowserProcessPlatformPart::InitializePrimaryProfileServices( ...@@ -177,9 +179,19 @@ void BrowserProcessPlatformPart::InitializePrimaryProfileServices(
browser_policy_connector_chromeos() browser_policy_connector_chromeos()
->GetSystemProxyManager() ->GetSystemProxyManager()
->StartObservingPrimaryProfilePrefs(primary_profile); ->StartObservingPrimaryProfilePrefs(primary_profile);
auto* manager = arc::data_snapshotd::ArcDataSnapshotdManager::Get();
if (manager) {
manager->policy_service()->StartObservingPrimaryProfilePrefs(
primary_profile->GetPrefs());
}
} }
void BrowserProcessPlatformPart::ShutdownPrimaryProfileServices() { void BrowserProcessPlatformPart::ShutdownPrimaryProfileServices() {
auto* manager = arc::data_snapshotd::ArcDataSnapshotdManager::Get();
if (manager)
manager->policy_service()->StopObservingPrimaryProfilePrefs();
browser_policy_connector_chromeos() browser_policy_connector_chromeos()
->GetSystemProxyManager() ->GetSystemProxyManager()
->StopObservingPrimaryProfilePrefs(); ->StopObservingPrimaryProfilePrefs();
......
...@@ -44,11 +44,6 @@ constexpr char kLast[] = "last"; ...@@ -44,11 +44,6 @@ constexpr char kLast[] = "last";
constexpr char kBlockedUiReboot[] = "blocked_ui_reboot"; constexpr char kBlockedUiReboot[] = "blocked_ui_reboot";
constexpr char kStarted[] = "started"; constexpr char kStarted[] = "started";
bool IsSnapshotEnabled() {
// TODO(pbond): implement policy processing.
return ArcDataSnapshotdManager::is_snapshot_enabled_for_testing();
}
// Returns true if the Chrome session is restored after crash. // Returns true if the Chrome session is restored after crash.
bool IsRestoredSession() { bool IsRestoredSession() {
auto* command_line = base::CommandLine::ForCurrentProcess(); auto* command_line = base::CommandLine::ForCurrentProcess();
...@@ -274,7 +269,8 @@ ArcDataSnapshotdManager::ArcDataSnapshotdManager( ...@@ -274,7 +269,8 @@ ArcDataSnapshotdManager::ArcDataSnapshotdManager(
std::unique_ptr<Delegate> delegate, std::unique_ptr<Delegate> delegate,
std::unique_ptr<ArcAppsTracker> apps_tracker, std::unique_ptr<ArcAppsTracker> apps_tracker,
base::OnceClosure attempt_user_exit_callback) base::OnceClosure attempt_user_exit_callback)
: snapshot_{local_state}, : policy_service_{local_state},
snapshot_{local_state},
delegate_(std::move(delegate)), delegate_(std::move(delegate)),
apps_tracker_(std::move(apps_tracker)), apps_tracker_(std::move(apps_tracker)),
attempt_user_exit_callback_(std::move(attempt_user_exit_callback)) { attempt_user_exit_callback_(std::move(attempt_user_exit_callback)) {
...@@ -286,6 +282,7 @@ ArcDataSnapshotdManager::ArcDataSnapshotdManager( ...@@ -286,6 +282,7 @@ ArcDataSnapshotdManager::ArcDataSnapshotdManager(
g_arc_data_snapshotd_manager = this; g_arc_data_snapshotd_manager = this;
snapshot_.Parse(); snapshot_.Parse();
policy_service_.AddObserver(this);
if (IsRestoredSession()) { if (IsRestoredSession()) {
state_ = State::kRestored; state_ = State::kRestored;
...@@ -305,6 +302,7 @@ ArcDataSnapshotdManager::~ArcDataSnapshotdManager() { ...@@ -305,6 +302,7 @@ ArcDataSnapshotdManager::~ArcDataSnapshotdManager() {
if (session_controller_) if (session_controller_)
session_controller_->RemoveObserver(this); session_controller_->RemoveObserver(this);
policy_service_.RemoveObserver(this);
snapshot_.Sync(); snapshot_.Sync();
EnsureDaemonStopped(base::DoNothing()); EnsureDaemonStopped(base::DoNothing());
...@@ -339,6 +337,7 @@ void ArcDataSnapshotdManager::StartLoadingSnapshot(base::OnceClosure callback) { ...@@ -339,6 +337,7 @@ void ArcDataSnapshotdManager::StartLoadingSnapshot(base::OnceClosure callback) {
std::string account_id = GetCryptohomeAccountId(); std::string account_id = GetCryptohomeAccountId();
if (!account_id.empty() && IsSnapshotEnabled() && if (!account_id.empty() && IsSnapshotEnabled() &&
(snapshot_.last() || snapshot_.previous())) { (snapshot_.last() || snapshot_.previous())) {
state_ = State::kLoading;
EnsureDaemonStarted(base::BindOnce( EnsureDaemonStarted(base::BindOnce(
&ArcDataSnapshotdManager::LoadSnapshot, weak_ptr_factory_.GetWeakPtr(), &ArcDataSnapshotdManager::LoadSnapshot, weak_ptr_factory_.GetWeakPtr(),
std::move(account_id), std::move(callback))); std::move(account_id), std::move(callback)));
...@@ -349,26 +348,30 @@ void ArcDataSnapshotdManager::StartLoadingSnapshot(base::OnceClosure callback) { ...@@ -349,26 +348,30 @@ void ArcDataSnapshotdManager::StartLoadingSnapshot(base::OnceClosure callback) {
bool ArcDataSnapshotdManager::IsAutoLoginConfigured() { bool ArcDataSnapshotdManager::IsAutoLoginConfigured() {
switch (state_) { switch (state_) {
case ArcDataSnapshotdManager::State::kBlockedUi: case State::kBlockedUi:
case ArcDataSnapshotdManager::State::kMgsToLaunch: case State::kMgsToLaunch:
case ArcDataSnapshotdManager::State::kMgsLaunched: case State::kMgsLaunched:
return true; return true;
case ArcDataSnapshotdManager::State::kNone: case State::kLoading:
case ArcDataSnapshotdManager::State::kRestored: case State::kNone:
case ArcDataSnapshotdManager::State::kRunning: case State::kRestored:
case State::kRunning:
case State::kStopping:
return false; return false;
} }
} }
bool ArcDataSnapshotdManager::IsAutoLoginAllowed() { bool ArcDataSnapshotdManager::IsAutoLoginAllowed() {
switch (state_) { switch (state_) {
case ArcDataSnapshotdManager::State::kBlockedUi: case State::kBlockedUi:
case State::kLoading:
case State::kStopping:
return false; return false;
case ArcDataSnapshotdManager::State::kNone: case State::kNone:
case ArcDataSnapshotdManager::State::kRestored: case State::kRestored:
case ArcDataSnapshotdManager::State::kRunning: case State::kRunning:
case ArcDataSnapshotdManager::State::kMgsLaunched: case State::kMgsLaunched:
case ArcDataSnapshotdManager::State::kMgsToLaunch: case State::kMgsToLaunch:
return true; return true;
} }
} }
...@@ -410,9 +413,11 @@ void ArcDataSnapshotdManager::OnSnapshotSessionFailed() { ...@@ -410,9 +413,11 @@ void ArcDataSnapshotdManager::OnSnapshotSessionFailed() {
EnsureDaemonStopped(std::move(attempt_user_exit_callback_)); EnsureDaemonStopped(std::move(attempt_user_exit_callback_));
break; break;
case State::kBlockedUi: case State::kBlockedUi:
case State::kLoading:
case State::kMgsToLaunch:
case State::kNone: case State::kNone:
case State::kRestored: case State::kRestored:
case State::kMgsToLaunch: case State::kStopping:
NOTREACHED(); NOTREACHED();
} }
} }
...@@ -423,6 +428,45 @@ void ArcDataSnapshotdManager::OnSnapshotAppInstalled(int percent) { ...@@ -423,6 +428,45 @@ void ArcDataSnapshotdManager::OnSnapshotAppInstalled(int percent) {
Update(percent); Update(percent);
} }
void ArcDataSnapshotdManager::OnSnapshotsDisabled() {
// Stop all ongoing flows.
daemon_weak_ptr_factory_.InvalidateWeakPtrs();
weak_ptr_factory_.InvalidateWeakPtrs();
switch (state_) {
// If in process of taking or loading a snapshot, stop and restart browser.
case State::kBlockedUi:
case State::kLoading:
case State::kMgsLaunched:
case State::kMgsToLaunch:
state_ = State::kStopping;
if (session_controller_)
session_controller_->RemoveObserver(this);
session_controller_.reset();
break;
// Otherwise, stop all flows, clear snapshots and do not restart browser.
case State::kNone:
case State::kRestored:
case State::kStopping:
case State::kRunning:
break;
}
DoClearSnapshots();
}
void ArcDataSnapshotdManager::OnSnapshotUpdateEndTimeChanged() {
if (policy_service_.snapshot_update_end_time().is_null())
return;
if (!IsSnapshotEnabled())
return;
// TODO(pbond): may be start a reboot process to update a snapshot.
}
bool ArcDataSnapshotdManager::IsSnapshotEnabled() {
if (ArcDataSnapshotdManager::is_snapshot_enabled_for_testing())
return true;
return policy_service_.is_snapshot_enabled();
}
void ArcDataSnapshotdManager::StopDaemon(base::OnceClosure callback) { void ArcDataSnapshotdManager::StopDaemon(base::OnceClosure callback) {
VLOG(1) << "Stopping arc-data-snapshotd"; VLOG(1) << "Stopping arc-data-snapshotd";
daemon_weak_ptr_factory_.InvalidateWeakPtrs(); daemon_weak_ptr_factory_.InvalidateWeakPtrs();
...@@ -520,12 +564,17 @@ void ArcDataSnapshotdManager::OnSnapshotsCleared(bool success) { ...@@ -520,12 +564,17 @@ void ArcDataSnapshotdManager::OnSnapshotsCleared(bool success) {
return; return;
case State::kNone: case State::kNone:
case State::kRestored: case State::kRestored:
case State::kRunning:
StopDaemon(base::DoNothing()); StopDaemon(base::DoNothing());
return; return;
case State::kStopping:
StopDaemon(std::move(attempt_user_exit_callback_));
return;
case State::kLoading:
case State::kMgsToLaunch: case State::kMgsToLaunch:
case State::kMgsLaunched: case State::kMgsLaunched:
case State::kRunning:
LOG(WARNING) << "Snapshots are cleared while in incorrect state"; LOG(WARNING) << "Snapshots are cleared while in incorrect state";
NOTREACHED();
return; return;
} }
} }
...@@ -657,6 +706,7 @@ void ArcDataSnapshotdManager::OnSnapshotLoaded(base::OnceClosure callback, ...@@ -657,6 +706,7 @@ void ArcDataSnapshotdManager::OnSnapshotLoaded(base::OnceClosure callback,
bool last) { bool last) {
if (!success) { if (!success) {
LOG(ERROR) << "Failed to load ARC data directory snapshot."; LOG(ERROR) << "Failed to load ARC data directory snapshot.";
state_ = State::kNone;
std::move(callback).Run(); std::move(callback).Run();
return; return;
} }
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "components/arc/enterprise/arc_apps_tracker.h" #include "components/arc/enterprise/arc_apps_tracker.h"
#include "components/arc/enterprise/snapshot_hours_policy_service.h"
#include "components/arc/enterprise/snapshot_session_controller.h" #include "components/arc/enterprise/snapshot_session_controller.h"
#include "components/session_manager/core/session_manager_observer.h" #include "components/session_manager/core/session_manager_observer.h"
...@@ -31,21 +32,27 @@ class ArcDataSnapshotdBridge; ...@@ -31,21 +32,27 @@ class ArcDataSnapshotdBridge;
// This class manages ARC data/ directory snapshots and controls the lifetime of // This class manages ARC data/ directory snapshots and controls the lifetime of
// the arc-data-snapshotd daemon. // the arc-data-snapshotd daemon.
class ArcDataSnapshotdManager final class ArcDataSnapshotdManager final
: public SnapshotSessionController::Observer { : public SnapshotSessionController::Observer,
public SnapshotHoursPolicyService::Observer {
public: public:
// State of the flow. // State of the flow.
enum class State { enum class State {
kNone, kNone,
// Blocked UI mode is ON. // Blocked UI mode is ON.
kBlockedUi, kBlockedUi,
// Running with a snapshot. // In process of loading a snapshot.
kRunning, kLoading,
// In blocked UI mode, MGS can be launched. // In blocked UI mode, MGS can be launched.
kMgsToLaunch, kMgsToLaunch,
// MGS is launched to create a snapshot. // MGS is launched to create a snapshot.
kMgsLaunched, kMgsLaunched,
// User session was restored. // User session was restored.
kRestored, kRestored,
// Snapshot feature is disabled, in the process of stopping. The browser
// must be restarted once snapshots are cleared.
kStopping,
// Running with a snapshot.
kRunning,
}; };
// This class is a delegate to perform actions in Chrome. // This class is a delegate to perform actions in Chrome.
...@@ -224,6 +231,15 @@ class ArcDataSnapshotdManager final ...@@ -224,6 +231,15 @@ class ArcDataSnapshotdManager final
return is_snapshot_enabled_for_testing_; return is_snapshot_enabled_for_testing_;
} }
// SnapshotHoursPolicyService::Observer overrides:
void OnSnapshotsDisabled() override;
void OnSnapshotUpdateEndTimeChanged() override;
// Returns true if the feature is enabled.
bool IsSnapshotEnabled();
SnapshotHoursPolicyService* policy_service() { return &policy_service_; }
// Get |bridge_| for testing. // Get |bridge_| for testing.
ArcDataSnapshotdBridge* bridge() { return bridge_.get(); } ArcDataSnapshotdBridge* bridge() { return bridge_.get(); }
...@@ -296,6 +312,9 @@ class ArcDataSnapshotdManager final ...@@ -296,6 +312,9 @@ class ArcDataSnapshotdManager final
std::string GetCryptohomeAccountId(); std::string GetCryptohomeAccountId();
static bool is_snapshot_enabled_for_testing_; static bool is_snapshot_enabled_for_testing_;
SnapshotHoursPolicyService policy_service_;
State state_ = State::kNone; State state_ = State::kNone;
Snapshot snapshot_; Snapshot snapshot_;
......
...@@ -268,6 +268,8 @@ class ArcDataSnapshotdManagerBasicTest : public testing::Test { ...@@ -268,6 +268,8 @@ class ArcDataSnapshotdManagerBasicTest : public testing::Test {
prefs::kArcDataRemoveRequested, true); prefs::kArcDataRemoveRequested, true);
} }
virtual void RunUntilIdle() { task_environment_.RunUntilIdle(); }
TestUpstartClient* upstart_client() { return upstart_client_.get(); } TestUpstartClient* upstart_client() { return upstart_client_.get(); }
PrefService* local_state() { return &local_state_; } PrefService* local_state() { return &local_state_; }
user_manager::FakeUserManager* user_manager() { return fake_user_manager_; } user_manager::FakeUserManager* user_manager() { return fake_user_manager_; }
...@@ -349,7 +351,7 @@ class ArcDataSnapshotdManagerFlowTest ...@@ -349,7 +351,7 @@ class ArcDataSnapshotdManagerFlowTest
"headless"); "headless");
} }
void RunUntilIdle() { void RunUntilIdle() override {
if (is_dbus_client_available()) { if (is_dbus_client_available()) {
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
return; return;
...@@ -731,15 +733,58 @@ TEST_P(ArcDataSnapshotdManagerStateTest, StartLoadingSnapshot) { ...@@ -731,15 +733,58 @@ TEST_P(ArcDataSnapshotdManagerStateTest, StartLoadingSnapshot) {
run_loop.Run(); run_loop.Run();
} }
// Test that once snapshot feature is disabled by policy, manager clears
// available snapshots and restarts the browser if in snapshot update flow.
TEST_P(ArcDataSnapshotdManagerStateTest, OnSnapshotsDisabled) {
SetupLocalState(false /* blocked_ui_mode */);
ArcDataSnapshotdManager::set_snapshot_enabled_for_testing(true /* enabled */);
ExpectStopDaemon(false /* success */);
base::RunLoop run_loop;
auto* manager = CreateManager(run_loop.QuitClosure());
CheckSnapshots(2 /* expected_snapshots_number */,
false /* expected_blocked_ui_mode */);
manager->set_state_for_testing(expected_state());
EXPECT_EQ(manager->state(), expected_state());
EXPECT_FALSE(manager->bridge());
ExpectStartDaemon(true /* success */);
ExpectStopDaemon(true /* success */);
ArcDataSnapshotdManager::set_snapshot_enabled_for_testing(
false /* enabled */);
manager->OnSnapshotsDisabled();
switch (expected_state()) {
case ArcDataSnapshotdManager::State::kBlockedUi:
case ArcDataSnapshotdManager::State::kMgsLaunched:
case ArcDataSnapshotdManager::State::kMgsToLaunch:
case ArcDataSnapshotdManager::State::kLoading:
EXPECT_EQ(manager->state(), ArcDataSnapshotdManager::State::kStopping);
run_loop.Run();
break;
case ArcDataSnapshotdManager::State::kNone:
case ArcDataSnapshotdManager::State::kRestored:
case ArcDataSnapshotdManager::State::kRunning:
case ArcDataSnapshotdManager::State::kStopping:
EXPECT_EQ(manager->state(), expected_state());
RunUntilIdle();
break;
}
CheckSnapshots(0 /* expected_snapshots_number */,
false /* expected_blocked_ui_mode */);
}
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(
ArcDataSnapshotdManagerTest, ArcDataSnapshotdManagerTest,
ArcDataSnapshotdManagerStateTest, ArcDataSnapshotdManagerStateTest,
::testing::Values(ArcDataSnapshotdManager::State::kNone, ::testing::Values(ArcDataSnapshotdManager::State::kNone,
ArcDataSnapshotdManager::State::kBlockedUi, ArcDataSnapshotdManager::State::kBlockedUi,
ArcDataSnapshotdManager::State::kLoading,
ArcDataSnapshotdManager::State::kMgsToLaunch, ArcDataSnapshotdManager::State::kMgsToLaunch,
ArcDataSnapshotdManager::State::kMgsLaunched, ArcDataSnapshotdManager::State::kMgsLaunched,
ArcDataSnapshotdManager::State::kRestored, ArcDataSnapshotdManager::State::kRestored,
ArcDataSnapshotdManager::State::kRunning)); ArcDataSnapshotdManager::State::kRunning,
ArcDataSnapshotdManager::State::kStopping));
// Test clear snapshots flow. // Test clear snapshots flow.
TEST_P(ArcDataSnapshotdManagerFlowTest, ClearSnapshotsBasic) { TEST_P(ArcDataSnapshotdManagerFlowTest, ClearSnapshotsBasic) {
...@@ -842,6 +887,8 @@ TEST_P(ArcDataSnapshotdManagerFlowTest, LoadSnapshotsBasic) { ...@@ -842,6 +887,8 @@ TEST_P(ArcDataSnapshotdManagerFlowTest, LoadSnapshotsBasic) {
base::RunLoop run_loop; base::RunLoop run_loop;
manager->StartLoadingSnapshot( manager->StartLoadingSnapshot(
base::BindLambdaForTesting([&run_loop]() { run_loop.Quit(); })); base::BindLambdaForTesting([&run_loop]() { run_loop.Quit(); }));
EXPECT_EQ(manager->state(), ArcDataSnapshotdManager::State::kLoading);
run_loop.Run(); run_loop.Run();
if (is_dbus_client_available()) { if (is_dbus_client_available()) {
EXPECT_EQ(manager->state(), ArcDataSnapshotdManager::State::kRunning); EXPECT_EQ(manager->state(), ArcDataSnapshotdManager::State::kRunning);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment