diff options
8 files changed, 249 insertions, 303 deletions
diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider.cc b/chrome/browser/extensions/api/system_info_storage/storage_info_provider.cc index ce65d14..4aac324 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider.cc +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider.cc @@ -1,12 +1,16 @@ // Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. +// // Use of this source code is governed by a BSD-style license that can be +// // found in the LICENSE file. #include "chrome/browser/extensions/api/system_info_storage/storage_info_provider.h" #include "base/stl_util.h" -#include "base/threading/sequenced_worker_pool.h" +#include "chrome/common/chrome_notification_types.h" #include "content/public/browser/browser_thread.h" +#include "content/public/browser/notification_details.h" +#include "content/public/browser/notification_service.h" +#include "content/public/browser/notification_source.h" +#include "content/public/browser/notification_types.h" namespace extensions { @@ -22,15 +26,19 @@ const char kStorageTypeRemovable[] = "removable"; } // namespace systeminfo // Default watching interval is 1000 ms. -const int kDefaultPollingIntervalMs = 1000; -const char kWatchingTokenName[] = "_storage_info_watching_token_"; +const unsigned int kDefaultPollingIntervalMs = 1000; StorageInfoProvider::StorageInfoProvider() - : observers_(new ObserverListThreadSafe<Observer>()), + : watching_timer_(NULL), + observers_(new ObserverListThreadSafe<Observer>()), watching_interval_(kDefaultPollingIntervalMs) { + registrar_.Add(this, chrome::NOTIFICATION_APP_TERMINATING, + content::NotificationService::AllSources()); } StorageInfoProvider::~StorageInfoProvider() { + DCHECK(watching_timer_ == NULL); + registrar_.RemoveAll(); } void StorageInfoProvider::AddObserver(Observer* obs) { @@ -42,24 +50,40 @@ void StorageInfoProvider::RemoveObserver(Observer* obs) { } void StorageInfoProvider::StartWatching(const std::string& id) { - BrowserThread::PostBlockingPoolSequencedTask( - kWatchingTokenName, + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, - base::Bind(&StorageInfoProvider::AddWatchedStorageOnBlockingPool, - this, id)); + base::Bind(&StorageInfoProvider::StartWatchingOnFileThread, + base::Unretained(this), id)); } void StorageInfoProvider::StopWatching(const std::string& id) { - BrowserThread::PostBlockingPoolSequencedTask( - kWatchingTokenName, + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, - base::Bind(&StorageInfoProvider::RemoveWatchedStorageOnBlockingPool, - this, id)); + base::Bind(&StorageInfoProvider::StopWatchingOnFileThread, + base::Unretained(this), id)); } -void StorageInfoProvider::AddWatchedStorageOnBlockingPool( - const std::string& id) { - DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread()); +void StorageInfoProvider::Observe( + int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) { + switch (type) { + case chrome::NOTIFICATION_APP_TERMINATING: + BrowserThread::PostTask(BrowserThread::FILE, + FROM_HERE, + base::Bind(&StorageInfoProvider::DestroyWatchingTimer, + base::Unretained(this))); + break; + default: + NOTREACHED(); + break; + } +} + +void StorageInfoProvider::StartWatchingOnFileThread(const std::string& id) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); // If the storage |id| is already being watched. if (ContainsKey(storage_id_to_size_map_, id)) return; @@ -73,35 +97,29 @@ void StorageInfoProvider::AddWatchedStorageOnBlockingPool( // If it is the first storage to be watched, we need to start the watching // timer. if (storage_id_to_size_map_.size() == 1) { - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&StorageInfoProvider::StartWatchingTimerOnUIThread, this)); + watching_timer_ = new base::RepeatingTimer<StorageInfoProvider>(); + watching_timer_->Start(FROM_HERE, + base::TimeDelta::FromMilliseconds(watching_interval_), + this, &StorageInfoProvider::CheckWatchedStorages); } } -void StorageInfoProvider::RemoveWatchedStorageOnBlockingPool( - const std::string& id) { - DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread()); +void StorageInfoProvider::StopWatchingOnFileThread(const std::string& id) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); if (!ContainsKey(storage_id_to_size_map_, id)) return; storage_id_to_size_map_.erase(id); - // Stop watching timer if there is no storage to be watched. - if (storage_id_to_size_map_.empty()) { - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&StorageInfoProvider::StopWatchingTimerOnUIThread, this)); - } + // If there is no storage to be watched, we need to destroy the watching + // timer. + if (storage_id_to_size_map_.size() == 0) + DestroyWatchingTimer(); } void StorageInfoProvider::CheckWatchedStorages() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - BrowserThread::PostBlockingPoolSequencedTask(kWatchingTokenName, FROM_HERE, - base::Bind(&StorageInfoProvider::CheckWatchedStoragesOnBlockingPool, - this)); -} - -void StorageInfoProvider::CheckWatchedStoragesOnBlockingPool() { - DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread()); + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + DCHECK(storage_id_to_size_map_.size() > 0); StorageIDToSizeMap::iterator it = storage_id_to_size_map_.begin(); for (; it != storage_id_to_size_map_.end(); ++it) { @@ -109,9 +127,7 @@ void StorageInfoProvider::CheckWatchedStoragesOnBlockingPool() { if (!QueryUnitInfo(it->first, &info)) { storage_id_to_size_map_.erase(it); if (storage_id_to_size_map_.size() == 0) { - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - base::Bind(&StorageInfoProvider::StopWatchingTimerOnUIThread, - this)); + DestroyWatchingTimer(); return; } } @@ -123,23 +139,12 @@ void StorageInfoProvider::CheckWatchedStoragesOnBlockingPool() { it->second = info.available_capacity; } } - - OnCheckWatchedStoragesFinishedForTesting(); -} - -void StorageInfoProvider::StartWatchingTimerOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - // Start the watching timer if it is not running. - if (!watching_timer_.IsRunning()) { - watching_timer_.Start(FROM_HERE, - base::TimeDelta::FromMilliseconds(watching_interval_), - this, &StorageInfoProvider::CheckWatchedStorages); - } } -void StorageInfoProvider::StopWatchingTimerOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - watching_timer_.Stop(); +void StorageInfoProvider::DestroyWatchingTimer() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); + delete watching_timer_; + watching_timer_ = NULL; } } // namespace extensions diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider.h b/chrome/browser/extensions/api/system_info_storage/storage_info_provider.h index 1bb4dcb..4f1b0ef 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider.h +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider.h @@ -34,19 +34,21 @@ typedef std::vector<linked_ptr< api::experimental_system_info_storage::StorageUnitInfo> > StorageInfo; class StorageInfoProvider - : public SystemInfoProvider<StorageInfo>, - public base::RefCountedThreadSafe<StorageInfoProvider> { + : public content::NotificationObserver, + public SystemInfoProvider<StorageInfo> { public: class Observer { public: virtual ~Observer() {} - // Called when the storage free space changes. + // Called from FILE thread when the storage free space changes. virtual void OnStorageFreeSpaceChanged(const std::string& id, double old_value, double new_value) = 0; }; + virtual ~StorageInfoProvider(); + // Get the single shared instance of StorageInfoProvider. static StorageInfoProvider* Get(); @@ -59,7 +61,7 @@ class StorageInfoProvider virtual void StopWatching(const std::string& id); // Set the watching time interval. - void set_watching_interval(size_t ms) { watching_interval_ = ms; } + void set_watching_interval(unsigned int ms) { watching_interval_ = ms; } // Get the information for the storage unit specified by the |id| parameter, // and output the result to the |info|. @@ -68,47 +70,45 @@ class StorageInfoProvider protected: StorageInfoProvider(); - virtual ~StorageInfoProvider(); private: - friend class TestStorageInfoProvider; - friend class base::RefCountedThreadSafe<StorageInfoProvider>; typedef std::map<std::string, double> StorageIDToSizeMap; - // Posts a task to check for free space changes on the blocking pool. - // Should be called on the UI thread. - void CheckWatchedStorages(); + // content::NotificationObserver implementation. + virtual void Observe(int type, + const content::NotificationSource& source, + const content::NotificationDetails& details) OVERRIDE; - // Check if the free space changes for the watched storages by iterating over - // the |storage_id_to_size_map_|. It is called on blocking pool. - void CheckWatchedStoragesOnBlockingPool(); + // Start watching the given storage |id| on FILE thread. + void StartWatchingOnFileThread(const std::string& id); - // Provides an explicit hook that tests can wait for to determine that the - // call to CheckWatchedStorages() has completed. - virtual void OnCheckWatchedStoragesFinishedForTesting() {} + // Stop the watching the the given storage |id| on FILE thread. + void StopWatchingOnFileThread(const std::string& id); - // Add and remove the storage to be watched. - void AddWatchedStorageOnBlockingPool(const std::string& id); - void RemoveWatchedStorageOnBlockingPool(const std::string& id); + // Check if the free space changes for the watched storages by iterating over + // the |storage_id_to_size_map_|. It is called on FILE thread. + void CheckWatchedStorages(); - void StartWatchingTimerOnUIThread(); - // Force to stop the watching timer or there is no any one storage to be - // watched. It is called on UI thread. - void StopWatchingTimerOnUIThread(); + // Force to destory the watching timer if the application is terminating or + // there is no any one storage to be watched. It is called on FILE thread. + void DestroyWatchingTimer(); - // Mapping of the storage being watched and the recent free space value. It - // is maintained on the blocking pool. + // Mapping of the storage being watched and the recent free space value. StorageIDToSizeMap storage_id_to_size_map_; // The timer used for watching the storage free space changes periodically. - base::RepeatingTimer<StorageInfoProvider> watching_timer_; + // It lives on the FILE thread. + base::RepeatingTimer<StorageInfoProvider>* watching_timer_; // The thread-safe observer list that observe the changes happening on the // storages. scoped_refptr<ObserverListThreadSafe<Observer> > observers_; // The time interval for watching the free space change, in milliseconds. - size_t watching_interval_; + unsigned int watching_interval_; + + // Used to observe app termination notification. + content::NotificationRegistrar registrar_; }; } // namespace extensions diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux.h b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux.h index da9cecb..191c0e4 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux.h +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux.h @@ -15,9 +15,9 @@ class StorageInfoProviderLinux : public StorageInfoProvider { public: StorageInfoProviderLinux(); - protected: virtual ~StorageInfoProviderLinux(); + protected: // For unit test. explicit StorageInfoProviderLinux(const FilePath& mtab_path); diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux_unittest.cc b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux_unittest.cc index 2547d98..71c3ba5 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux_unittest.cc +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_linux_unittest.cc @@ -61,8 +61,6 @@ class StorageInfoProviderLinuxWrapper : public StorageInfoProviderLinux { private: friend class StorageInfoProviderLinuxTest; - virtual ~StorageInfoProviderLinuxWrapper() {} - virtual bool QueryUnitInfo(const std::string& mount_path, StorageUnitInfo* info) OVERRIDE { std::string type; @@ -102,14 +100,15 @@ class StorageInfoProviderLinuxTest : public testing::Test { int bytes = file_util::WriteFile(mtab_file_, mtab_test_data, strlen(mtab_test_data)); ASSERT_EQ(static_cast<int>(strlen(mtab_test_data)), bytes); - storage_info_provider_ = new StorageInfoProviderLinuxWrapper(mtab_file_); + storage_info_provider_.reset( + new StorageInfoProviderLinuxWrapper(mtab_file_)); } virtual void TearDown() OVERRIDE { file_util::Delete(mtab_file_, false); } - scoped_refptr<StorageInfoProviderLinuxWrapper> storage_info_provider_; + scoped_ptr<StorageInfoProviderLinuxWrapper> storage_info_provider_; FilePath mtab_file_; }; @@ -131,8 +130,8 @@ TEST_F(StorageInfoProviderLinuxTest, QueryInfo) { } TEST_F(StorageInfoProviderLinuxTest, QueryInfoFailed) { - storage_info_provider_ = - new StorageInfoProviderLinuxWrapper(FilePath("/invalid/file/path")); + storage_info_provider_.reset( + new StorageInfoProviderLinuxWrapper(FilePath("/invalid/file/path"))); StorageInfo info; ASSERT_FALSE(QueryInfo(&info)); EXPECT_EQ(0u, info.size()); diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_mac.cc b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_mac.cc index 5a17653..2ae4589 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_mac.cc +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_mac.cc @@ -31,13 +31,13 @@ using api::experimental_system_info_storage::StorageUnitInfo; class StorageInfoProviderMac : public StorageInfoProvider { public: StorageInfoProviderMac() {} + virtual ~StorageInfoProviderMac() {} virtual bool QueryInfo(StorageInfo* info) OVERRIDE; virtual bool QueryUnitInfo(const std::string& id, StorageUnitInfo* info) OVERRIDE; private: - virtual ~StorageInfoProviderMac() {} void BuildStorageTypeMap(); std::map<std::string, std::string> dev_path_to_type_map_; }; diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc index a1b77c5f..a711169 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_unittest.cc @@ -5,10 +5,9 @@ // StorageInfoProvider unit tests. #include "base/message_loop.h" -#include "base/stl_util.h" +#include "base/run_loop.h" #include "chrome/browser/extensions/api/system_info_storage/storage_info_provider.h" #include "content/public/test/test_browser_thread.h" -#include "content/public/test/test_utils.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,8 +17,7 @@ using api::experimental_system_info_storage::FromStorageUnitTypeString; using api::experimental_system_info_storage::StorageUnitInfo; using api::experimental_system_info_storage::StorageUnitType; using content::BrowserThread; -using content::RunAllPendingInMessageLoop; -using content::RunMessageLoop; +using testing::AnyNumber; using testing::Return; using testing::_; @@ -32,7 +30,7 @@ struct TestUnitInfo { int change_step; }; -const struct TestUnitInfo kTestingData[] = { +struct TestUnitInfo testing_data[] = { {"C:", "unknown", 1000, 10, 0}, {"d:", "removable", 2000, 10, 1 }, {"/home","harddisk", 3000, 10, 2}, @@ -40,9 +38,8 @@ const struct TestUnitInfo kTestingData[] = { }; // The watching interval for unit test is 1 milliseconds. -const size_t kWatchingIntervalMs = 1u; -// The number of times of checking watched storages. -const int kCheckTimes = 10; +const unsigned int kWatchingIntervalMs = 1u; +const int kCallTimes = 3; class MockStorageObserver : public StorageInfoProvider::Observer { public: @@ -56,145 +53,69 @@ class MockStorageObserver : public StorageInfoProvider::Observer { DISALLOW_COPY_AND_ASSIGN(MockStorageObserver); }; -// A testing observer used to provide the statistics of how many times -// that the storage free space has been changed and check the change against -// our expectation. -class TestStorageObserver : public StorageInfoProvider::Observer { - public: - TestStorageObserver() { - for (size_t i = 0; i < arraysize(kTestingData); ++i) - testing_data_.push_back(kTestingData[i]); - } - - virtual ~TestStorageObserver() {} - - virtual void OnStorageFreeSpaceChanged(const std::string& id, - double old_value, - double new_value) OVERRIDE { - // The observer is added on UI thread, so the callback should be also - // called on UI thread. - ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI)); - for (size_t i = 0; i < testing_data_.size(); ++i) { - if (testing_data_[i].id == id) { - EXPECT_EQ(old_value, testing_data_[i].available_capacity); - EXPECT_EQ(new_value, testing_data_[i].available_capacity + - testing_data_[i].change_step); - // Increase the available capacity with the change step for comparison - // next time. - testing_data_[i].available_capacity += testing_data_[i].change_step; - ++free_space_change_times_[id]; - return; - } - } - EXPECT_TRUE(false); - } - - // Returns the number of change times for the given storage |id|. - int GetFreeSpaceChangeTimes(const std::string& id) { - if (ContainsKey(free_space_change_times_, id)) - return free_space_change_times_[id]; - return 0; - } - - private: - // A copy of |kTestingData|. - std::vector<TestUnitInfo> testing_data_; - // Mapping of storage id and the times of free space has been changed. - std::map<std::string, int> free_space_change_times_; -}; - class TestStorageInfoProvider : public StorageInfoProvider { public: - TestStorageInfoProvider(); + TestStorageInfoProvider() {} + virtual ~TestStorageInfoProvider() {} private: - virtual ~TestStorageInfoProvider(); + virtual bool QueryInfo(StorageInfo* info) OVERRIDE { + info->clear(); - virtual bool QueryInfo(StorageInfo* info) OVERRIDE; - virtual bool QueryUnitInfo(const std::string& id, - StorageUnitInfo* info) OVERRIDE; - - // Called each time CheckWatchedStoragesOnBlockingPool is finished. - void OnCheckWatchedStoragesFinishedForTesting(); - - // The testing data maintained on the blocking pool. - std::vector<TestUnitInfo> testing_data_; - // The number of times for checking watched storage. - int checking_times_; -}; - -// -// TestStorageInfoProvider Impl. -// -TestStorageInfoProvider::TestStorageInfoProvider(): checking_times_(0) { - for (size_t i = 0; i < arraysize(kTestingData); ++i) - testing_data_.push_back(kTestingData[i]); -} - -TestStorageInfoProvider::~TestStorageInfoProvider() {} - -bool TestStorageInfoProvider::QueryInfo(StorageInfo* info) { - info->clear(); - - for (size_t i = 0; i < testing_data_.size(); ++i) { - linked_ptr<StorageUnitInfo> unit(new StorageUnitInfo()); - QueryUnitInfo(testing_data_[i].id, unit.get()); - info->push_back(unit); + for (size_t i = 0; i < arraysize(testing_data); i++) { + linked_ptr<StorageUnitInfo> unit(new StorageUnitInfo()); + QueryUnitInfo(testing_data[i].id, unit.get()); + info->push_back(unit); + } + return true; } - return true; -} -bool TestStorageInfoProvider::QueryUnitInfo( - const std::string& id, StorageUnitInfo* info) { - for (size_t i = 0; i < testing_data_.size(); ++i) { - if (testing_data_[i].id == id) { - info->id = testing_data_[i].id; - info->type = FromStorageUnitTypeString(testing_data_[i].type); - info->capacity = testing_data_[i].capacity; - info->available_capacity = testing_data_[i].available_capacity; - // Increase the available capacity with a fixed change step. - testing_data_[i].available_capacity += testing_data_[i].change_step; - return true; + virtual bool QueryUnitInfo(const std::string& id, + StorageUnitInfo* info) OVERRIDE { + for (size_t i = 0; i < arraysize(testing_data); i++) { + if (testing_data[i].id == id) { + info->id = testing_data[i].id; + info->type = FromStorageUnitTypeString(testing_data[i].type); + info->capacity = testing_data[i].capacity; + info->available_capacity = testing_data[i].available_capacity; + // Increase the available capacity with a fix change step. + testing_data[i].available_capacity += testing_data[i].change_step; + return true; + } } + return false; } - return false; -} - -void TestStorageInfoProvider::OnCheckWatchedStoragesFinishedForTesting() { - ++checking_times_; - if (checking_times_ < kCheckTimes) - return; - checking_times_ = 0; - // Once the number of checking times reaches up to kCheckTimes, we need to - // quit the message loop to given UI thread a chance to verify the results. - // Note the QuitClosure is actually bound to QuitCurrentWhenIdle, it means - // that the UI thread wil continue to process pending messages util idle. - BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, - MessageLoop::QuitClosure()); -} +}; class StorageInfoProviderTest : public testing::Test { public: StorageInfoProviderTest(); virtual ~StorageInfoProviderTest(); + MockStorageObserver& observer() { return observer_; } + protected: virtual void SetUp() OVERRIDE; virtual void TearDown() OVERRIDE; - // Run message loop and flush blocking pool to make sure there is no pending - // tasks on blocking pool. - static void RunLoopAndFlushBlockingPool(); - static void RunAllPendingAndFlushBlockingPool(); + // Run message loop until the given |ms| milliseconds has passed. + void RunMessageLoopUntilTimeout(int ms); + + void RunMessageLoopUntilIdle(); + + // Reset the testing data once a round of querying operation is completed. + void ResetTestingData(); MessageLoop message_loop_; content::TestBrowserThread ui_thread_; - scoped_refptr<TestStorageInfoProvider> storage_info_provider_; + content::TestBrowserThread file_thread_; + MockStorageObserver observer_; }; StorageInfoProviderTest::StorageInfoProviderTest() : message_loop_(MessageLoop::TYPE_UI), - ui_thread_(BrowserThread::UI, &message_loop_) { + ui_thread_(BrowserThread::UI, &message_loop_), + file_thread_(BrowserThread::FILE, &message_loop_) { } StorageInfoProviderTest::~StorageInfoProviderTest() { @@ -202,127 +123,151 @@ StorageInfoProviderTest::~StorageInfoProviderTest() { void StorageInfoProviderTest::SetUp() { ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI)); - storage_info_provider_= new TestStorageInfoProvider(); - storage_info_provider_->set_watching_interval(kWatchingIntervalMs); + TestStorageInfoProvider* provider = new TestStorageInfoProvider(); + provider->set_watching_interval(kWatchingIntervalMs); + // Now the provider is owned by the singleton instance. + StorageInfoProvider::InitializeForTesting(provider); + StorageInfoProvider::Get()->AddObserver(&observer_); } void StorageInfoProviderTest::TearDown() { - RunAllPendingAndFlushBlockingPool(); + StorageInfoProvider::Get()->RemoveObserver(&observer_); } -void StorageInfoProviderTest::RunLoopAndFlushBlockingPool() { - RunMessageLoop(); - content::BrowserThread::GetBlockingPool()->FlushForTesting(); +void StorageInfoProviderTest::RunMessageLoopUntilTimeout(int ms) { + base::RunLoop run_loop; + message_loop_.PostDelayedTask(FROM_HERE, run_loop.QuitClosure(), + base::TimeDelta::FromMilliseconds(ms)); + run_loop.Run(); + ResetTestingData(); } -void StorageInfoProviderTest::RunAllPendingAndFlushBlockingPool() { - RunAllPendingInMessageLoop(); - content::BrowserThread::GetBlockingPool()->FlushForTesting(); +void StorageInfoProviderTest::RunMessageLoopUntilIdle() { + base::RunLoop run_loop; + run_loop.RunUntilIdle(); + ResetTestingData(); +} + +void StorageInfoProviderTest::ResetTestingData() { + for (size_t i = 0; i < arraysize(testing_data); ++i) + testing_data[i].available_capacity = 10; } TEST_F(StorageInfoProviderTest, WatchingNoChangedStorage) { // Case 1: watching a storage that the free space is not changed. - MockStorageObserver observer; - storage_info_provider_->AddObserver(&observer); - storage_info_provider_->StartWatching(kTestingData[0].id); - EXPECT_CALL(observer, OnStorageFreeSpaceChanged(kTestingData[0].id, _, _)) + StorageInfoProvider::Get()->StartWatching(testing_data[0].id); + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[0].id, _, _)) .Times(0); - RunLoopAndFlushBlockingPool(); - - storage_info_provider_->RemoveObserver(&observer); - storage_info_provider_->StopWatching(kTestingData[0].id); - RunAllPendingAndFlushBlockingPool(); + RunMessageLoopUntilTimeout(10 * kWatchingIntervalMs); + StorageInfoProvider::Get()->StopWatching(testing_data[0].id); + RunMessageLoopUntilIdle(); } TEST_F(StorageInfoProviderTest, WatchingOneStorage) { // Case 2: only watching one storage. - TestStorageObserver observer; - storage_info_provider_->AddObserver(&observer); - storage_info_provider_->StartWatching(kTestingData[1].id); - RunLoopAndFlushBlockingPool(); - - // The times of free space change is at least |kCheckTimes|, it is not easy - // to anticiapte the accurate number since it is still possible for blocking - // pool to run the pending checking task after completing |kCheckTimes| - // checking tasks and posting Quit to the message loop of UI thread. The - // observer guarantees that the free space is changed with the expected - // delta value. - EXPECT_GE(observer.GetFreeSpaceChangeTimes(kTestingData[1].id), - kCheckTimes); - // Other storages should not be changed. The first two entries are skipped. - for (size_t i = 2; i < arraysize(kTestingData); ++i) { - EXPECT_EQ(0, observer.GetFreeSpaceChangeTimes(kTestingData[i].id)); + StorageInfoProvider::Get()->StartWatching(testing_data[1].id); + RunMessageLoopUntilIdle(); + + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id, _, _)) + .WillRepeatedly(Return()); + double base_value = testing_data[1].available_capacity; + int step = testing_data[1].change_step; + for (int i = 0; i < kCallTimes; i++) { + double expected_old_value = base_value + i * step; + double expected_new_value = base_value + (i + 1) * step; + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id, + expected_old_value, expected_new_value)).WillOnce(Return()); } + // The other storages won't get free space change notification. + for (size_t i = 2; i < arraysize(testing_data); ++i) { + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[i].id, _, _)) + .Times(0); + } + RunMessageLoopUntilTimeout(100 * kWatchingIntervalMs); - storage_info_provider_->StopWatching(kTestingData[1].id); - // Give a chance to run StopWatching task on the blocking pool. - RunAllPendingAndFlushBlockingPool(); - - MockStorageObserver mock_observer; - storage_info_provider_->AddObserver(&mock_observer); - // The watched storage won't get free space change notification. - EXPECT_CALL(mock_observer, - OnStorageFreeSpaceChanged(kTestingData[1].id, _, _)).Times(0); - RunAllPendingAndFlushBlockingPool(); - - storage_info_provider_->RemoveObserver(&observer); - storage_info_provider_->RemoveObserver(&mock_observer); + StorageInfoProvider::Get()->StopWatching(testing_data[1].id); + RunMessageLoopUntilIdle(); + // The watched storage won't get free space change notification after + // stopping. + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id, _, _)) + .Times(0); + RunMessageLoopUntilIdle(); } -TEST_F(StorageInfoProviderTest, WatchingMultipleStorages) { +TEST_F(StorageInfoProviderTest, DISABLED_WatchingMultipleStorages) { // Case 2: watching multiple storages. We ignore the first entry in - // |kTestingData| since its change_step is zero. - TestStorageObserver observer; - storage_info_provider_->AddObserver(&observer); - - for (size_t k = 1; k < arraysize(kTestingData); ++k) { - storage_info_provider_->StartWatching(kTestingData[k].id); + // |testing_data| since its change_step is zero. + for (size_t k = 1; k < arraysize(testing_data); ++k) { + StorageInfoProvider::Get()->StartWatching(testing_data[k].id); } - RunLoopAndFlushBlockingPool(); - - // Right now let's verify the results. - for (size_t k = 1; k < arraysize(kTestingData); ++k) { - // See the above comments about the reason why the times of free space - // changes is at least kCheckTimes. - EXPECT_GE(observer.GetFreeSpaceChangeTimes(kTestingData[k].id), - kCheckTimes); + // Run the message loop to given a chance for storage info provider to start + // watching. + RunMessageLoopUntilIdle(); + + for (size_t k = 1; k < arraysize(testing_data); ++k) { + EXPECT_CALL(observer(), + OnStorageFreeSpaceChanged(testing_data[k].id, _, _)) + .WillRepeatedly(Return()); + + double base_value = testing_data[k].available_capacity; + int step = testing_data[k].change_step; + for (int i = 0; i < kCallTimes; i++) { + double expected_old_value = base_value + i * step; + double expected_new_value = base_value + (i + 1) * step; + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[k].id, + expected_old_value, expected_new_value)).WillOnce(Return()); + } } + RunMessageLoopUntilTimeout(100 * kWatchingIntervalMs); // Stop watching the first storage. - storage_info_provider_->StopWatching(kTestingData[1].id); - RunAllPendingAndFlushBlockingPool(); - - MockStorageObserver mock_observer; - storage_info_provider_->AddObserver(&mock_observer); - for (size_t k = 2; k < arraysize(kTestingData); ++k) { - EXPECT_CALL(mock_observer, - OnStorageFreeSpaceChanged(kTestingData[k].id, _, _)) + StorageInfoProvider::Get()->StopWatching(testing_data[1].id); + RunMessageLoopUntilIdle(); + + for (size_t k = 2; k < arraysize(testing_data); ++k) { + EXPECT_CALL(observer(), + OnStorageFreeSpaceChanged(testing_data[k].id, _, _)) .WillRepeatedly(Return()); - } - // After stopping watching, the observer won't get change notification. - EXPECT_CALL(mock_observer, - OnStorageFreeSpaceChanged(kTestingData[1].id, _, _)).Times(0); - RunLoopAndFlushBlockingPool(); + double base_value = testing_data[k].available_capacity; + int step = testing_data[k].change_step; + for (int i = 0; i < kCallTimes; i++) { + double expected_old_value = base_value + i * step; + double expected_new_value = base_value + (i + 1) * step; + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[k].id, + expected_old_value, expected_new_value)).WillOnce(Return()); + } + } + // After stopping watching, the callback won't get called. + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[1].id, _, _)) + .Times(0); + RunMessageLoopUntilTimeout(100 * kWatchingIntervalMs); - for (size_t k = 1; k < arraysize(kTestingData); ++k) { - storage_info_provider_->StopWatching(kTestingData[k].id); + // Stop watching all storages. + for (size_t k = 1; k < arraysize(testing_data); ++k) { + StorageInfoProvider::Get()->StopWatching(testing_data[k].id); + } + // Run the message loop to given a chance for storage info provider to stop + // watching. + RunMessageLoopUntilIdle(); + + // After stopping watching, the callback won't get called. + for (size_t k = 1; k < arraysize(testing_data); ++k) { + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(testing_data[k].id, _, _)) + .Times(0); } - RunAllPendingAndFlushBlockingPool(); - storage_info_provider_->RemoveObserver(&observer); - storage_info_provider_->RemoveObserver(&mock_observer); + RunMessageLoopUntilIdle(); } TEST_F(StorageInfoProviderTest, WatchingInvalidStorage) { // Case 3: watching an invalid storage. std::string invalid_id("invalid_id"); - MockStorageObserver mock_observer; - storage_info_provider_->AddObserver(&mock_observer); - storage_info_provider_->StartWatching(invalid_id); - EXPECT_CALL(mock_observer, - OnStorageFreeSpaceChanged(invalid_id, _, _)).Times(0); - RunAllPendingAndFlushBlockingPool(); - storage_info_provider_->RemoveObserver(&mock_observer); + StorageInfoProvider::Get()->StartWatching(invalid_id); + EXPECT_CALL(observer(), OnStorageFreeSpaceChanged(invalid_id, _, _)) + .Times(0); + RunMessageLoopUntilTimeout(10 * kWatchingIntervalMs); + StorageInfoProvider::Get()->StopWatching(invalid_id); + RunMessageLoopUntilIdle(); } } // namespace extensions diff --git a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_win.cc b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_win.cc index d56f9ea..83c347f 100644 --- a/chrome/browser/extensions/api/system_info_storage/storage_info_provider_win.cc +++ b/chrome/browser/extensions/api/system_info_storage/storage_info_provider_win.cc @@ -22,13 +22,11 @@ const unsigned long kMaxLogicalDriveString = 4 * 26; class StorageInfoProviderWin : public StorageInfoProvider { public: StorageInfoProviderWin() {} + virtual ~StorageInfoProviderWin() {} virtual bool QueryInfo(StorageInfo* info) OVERRIDE; virtual bool QueryUnitInfo(const std::string& id, StorageUnitInfo* info) OVERRIDE; - - private: - virtual ~StorageInfoProviderWin() {} }; bool StorageInfoProviderWin::QueryInfo(StorageInfo* info) { diff --git a/chrome/browser/extensions/api/system_info_storage/system_info_storage_apitest.cc b/chrome/browser/extensions/api/system_info_storage/system_info_storage_apitest.cc index 3493c06..a03b8e9 100644 --- a/chrome/browser/extensions/api/system_info_storage/system_info_storage_apitest.cc +++ b/chrome/browser/extensions/api/system_info_storage/system_info_storage_apitest.cc @@ -23,6 +23,9 @@ class MockStorageInfoProvider : public StorageInfoProvider { public: MockStorageInfoProvider() : is_watching_(false) { } + virtual ~MockStorageInfoProvider() { + Stop(); + } virtual bool QueryInfo(StorageInfo* info) OVERRIDE { info->clear(); @@ -63,10 +66,6 @@ class MockStorageInfoProvider : public StorageInfoProvider { } private: - virtual ~MockStorageInfoProvider() { - Stop(); - } - void OnTimeoutEvent() { static int count; SystemInfoEventRouter::GetInstance()-> |