update_engine: Policy determines P2P enabled status.
This switches the P2P Manager to use the newly introduced Update Manger
policy requests in determining whether P2P is enabled on the system.
There is a policy request for the initial state (P2PEnabled, sync) and
for tracking changes (P2PEnabledChanged, async), with the latest known
value being cached by the P2P Manager.
This also reverses a recent change that moved P2P prefs setting into the
P2PManager. In the absence of any additional logic (now cleared) there
was no point in having a dedicated method just for that, and so
dbus_service writes the prefs value directly. This affords us removing
the prefs argument when initializing the P2PManager.
BUG=chromium:425233
TEST=Unit tests.
Change-Id: I53280f05da8fe532b6502c175a8cc9ddc1e15a87
Reviewed-on: https://chromium-review.googlesource.com/226937
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/dbus_service.cc b/dbus_service.cc
index 0c39340..ba99586 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -355,11 +355,9 @@
UpdateEngineService* self,
gboolean enabled,
GError **error) {
- chromeos_update_engine::P2PManager* p2p_manager =
- self->system_state_->p2p_manager();
+ chromeos_update_engine::PrefsInterface* prefs = self->system_state_->prefs();
- if (!(p2p_manager &&
- p2p_manager->SetP2PEnabledPref(enabled))) {
+ if (!prefs->SetBoolean(chromeos_update_engine::kPrefsP2PEnabled, enabled)) {
log_and_set_response_error(
error, UPDATE_ENGINE_SERVICE_ERROR_FAILED,
StringPrintf("Error setting the update via p2p permission to %s.",
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 78e3f5c..d27f963 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -24,6 +24,7 @@
#include "update_engine/mock_prefs.h"
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/test_utils.h"
+#include "update_engine/update_manager/fake_update_manager.h"
#include "update_engine/utils.h"
namespace chromeos_update_engine {
@@ -444,7 +445,8 @@
protected:
P2PDownloadActionTest()
: loop_(nullptr),
- start_at_offset_(0) {}
+ start_at_offset_(0),
+ fake_um_(fake_system_state_.fake_clock()) {}
virtual ~P2PDownloadActionTest() {}
@@ -471,7 +473,7 @@
// Setup p2p.
FakeP2PManagerConfiguration *test_conf = new FakeP2PManagerConfiguration();
p2p_manager_.reset(P2PManager::Construct(
- test_conf, nullptr, nullptr, "cros_au", 3,
+ test_conf, nullptr, &fake_um_, "cros_au", 3,
base::TimeDelta::FromDays(5)));
fake_system_state_.set_p2p_manager(p2p_manager_.get());
}
@@ -553,6 +555,8 @@
// The requested starting offset passed to SetupDownload().
off_t start_at_offset_;
+
+ chromeos_update_manager::FakeUpdateManager fake_um_;
};
TEST_F(P2PDownloadActionTest, IsWrittenTo) {
diff --git a/fake_p2p_manager.h b/fake_p2p_manager.h
index 9ec0812..a00b91f 100644
--- a/fake_p2p_manager.h
+++ b/fake_p2p_manager.h
@@ -19,8 +19,7 @@
ensure_p2p_running_result_(false),
ensure_p2p_not_running_result_(false),
perform_housekeeping_result_(false),
- count_shared_files_result_(0),
- set_p2p_enabled_pref_result_(true) {}
+ count_shared_files_result_(0) {}
virtual ~FakeP2PManager() {}
@@ -80,10 +79,6 @@
return count_shared_files_result_;
}
- bool SetP2PEnabledPref(bool /* enabled */) override {
- return set_p2p_enabled_pref_result_;
- }
-
// Methods for controlling what the fake returns and how it acts.
void SetP2PEnabled(bool is_p2p_enabled) {
is_p2p_enabled_ = is_p2p_enabled;
@@ -105,10 +100,6 @@
count_shared_files_result_ = count_shared_files_result;
}
- void SetSetP2PEnabledPrefResult(bool set_p2p_enabled_pref_result) {
- set_p2p_enabled_pref_result_ = set_p2p_enabled_pref_result;
- }
-
void SetLookupUrlForFileResult(const std::string& url) {
lookup_url_for_file_result_ = url;
}
@@ -119,7 +110,6 @@
bool ensure_p2p_not_running_result_;
bool perform_housekeeping_result_;
int count_shared_files_result_;
- bool set_p2p_enabled_pref_result_;
std::string lookup_url_for_file_result_;
DISALLOW_COPY_AND_ASSIGN(FakeP2PManager);
diff --git a/mock_p2p_manager.h b/mock_p2p_manager.h
index 64c9c82..a0ed920 100644
--- a/mock_p2p_manager.h
+++ b/mock_p2p_manager.h
@@ -58,9 +58,6 @@
ON_CALL(*this, CountSharedFiles())
.WillByDefault(testing::Invoke(&fake_,
&FakeP2PManager::CountSharedFiles));
- ON_CALL(*this, SetP2PEnabledPref(testing::_))
- .WillByDefault(testing::Invoke(&fake_,
- &FakeP2PManager::SetP2PEnabledPref));
}
virtual ~MockP2PManager() {}
@@ -82,7 +79,6 @@
MOCK_METHOD2(FileGetVisible, bool(const std::string&, bool*));
MOCK_METHOD1(FileMakeVisible, bool(const std::string&));
MOCK_METHOD0(CountSharedFiles, int());
- MOCK_METHOD1(SetP2PEnabledPref, bool(bool));
// Returns a reference to the underlying FakeP2PManager.
FakeP2PManager& fake() {
diff --git a/p2p_manager.cc b/p2p_manager.cc
index abfb3d7..95623e9 100644
--- a/p2p_manager.cc
+++ b/p2p_manager.cc
@@ -29,17 +29,25 @@
#include <utility>
#include <vector>
+#include <base/bind.h>
#include <base/files/file_path.h>
#include <base/logging.h>
#include <base/strings/stringprintf.h>
#include "update_engine/glib_utils.h"
+#include "update_engine/update_manager/policy.h"
+#include "update_engine/update_manager/update_manager.h"
#include "update_engine/utils.h"
+using base::Bind;
+using base::Callback;
using base::FilePath;
using base::StringPrintf;
using base::Time;
using base::TimeDelta;
+using chromeos_update_manager::EvalStatus;
+using chromeos_update_manager::Policy;
+using chromeos_update_manager::UpdateManager;
using std::map;
using std::pair;
using std::string;
@@ -95,8 +103,8 @@
class P2PManagerImpl : public P2PManager {
public:
P2PManagerImpl(Configuration *configuration,
- PrefsInterface *prefs,
ClockInterface *clock,
+ UpdateManager* update_manager,
const string& file_extension,
const int num_files_to_keep,
const base::TimeDelta& max_file_age);
@@ -120,7 +128,6 @@
bool *out_result);
virtual bool FileMakeVisible(const string& file_id);
virtual int CountSharedFiles();
- bool SetP2PEnabledPref(bool enabled) override;
private:
// Enumeration for specifying visibility.
@@ -144,18 +151,24 @@
// path as well as |reason|. Returns false on failure.
bool DeleteP2PFile(const FilePath& path, const std::string& reason);
+ // Schedules an async request for tracking changes in P2P enabled status.
+ void ScheduleEnabledStatusChange();
+
+ // An async callback used by the above.
+ void OnEnabledStatusChange(EvalStatus status, const bool& result);
+
// The device policy being used or null if no policy is being used.
- const policy::DevicePolicy* device_policy_;
+ const policy::DevicePolicy* device_policy_ = nullptr;
// Configuration object.
unique_ptr<Configuration> configuration_;
- // Object for persisted state.
- PrefsInterface* prefs_;
-
// Object for telling the time.
ClockInterface* clock_;
+ // A pointer to the global Update Manager.
+ UpdateManager* update_manager_;
+
// A short string unique to the application (for example "cros_au")
// used to mark a file as being owned by a particular application.
const string file_extension_;
@@ -178,6 +191,11 @@
// Whether P2P service may be running; initially, we assume it may be.
bool may_be_running_ = true;
+ // The current known enabled status of the P2P feature (initialized lazily),
+ // and whether an async status check has been scheduled.
+ bool is_enabled_;
+ bool waiting_for_enabled_status_change_ = false;
+
DISALLOW_COPY_AND_ASSIGN(P2PManagerImpl);
};
@@ -186,14 +204,13 @@
const char P2PManagerImpl::kTmpExtension[] = ".tmp";
P2PManagerImpl::P2PManagerImpl(Configuration *configuration,
- PrefsInterface *prefs,
ClockInterface *clock,
+ UpdateManager* update_manager,
const string& file_extension,
const int num_files_to_keep,
const base::TimeDelta& max_file_age)
- : device_policy_(nullptr),
- prefs_(prefs),
- clock_(clock),
+ : clock_(clock),
+ update_manager_(update_manager),
file_extension_(file_extension),
num_files_to_keep_(num_files_to_keep),
max_file_age_(max_file_age) {
@@ -207,49 +224,19 @@
}
bool P2PManagerImpl::IsP2PEnabled() {
- bool p2p_enabled = false;
-
- // The logic we want here is additive, e.g. p2p can be enabled by
- // either the crosh flag OR by Enterprise Policy, e.g. the following
- // truth table:
- //
- // crosh_flag == FALSE && enterprise_policy == unset -> use_p2p == *
- // crosh_flag == TRUE && enterprise_policy == unset -> use_p2p == TRUE
- // crosh_flag == FALSE && enterprise_policy == FALSE -> use_p2p == FALSE
- // crosh_flag == FALSE && enterprise_policy == TRUE -> use_p2p == TRUE
- // crosh_flag == TRUE && enterprise_policy == FALSE -> use_p2p == TRUE
- // crosh_flag == TRUE && enterprise_policy == TRUE -> use_p2p == TRUE
- //
- // *: TRUE if Enterprise Enrolled, FALSE otherwise.
-
- if (prefs_ != nullptr &&
- prefs_->Exists(kPrefsP2PEnabled) &&
- prefs_->GetBoolean(kPrefsP2PEnabled, &p2p_enabled) &&
- p2p_enabled) {
- LOG(INFO) << "The crosh flag indicates that p2p is enabled.";
- return true;
- }
-
- if (device_policy_ != nullptr) {
- if (device_policy_->GetAuP2PEnabled(&p2p_enabled)) {
- if (p2p_enabled) {
- LOG(INFO) << "Enterprise Policy indicates that p2p is enabled.";
- return true;
- }
- } else {
- // Enterprise-enrolled devices have an empty owner in their device policy.
- string owner;
- if (!device_policy_->GetOwner(&owner) || owner.empty()) {
- LOG(INFO) << "No p2p_enabled setting in Enterprise Policy but device "
- << "is Enterprise Enrolled so allowing p2p.";
- return true;
- }
+ if (!waiting_for_enabled_status_change_) {
+ // Get and store an initial value.
+ if (update_manager_->PolicyRequest(&Policy::P2PEnabled, &is_enabled_) ==
+ EvalStatus::kFailed) {
+ is_enabled_ = false;
+ LOG(ERROR) << "Querying P2P enabled status failed, disabling.";
}
+
+ // Track future changes (async).
+ ScheduleEnabledStatusChange();
}
- LOG(INFO) << "Neither Enterprise Policy nor crosh flag indicates that p2p "
- << "is enabled.";
- return false;
+ return is_enabled_;
}
bool P2PManagerImpl::EnsureP2P(bool should_be_running) {
@@ -794,26 +781,53 @@
return num_files;
}
-bool P2PManagerImpl::SetP2PEnabledPref(bool enabled) {
- if (!prefs_->SetBoolean(chromeos_update_engine::kPrefsP2PEnabled, enabled))
- return false;
+void P2PManagerImpl::ScheduleEnabledStatusChange() {
+ if (waiting_for_enabled_status_change_)
+ return;
- // If P2P should not be running, make sure it isn't.
- if (may_be_running_ && !IsP2PEnabled())
- EnsureP2PNotRunning();
-
- return true;
+ Callback<void(EvalStatus, const bool&)> callback = Bind(
+ &P2PManagerImpl::OnEnabledStatusChange, base::Unretained(this));
+ update_manager_->AsyncPolicyRequest(callback, &Policy::P2PEnabledChanged,
+ is_enabled_);
+ waiting_for_enabled_status_change_ = true;
}
-P2PManager* P2PManager::Construct(Configuration *configuration,
- PrefsInterface *prefs,
- ClockInterface *clock,
- const string& file_extension,
- const int num_files_to_keep,
- const base::TimeDelta& max_file_age) {
+void P2PManagerImpl::OnEnabledStatusChange(EvalStatus status,
+ const bool& result) {
+ waiting_for_enabled_status_change_ = false;
+
+ if (status == EvalStatus::kSucceeded) {
+ if (result == is_enabled_) {
+ LOG(WARNING) << "P2P enabled status did not change, which means that it "
+ "is permanent; not scheduling further checks.";
+ waiting_for_enabled_status_change_ = true;
+ return;
+ }
+
+ is_enabled_ = result;
+
+ // If P2P is running but shouldn't be, make sure it isn't.
+ if (may_be_running_ && !is_enabled_ && !EnsureP2PNotRunning()) {
+ LOG(WARNING) << "Failed to stop P2P service.";
+ }
+ } else {
+ LOG(WARNING)
+ << "P2P enabled tracking failed (possibly timed out); retrying.";
+ }
+
+ ScheduleEnabledStatusChange();
+}
+
+P2PManager* P2PManager::Construct(
+ Configuration *configuration,
+ ClockInterface *clock,
+ UpdateManager* update_manager,
+ const string& file_extension,
+ const int num_files_to_keep,
+ const base::TimeDelta& max_file_age) {
return new P2PManagerImpl(configuration,
- prefs,
clock,
+ update_manager,
file_extension,
num_files_to_keep,
max_file_age);
diff --git a/p2p_manager.h b/p2p_manager.h
index 4c6e508..fb07c2e 100644
--- a/p2p_manager.h
+++ b/p2p_manager.h
@@ -17,6 +17,7 @@
#include "update_engine/clock_interface.h"
#include "update_engine/prefs_interface.h"
+#include "update_engine/update_manager/update_manager.h"
namespace chromeos_update_engine {
@@ -53,9 +54,8 @@
// null, then no device policy is used.
virtual void SetDevicePolicy(const policy::DevicePolicy* device_policy) = 0;
- // Returns true if - and only if - P2P should be used on this
- // device. This value is derived from a variety of sources including
- // enterprise policy.
+ // Returns true iff P2P is currently allowed for use on this device. This
+ // value is determined and maintained by the Update Manager.
virtual bool IsP2PEnabled() = 0;
// Ensures that the p2p subsystem is running (e.g. starts it if it's
@@ -150,12 +150,6 @@
// occurred.
virtual int CountSharedFiles() = 0;
- // Updates the preference setting for enabling P2P. If P2P is disabled as a
- // result, attempts to ensure that the service is not running. Returns true if
- // the setting was updated successfully (even through stopping the service may
- // have failed).
- virtual bool SetP2PEnabledPref(bool enabled) = 0;
-
// Creates a suitable P2PManager instance and initializes the object
// so it's ready for use. The |file_extension| parameter is used to
// identify your application, use e.g. "cros_au". If
@@ -168,12 +162,13 @@
// method) - pass zero to allow infinitely many files. The
// |max_file_age| parameter specifies the maximum file age after
// performing housekeeping (pass zero to allow files of any age).
- static P2PManager* Construct(Configuration *configuration,
- PrefsInterface *prefs,
- ClockInterface *clock,
- const std::string& file_extension,
- const int num_files_to_keep,
- const base::TimeDelta& max_file_age);
+ static P2PManager* Construct(
+ Configuration *configuration,
+ ClockInterface *clock,
+ chromeos_update_manager::UpdateManager* update_manager,
+ const std::string& file_extension,
+ const int num_files_to_keep,
+ const base::TimeDelta& max_file_age);
};
} // namespace chromeos_update_engine
diff --git a/p2p_manager_unittest.cc b/p2p_manager_unittest.cc
index fe53955..a790289 100644
--- a/p2p_manager_unittest.cc
+++ b/p2p_manager_unittest.cc
@@ -4,10 +4,9 @@
#include "update_engine/p2p_manager.h"
-#include <glib.h>
-
#include <dirent.h>
#include <fcntl.h>
+#include <glib.h>
#include <sys/stat.h>
#include <unistd.h>
#include <attr/xattr.h> // NOLINT - requires typed defined in unistd.h
@@ -26,11 +25,17 @@
#include "update_engine/fake_p2p_manager_configuration.h"
#include "update_engine/prefs.h"
#include "update_engine/test_utils.h"
+#include "update_engine/update_manager/fake_update_manager.h"
+#include "update_engine/update_manager/mock_policy.h"
#include "update_engine/utils.h"
using base::TimeDelta;
using std::string;
using std::unique_ptr;
+using testing::DoAll;
+using testing::Return;
+using testing::SetArgPointee;
+using testing::_;
namespace chromeos_update_engine {
@@ -39,176 +44,73 @@
// done.
class P2PManagerTest : public testing::Test {
protected:
- P2PManagerTest() {}
+ P2PManagerTest() : fake_um_(&fake_clock_) {}
virtual ~P2PManagerTest() {}
// Derived from testing::Test.
virtual void SetUp() {
test_conf_ = new FakeP2PManagerConfiguration();
+
+ // Allocate and install a mock policy implementation in the fake Update
+ // Manager. Note that the FakeUpdateManager takes ownership of the policy
+ // object.
+ mock_policy_ = new chromeos_update_manager::MockPolicy(&fake_clock_);
+ fake_um_.set_policy(mock_policy_);
+
+ // Construct the P2P manager under test.
+ manager_.reset(P2PManager::Construct(test_conf_, &fake_clock_, &fake_um_,
+ "cros_au", 3,
+ base::TimeDelta::FromDays(5)));
}
virtual void TearDown() {}
// The P2PManager::Configuration instance used for testing.
FakeP2PManagerConfiguration *test_conf_;
+
+ FakeClock fake_clock_;
+ chromeos_update_manager::MockPolicy *mock_policy_ = nullptr;
+ chromeos_update_manager::FakeUpdateManager fake_um_;
+
+ unique_ptr<P2PManager> manager_;
};
-// Check that IsP2PEnabled() returns false if neither the crosh flag
-// nor the Enterprise Policy indicates p2p is to be used.
-TEST_F(P2PManagerTest, P2PEnabledNeitherCroshFlagNotEnterpriseSetting) {
- string temp_dir;
- Prefs prefs;
- EXPECT_TRUE(utils::MakeTempDirectory("PayloadStateP2PTests.XXXXXX",
- &temp_dir));
- prefs.Init(base::FilePath(temp_dir));
+// Check that IsP2PEnabled() polls the policy correctly, with the value not
+// changing between calls.
+TEST_F(P2PManagerTest, P2PEnabledInitAndNotChanged) {
+ EXPECT_CALL(*mock_policy_, P2PEnabled(_, _, _, _));
+ EXPECT_CALL(*mock_policy_, P2PEnabledChanged(_, _, _, _, false));
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, &prefs, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- EXPECT_FALSE(manager->IsP2PEnabled());
-
- EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
+ EXPECT_FALSE(manager_->IsP2PEnabled());
+ RunGMainLoopMaxIterations(100);
+ EXPECT_FALSE(manager_->IsP2PEnabled());
}
-// Check that IsP2PEnabled() corresponds to value of the crosh flag
-// when Enterprise Policy is not set.
-TEST_F(P2PManagerTest, P2PEnabledCroshFlag) {
- string temp_dir;
- Prefs prefs;
- EXPECT_TRUE(utils::MakeTempDirectory("PayloadStateP2PTests.XXXXXX",
- &temp_dir));
- prefs.Init(base::FilePath(temp_dir));
+// Check that IsP2PEnabled() polls the policy correctly, with the value changing
+// between calls.
+TEST_F(P2PManagerTest, P2PEnabledInitAndChanged) {
+ EXPECT_CALL(*mock_policy_, P2PEnabled(_, _, _, _))
+ .WillOnce(DoAll(
+ SetArgPointee<3>(true),
+ Return(chromeos_update_manager::EvalStatus::kSucceeded)));
+ EXPECT_CALL(*mock_policy_, P2PEnabledChanged(_, _, _, _, true));
+ EXPECT_CALL(*mock_policy_, P2PEnabledChanged(_, _, _, _, false));
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, &prefs, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- EXPECT_FALSE(manager->IsP2PEnabled());
- prefs.SetBoolean(kPrefsP2PEnabled, true);
- EXPECT_TRUE(manager->IsP2PEnabled());
- prefs.SetBoolean(kPrefsP2PEnabled, false);
- EXPECT_FALSE(manager->IsP2PEnabled());
-
- EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
-}
-
-// Check that IsP2PEnabled() always returns true if Enterprise Policy
-// indicates that p2p is to be used.
-TEST_F(P2PManagerTest, P2PEnabledEnterpriseSettingTrue) {
- string temp_dir;
- Prefs prefs;
- EXPECT_TRUE(utils::MakeTempDirectory("PayloadStateP2PTests.XXXXXX",
- &temp_dir));
- prefs.Init(base::FilePath(temp_dir));
-
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, &prefs, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- unique_ptr<policy::MockDevicePolicy> device_policy(
- new policy::MockDevicePolicy());
- EXPECT_CALL(*device_policy, GetAuP2PEnabled(testing::_)).WillRepeatedly(
- DoAll(testing::SetArgumentPointee<0>(true),
- testing::Return(true)));
- manager->SetDevicePolicy(device_policy.get());
- EXPECT_TRUE(manager->IsP2PEnabled());
-
- // Should still return true even if crosh flag says otherwise.
- prefs.SetBoolean(kPrefsP2PEnabled, false);
- EXPECT_TRUE(manager->IsP2PEnabled());
-
- EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
-}
-
-// Check that IsP2PEnabled() corresponds to the value of the crosh
-// flag if Enterprise Policy indicates that p2p is not to be used.
-TEST_F(P2PManagerTest, P2PEnabledEnterpriseSettingFalse) {
- string temp_dir;
- Prefs prefs;
- EXPECT_TRUE(utils::MakeTempDirectory("PayloadStateP2PTests.XXXXXX",
- &temp_dir));
- prefs.Init(base::FilePath(temp_dir));
-
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, &prefs, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- unique_ptr<policy::MockDevicePolicy> device_policy(
- new policy::MockDevicePolicy());
- EXPECT_CALL(*device_policy, GetAuP2PEnabled(testing::_)).WillRepeatedly(
- DoAll(testing::SetArgumentPointee<0>(false),
- testing::Return(true)));
- manager->SetDevicePolicy(device_policy.get());
- EXPECT_FALSE(manager->IsP2PEnabled());
-
- prefs.SetBoolean(kPrefsP2PEnabled, true);
- EXPECT_TRUE(manager->IsP2PEnabled());
- prefs.SetBoolean(kPrefsP2PEnabled, false);
- EXPECT_FALSE(manager->IsP2PEnabled());
-
- EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
-}
-
-// Check that IsP2PEnabled() returns TRUE if
-// - The crosh flag is not set.
-// - Enterprise Policy is not set.
-// - Device is Enterprise Enrolled.
-TEST_F(P2PManagerTest, P2PEnabledEnterpriseEnrolledDevicesDefaultToEnabled) {
- string temp_dir;
- Prefs prefs;
- EXPECT_TRUE(utils::MakeTempDirectory("PayloadStateP2PTests.XXXXXX",
- &temp_dir));
- prefs.Init(base::FilePath(temp_dir));
-
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, &prefs, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- unique_ptr<policy::MockDevicePolicy> device_policy(
- new policy::MockDevicePolicy());
- // We return an empty owner as this is an enterprise.
- EXPECT_CALL(*device_policy, GetOwner(testing::_)).WillRepeatedly(
- DoAll(testing::SetArgumentPointee<0>(string("")),
- testing::Return(true)));
- manager->SetDevicePolicy(device_policy.get());
- EXPECT_TRUE(manager->IsP2PEnabled());
-
- EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
-}
-
-// Check that IsP2PEnabled() returns FALSE if
-// - The crosh flag is not set.
-// - Enterprise Policy is set to FALSE.
-// - Device is Enterprise Enrolled.
-TEST_F(P2PManagerTest, P2PEnabledEnterpriseEnrolledDevicesOverrideDefault) {
- string temp_dir;
- Prefs prefs;
- EXPECT_TRUE(utils::MakeTempDirectory("PayloadStateP2PTests.XXXXXX",
- &temp_dir));
- prefs.Init(base::FilePath(temp_dir));
-
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, &prefs, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- unique_ptr<policy::MockDevicePolicy> device_policy(
- new policy::MockDevicePolicy());
- // We return an empty owner as this is an enterprise.
- EXPECT_CALL(*device_policy, GetOwner(testing::_)).WillRepeatedly(
- DoAll(testing::SetArgumentPointee<0>(string("")),
- testing::Return(true)));
- // Make Enterprise Policy indicate p2p is not enabled.
- EXPECT_CALL(*device_policy, GetAuP2PEnabled(testing::_)).WillRepeatedly(
- DoAll(testing::SetArgumentPointee<0>(false),
- testing::Return(true)));
- manager->SetDevicePolicy(device_policy.get());
- EXPECT_FALSE(manager->IsP2PEnabled());
-
- EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir));
+ EXPECT_TRUE(manager_->IsP2PEnabled());
+ RunGMainLoopMaxIterations(100);
+ EXPECT_FALSE(manager_->IsP2PEnabled());
}
// Check that we keep the $N newest files with the .$EXT.p2p extension.
TEST_F(P2PManagerTest, HousekeepingCountLimit) {
- // Specifically pass 0 for |max_file_age| to allow files of any age.
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
+ // Specifically pass 0 for |max_file_age| to allow files of any age. Note that
+ // we need to reallocate the test_conf_ member, whose currently aliased object
+ // will be freed.
+ test_conf_ = new FakeP2PManagerConfiguration();
+ manager_.reset(P2PManager::Construct(
+ test_conf_, &fake_clock_, &fake_um_, "cros_au", 3,
base::TimeDelta() /* max_file_age */));
- EXPECT_EQ(manager->CountSharedFiles(), 0);
+ EXPECT_EQ(manager_->CountSharedFiles(), 0);
// Generate files with different timestamps matching our pattern and generate
// other files not matching the pattern.
@@ -245,9 +147,9 @@
g_usleep(1);
}
// CountSharedFiles() only counts 'cros_au' files.
- EXPECT_EQ(manager->CountSharedFiles(), 5);
+ EXPECT_EQ(manager_->CountSharedFiles(), 5);
- EXPECT_TRUE(manager->PerformHousekeeping());
+ EXPECT_TRUE(manager_->PerformHousekeeping());
// At this point - after HouseKeeping - we should only have
// eight files left.
@@ -267,7 +169,7 @@
EXPECT_TRUE(g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS));
}
// CountSharedFiles() only counts 'cros_au' files.
- EXPECT_EQ(manager->CountSharedFiles(), 3);
+ EXPECT_EQ(manager_->CountSharedFiles(), 3);
}
// Check that we keep files with the .$EXT.p2p extension not older
@@ -282,14 +184,16 @@
// Set the clock just so files with a timestamp before |cutoff_time|
// will be deleted at housekeeping.
- FakeClock fake_clock;
- fake_clock.SetWallclockTime(base::Time::FromTimeT(cutoff_time) + age_limit);
+ fake_clock_.SetWallclockTime(base::Time::FromTimeT(cutoff_time) + age_limit);
// Specifically pass 0 for |num_files_to_keep| to allow files of any age.
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, &fake_clock, "cros_au",
+ // Note that we need to reallocate the test_conf_ member, whose currently
+ // aliased object will be freed.
+ test_conf_ = new FakeP2PManagerConfiguration();
+ manager_.reset(P2PManager::Construct(
+ test_conf_, &fake_clock_, &fake_um_, "cros_au",
0 /* num_files_to_keep */, age_limit));
- EXPECT_EQ(manager->CountSharedFiles(), 0);
+ EXPECT_EQ(manager_->CountSharedFiles(), 0);
// Generate files with different timestamps matching our pattern and generate
// other files not matching the pattern.
@@ -332,9 +236,9 @@
test_conf_->GetP2PDir().value().c_str(), n)));
}
// CountSharedFiles() only counts 'cros_au' files.
- EXPECT_EQ(manager->CountSharedFiles(), 5);
+ EXPECT_EQ(manager_->CountSharedFiles(), 5);
- EXPECT_TRUE(manager->PerformHousekeeping());
+ EXPECT_TRUE(manager_->PerformHousekeeping());
// At this point - after HouseKeeping - we should only have
// eight files left.
@@ -354,7 +258,7 @@
EXPECT_TRUE(g_file_test(file_name.c_str(), G_FILE_TEST_EXISTS));
}
// CountSharedFiles() only counts 'cros_au' files.
- EXPECT_EQ(manager->CountSharedFiles(), 3);
+ EXPECT_EQ(manager_->CountSharedFiles(), 3);
}
static bool CheckP2PFile(const string& p2p_dir, const string& file_name,
@@ -448,20 +352,17 @@
return;
}
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
- EXPECT_TRUE(manager->FileShare("foo", 10 * 1000 * 1000));
- EXPECT_EQ(manager->FileGetPath("foo"),
+ EXPECT_TRUE(manager_->FileShare("foo", 10 * 1000 * 1000));
+ EXPECT_EQ(manager_->FileGetPath("foo"),
test_conf_->GetP2PDir().Append("foo.cros_au.p2p.tmp"));
EXPECT_TRUE(CheckP2PFile(test_conf_->GetP2PDir().value(),
"foo.cros_au.p2p.tmp", 0, 10 * 1000 * 1000));
// Sharing it again - with the same expected size - should return true
- EXPECT_TRUE(manager->FileShare("foo", 10 * 1000 * 1000));
+ EXPECT_TRUE(manager_->FileShare("foo", 10 * 1000 * 1000));
// ... but if we use the wrong size, it should fail
- EXPECT_FALSE(manager->FileShare("foo", 10 * 1000 * 1000 + 1));
+ EXPECT_FALSE(manager_->FileShare("foo", 10 * 1000 * 1000 + 1));
}
// Check that making a shared file visible, does what is expected.
@@ -472,20 +373,17 @@
return;
}
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
// First, check that it's not visible.
- manager->FileShare("foo", 10*1000*1000);
- EXPECT_EQ(manager->FileGetPath("foo"),
+ manager_->FileShare("foo", 10*1000*1000);
+ EXPECT_EQ(manager_->FileGetPath("foo"),
test_conf_->GetP2PDir().Append("foo.cros_au.p2p.tmp"));
EXPECT_TRUE(CheckP2PFile(test_conf_->GetP2PDir().value(),
"foo.cros_au.p2p.tmp", 0, 10 * 1000 * 1000));
// Make the file visible and check that it changed its name. Do it
// twice to check that FileMakeVisible() is idempotent.
for (int n = 0; n < 2; n++) {
- manager->FileMakeVisible("foo");
- EXPECT_EQ(manager->FileGetPath("foo"),
+ manager_->FileMakeVisible("foo");
+ EXPECT_EQ(manager_->FileGetPath("foo"),
test_conf_->GetP2PDir().Append("foo.cros_au.p2p"));
EXPECT_TRUE(CheckP2PFile(test_conf_->GetP2PDir().value(),
"foo.cros_au.p2p", 0, 10 * 1000 * 1000));
@@ -500,41 +398,38 @@
return;
}
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
bool visible;
// Check that errors are returned if the file does not exist
- EXPECT_EQ(manager->FileGetPath("foo"), base::FilePath());
- EXPECT_EQ(manager->FileGetSize("foo"), -1);
- EXPECT_EQ(manager->FileGetExpectedSize("foo"), -1);
- EXPECT_FALSE(manager->FileGetVisible("foo", nullptr));
+ EXPECT_EQ(manager_->FileGetPath("foo"), base::FilePath());
+ EXPECT_EQ(manager_->FileGetSize("foo"), -1);
+ EXPECT_EQ(manager_->FileGetExpectedSize("foo"), -1);
+ EXPECT_FALSE(manager_->FileGetVisible("foo", nullptr));
// ... then create the file ...
EXPECT_TRUE(CreateP2PFile(test_conf_->GetP2PDir().value(),
"foo.cros_au.p2p", 42, 43));
// ... and then check that the expected values are returned
- EXPECT_EQ(manager->FileGetPath("foo"),
+ EXPECT_EQ(manager_->FileGetPath("foo"),
test_conf_->GetP2PDir().Append("foo.cros_au.p2p"));
- EXPECT_EQ(manager->FileGetSize("foo"), 42);
- EXPECT_EQ(manager->FileGetExpectedSize("foo"), 43);
- EXPECT_TRUE(manager->FileGetVisible("foo", &visible));
+ EXPECT_EQ(manager_->FileGetSize("foo"), 42);
+ EXPECT_EQ(manager_->FileGetExpectedSize("foo"), 43);
+ EXPECT_TRUE(manager_->FileGetVisible("foo", &visible));
EXPECT_TRUE(visible);
// One more time, this time with a .tmp variant. First ensure it errors out..
- EXPECT_EQ(manager->FileGetPath("bar"), base::FilePath());
- EXPECT_EQ(manager->FileGetSize("bar"), -1);
- EXPECT_EQ(manager->FileGetExpectedSize("bar"), -1);
- EXPECT_FALSE(manager->FileGetVisible("bar", nullptr));
+ EXPECT_EQ(manager_->FileGetPath("bar"), base::FilePath());
+ EXPECT_EQ(manager_->FileGetSize("bar"), -1);
+ EXPECT_EQ(manager_->FileGetExpectedSize("bar"), -1);
+ EXPECT_FALSE(manager_->FileGetVisible("bar", nullptr));
// ... then create the file ...
EXPECT_TRUE(CreateP2PFile(test_conf_->GetP2PDir().value(),
"bar.cros_au.p2p.tmp", 44, 45));
// ... and then check that the expected values are returned
- EXPECT_EQ(manager->FileGetPath("bar"),
+ EXPECT_EQ(manager_->FileGetPath("bar"),
test_conf_->GetP2PDir().Append("bar.cros_au.p2p.tmp"));
- EXPECT_EQ(manager->FileGetSize("bar"), 44);
- EXPECT_EQ(manager->FileGetExpectedSize("bar"), 45);
- EXPECT_TRUE(manager->FileGetVisible("bar", &visible));
+ EXPECT_EQ(manager_->FileGetSize("bar"), 44);
+ EXPECT_EQ(manager_->FileGetExpectedSize("bar"), 45);
+ EXPECT_TRUE(manager_->FileGetVisible("bar", &visible));
EXPECT_FALSE(visible);
}
@@ -542,40 +437,32 @@
// will have to do. E.g. we essentially simulate the various
// behaviours of initctl(8) that we rely on.
TEST_F(P2PManagerTest, StartP2P) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
-
// Check that we can start the service
test_conf_->SetInitctlStartCommandLine("true");
- EXPECT_TRUE(manager->EnsureP2PRunning());
+ EXPECT_TRUE(manager_->EnsureP2PRunning());
test_conf_->SetInitctlStartCommandLine("false");
- EXPECT_FALSE(manager->EnsureP2PRunning());
+ EXPECT_FALSE(manager_->EnsureP2PRunning());
test_conf_->SetInitctlStartCommandLine(
"sh -c 'echo \"initctl: Job is already running: p2p\" >&2; false'");
- EXPECT_TRUE(manager->EnsureP2PRunning());
+ EXPECT_TRUE(manager_->EnsureP2PRunning());
test_conf_->SetInitctlStartCommandLine(
"sh -c 'echo something else >&2; false'");
- EXPECT_FALSE(manager->EnsureP2PRunning());
+ EXPECT_FALSE(manager_->EnsureP2PRunning());
}
// Same comment as for StartP2P
TEST_F(P2PManagerTest, StopP2P) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
-
// Check that we can start the service
test_conf_->SetInitctlStopCommandLine("true");
- EXPECT_TRUE(manager->EnsureP2PNotRunning());
+ EXPECT_TRUE(manager_->EnsureP2PNotRunning());
test_conf_->SetInitctlStopCommandLine("false");
- EXPECT_FALSE(manager->EnsureP2PNotRunning());
+ EXPECT_FALSE(manager_->EnsureP2PNotRunning());
test_conf_->SetInitctlStopCommandLine(
"sh -c 'echo \"initctl: Unknown instance \" >&2; false'");
- EXPECT_TRUE(manager->EnsureP2PNotRunning());
+ EXPECT_TRUE(manager_->EnsureP2PNotRunning());
test_conf_->SetInitctlStopCommandLine(
"sh -c 'echo something else >&2; false'");
- EXPECT_FALSE(manager->EnsureP2PNotRunning());
+ EXPECT_FALSE(manager_->EnsureP2PNotRunning());
}
static void ExpectUrl(const string& expected_url,
@@ -588,49 +475,47 @@
// Like StartP2P, we're mocking the different results that p2p-client
// can return. It's not pretty but it works.
TEST_F(P2PManagerTest, LookupURL) {
- unique_ptr<P2PManager> manager(P2PManager::Construct(
- test_conf_, nullptr, nullptr, "cros_au", 3,
- base::TimeDelta::FromDays(5)));
GMainLoop *loop = g_main_loop_new(nullptr, FALSE);
// Emulate p2p-client returning valid URL with "fooX", 42 and "cros_au"
// being propagated in the right places.
test_conf_->SetP2PClientCommandLine(
"echo 'http://1.2.3.4/{file_id}_{minsize}'");
- manager->LookupUrlForFile("fooX", 42, TimeDelta(),
- base::Bind(ExpectUrl,
- "http://1.2.3.4/fooX.cros_au_42", loop));
+ manager_->LookupUrlForFile("fooX", 42, TimeDelta(),
+ base::Bind(ExpectUrl,
+ "http://1.2.3.4/fooX.cros_au_42",
+ loop));
g_main_loop_run(loop);
// Emulate p2p-client returning invalid URL.
test_conf_->SetP2PClientCommandLine("echo 'not_a_valid_url'");
- manager->LookupUrlForFile("foobar", 42, TimeDelta(),
- base::Bind(ExpectUrl, "", loop));
+ manager_->LookupUrlForFile("foobar", 42, TimeDelta(),
+ base::Bind(ExpectUrl, "", loop));
g_main_loop_run(loop);
// Emulate p2p-client conveying failure.
test_conf_->SetP2PClientCommandLine("false");
- manager->LookupUrlForFile("foobar", 42, TimeDelta(),
- base::Bind(ExpectUrl, "", loop));
+ manager_->LookupUrlForFile("foobar", 42, TimeDelta(),
+ base::Bind(ExpectUrl, "", loop));
g_main_loop_run(loop);
// Emulate p2p-client not existing.
test_conf_->SetP2PClientCommandLine("/path/to/non/existent/helper/program");
- manager->LookupUrlForFile("foobar", 42,
- TimeDelta(),
- base::Bind(ExpectUrl, "", loop));
+ manager_->LookupUrlForFile("foobar", 42,
+ TimeDelta(),
+ base::Bind(ExpectUrl, "", loop));
g_main_loop_run(loop);
// Emulate p2p-client crashing.
test_conf_->SetP2PClientCommandLine("sh -c 'kill -SEGV $$'");
- manager->LookupUrlForFile("foobar", 42, TimeDelta(),
- base::Bind(ExpectUrl, "", loop));
+ manager_->LookupUrlForFile("foobar", 42, TimeDelta(),
+ base::Bind(ExpectUrl, "", loop));
g_main_loop_run(loop);
// Emulate p2p-client exceeding its timeout.
test_conf_->SetP2PClientCommandLine("sh -c 'echo http://1.2.3.4/; sleep 2'");
- manager->LookupUrlForFile("foobar", 42, TimeDelta::FromMilliseconds(500),
- base::Bind(ExpectUrl, "", loop));
+ manager_->LookupUrlForFile("foobar", 42, TimeDelta::FromMilliseconds(500),
+ base::Bind(ExpectUrl, "", loop));
g_main_loop_run(loop);
g_main_loop_unref(loop);
diff --git a/real_system_state.cc b/real_system_state.cc
index d61e8c3..f16f2f8 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -41,10 +41,6 @@
system_rebooted_ = true;
}
- p2p_manager_.reset(P2PManager::Construct(
- nullptr, &prefs_, &clock_, "cros_au", kMaxP2PFilesToKeep,
- base::TimeDelta::FromDays(kMaxP2PFileAgeDays)));
-
// Initialize the Update Manager using the default state factory.
chromeos_update_manager::State* um_state =
chromeos_update_manager::DefaultStateFactory(
@@ -58,6 +54,11 @@
&clock_, base::TimeDelta::FromSeconds(5),
base::TimeDelta::FromHours(12), um_state));
+ // The P2P Manager depends on the Update Manager for its initialization.
+ p2p_manager_.reset(P2PManager::Construct(
+ nullptr, &clock_, update_manager_.get(), "cros_au",
+ kMaxP2PFilesToKeep, base::TimeDelta::FromDays(kMaxP2PFileAgeDays)));
+
if (!payload_state_.Initialize(this)) {
LOG(ERROR) << "Failed to initialize the payload state object.";
return false;