diff options
-rw-r--r-- | dbus_service.cc | 6 | ||||
-rw-r--r-- | download_action_unittest.cc | 8 | ||||
-rw-r--r-- | fake_p2p_manager.h | 12 | ||||
-rw-r--r-- | mock_p2p_manager.h | 4 | ||||
-rw-r--r-- | p2p_manager.cc | 142 | ||||
-rw-r--r-- | p2p_manager.h | 25 | ||||
-rw-r--r-- | p2p_manager_unittest.cc | 339 | ||||
-rw-r--r-- | real_system_state.cc | 9 |
8 files changed, 214 insertions, 331 deletions
diff --git a/dbus_service.cc b/dbus_service.cc index 0c393404..ba995868 100644 --- a/dbus_service.cc +++ b/dbus_service.cc @@ -355,11 +355,9 @@ gboolean update_engine_service_set_p2p_update_permission( 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 78e3f5cb..d27f9636 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 @@ class P2PDownloadActionTest : public testing::Test { 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 @@ class P2PDownloadActionTest : public testing::Test { // 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 @@ class P2PDownloadActionTest : public testing::Test { // 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 9ec0812a..a00b91f0 100644 --- a/fake_p2p_manager.h +++ b/fake_p2p_manager.h @@ -19,8 +19,7 @@ class FakeP2PManager : public P2PManager { 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 @@ class FakeP2PManager : public P2PManager { 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 @@ class FakeP2PManager : public P2PManager { 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 @@ class FakeP2PManager : public P2PManager { 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 64c9c828..a0ed920b 100644 --- a/mock_p2p_manager.h +++ b/mock_p2p_manager.h @@ -58,9 +58,6 @@ class MockP2PManager : public P2PManager { 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 @@ class MockP2PManager : public P2PManager { 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 abfb3d7d..95623e91 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 ConfigurationImpl : public P2PManager::Configuration { 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 @@ class P2PManagerImpl : public P2PManager { 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 @@ class P2PManagerImpl : public P2PManager { // 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 @@ class P2PManagerImpl : public P2PManager { // 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::kP2PExtension[] = ".p2p"; 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 @@ void P2PManagerImpl::SetDevicePolicy( } 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 @@ int P2PManagerImpl::CountSharedFiles() { 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(); + 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; +} - return true; +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, - PrefsInterface *prefs, - ClockInterface *clock, - const string& file_extension, - const int num_files_to_keep, - const base::TimeDelta& max_file_age) { +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 4c6e508c..fb07c2e0 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 @@ class P2PManager { // 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 @@ class P2PManager { // 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 @@ class P2PManager { // 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 fe53955a..a790289a 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 @@ namespace chromeos_update_engine { // 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_; -}; - - -// 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)); - unique_ptr<P2PManager> manager(P2PManager::Construct( - test_conf_, &prefs, nullptr, "cros_au", 3, - base::TimeDelta::FromDays(5))); - EXPECT_FALSE(manager->IsP2PEnabled()); + FakeClock fake_clock_; + chromeos_update_manager::MockPolicy *mock_policy_ = nullptr; + chromeos_update_manager::FakeUpdateManager fake_um_; - EXPECT_TRUE(utils::RecursiveUnlinkDir(temp_dir)); -} - -// 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)); - - 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)); -} + unique_ptr<P2PManager> manager_; +}; -// 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() 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)); -// 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)); + EXPECT_FALSE(manager_->IsP2PEnabled()); + RunGMainLoopMaxIterations(100); + EXPECT_FALSE(manager_->IsP2PEnabled()); } -// 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)); +// 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)); + + 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 @@ TEST_F(P2PManagerTest, HousekeepingCountLimit) { 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 @@ TEST_F(P2PManagerTest, HousekeepingCountLimit) { 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 @@ TEST_F(P2PManagerTest, HousekeepingAgeLimit) { // 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_F(P2PManagerTest, HousekeepingAgeLimit) { 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 @@ TEST_F(P2PManagerTest, HousekeepingAgeLimit) { 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 @@ TEST_F(P2PManagerTest, ShareFile) { 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 @@ TEST_F(P2PManagerTest, MakeFileVisible) { 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 @@ TEST_F(P2PManagerTest, ExistingFiles) { 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 @@ TEST_F(P2PManagerTest, ExistingFiles) { // 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 @@ static void ExpectUrl(const string& expected_url, // 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 d61e8c38..f16f2f8f 100644 --- a/real_system_state.cc +++ b/real_system_state.cc @@ -41,10 +41,6 @@ bool RealSystemState::Initialize() { 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 @@ bool RealSystemState::Initialize() { &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; |