[SystemInfo API] Finish TODOs in SystemInfoProvider

* Remove parameter T-type in SystemInfoProvider::QueryInfo method.
* Use PostBlockingPoolTaskAndReply to avoid unnecessary trampolines trip
  on UI thread and blocking pool.
* Remove parameter T-type in QueryInfoCompletionCallback function.

BUG=None
TEST=unit_tests --gtest_filter=*InfoProviderTest.*
TEST=browser_tests --gtest_filter=SystemInfo*.*

Review URL: https://chromiumcodereview.appspot.com/18290002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211087 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 72 insertions, 51 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 581e02f..c43fe8f 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
@@ -42,6 +42,12 @@ void BuildStorageUnitInfo(const chrome::StorageInfo& info,
 const int kDefaultPollingIntervalMs = 1000;
 const char kWatchingTokenName[] = "_storage_info_watching_token_";
 
+// Static member intialization.
+template<>
+base::LazyInstance<scoped_refptr<SystemInfoProvider<StorageUnitInfoList> > >
+  SystemInfoProvider<StorageUnitInfoList>::provider_
+      = LAZY_INSTANCE_INITIALIZER;
+
 StorageInfoProvider::StorageInfoProvider()
     : observers_(new ObserverListThreadSafe<StorageFreeSpaceObserver>()),
       watching_interval_(kDefaultPollingIntervalMs) {
@@ -50,55 +56,50 @@ StorageInfoProvider::StorageInfoProvider()
 StorageInfoProvider::~StorageInfoProvider() {
 }
 
-void StorageInfoProvider::StartQueryInfoImpl() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // Register a callback to notify UI thread that StorageMonitor finishes the
-  // storage metadata retrieval on FILE thread. See the comments of
-  // StorageMonitor::Initialize about when the callback gets run.
-  StorageMonitor::GetInstance()->EnsureInitialized(
-      base::Bind(&StorageInfoProvider::QueryInfoImplOnUIThread, this));
+const StorageUnitInfoList& StorageInfoProvider::storage_unit_info_list() const {
+  return info_;
+}
+
+void StorageInfoProvider::PrepareQueryOnUIThread() {
+  // Get all available storage devices before invoking |QueryInfo()| to get
+  // available capacity.
+  GetAllStoragesIntoInfoList();
 }
 
-void StorageInfoProvider::QueryInfoImplOnUIThread() {
+void StorageInfoProvider::InitializeProvider(
+    const base::Closure& do_query_info_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // At this point, we can call StorageMonitor::GetAllAvailableStorages to
-  // get the correct results.
-  QueryInfo(&info_);
-  // The amount of available capacity should be queried on blocking pool.
-  PostQueryTaskToBlockingPool(FROM_HERE,
-      base::Bind(&StorageInfoProvider::QueryAvailableCapacityOnBlockingPool,
-          this));
+  // Register the |do_query_info_callback| callback to StorageMonitor.
+  // See the comments of StorageMonitor::EnsureInitialized about when the
+  // callback gets run.
+  StorageMonitor::GetInstance()->EnsureInitialized(do_query_info_callback);
 }
 
-void StorageInfoProvider::QueryAvailableCapacityOnBlockingPool() {
+bool StorageInfoProvider::QueryInfo() {
   DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
-  for (StorageInfo::iterator it = info_.begin(); it != info_.end(); ++it) {
+  for (StorageUnitInfoList::iterator it = info_.begin();
+       it != info_.end(); ++it) {
     int64 amount = GetStorageFreeSpaceFromTransientId((*it)->id);
     if (amount > 0)
       (*it)->available_capacity = static_cast<double>(amount);
   }
 
-  // Notify UI thread that the querying operation has completed.
-  BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-      base::Bind(&StorageInfoProvider::OnQueryCompleted, this, true));
+  return true;
 }
 
 std::vector<chrome::StorageInfo> StorageInfoProvider::GetAllStorages() const {
   return StorageMonitor::GetInstance()->GetAllAvailableStorages();
 }
 
-bool StorageInfoProvider::QueryInfo(StorageInfo* info) {
+void StorageInfoProvider::GetAllStoragesIntoInfoList() {
+  info_.clear();
   std::vector<chrome::StorageInfo> storage_list = GetAllStorages();
-  StorageInfo results;
   std::vector<chrome::StorageInfo>::const_iterator it = storage_list.begin();
   for (; it != storage_list.end(); ++it) {
     linked_ptr<StorageUnitInfo> unit(new StorageUnitInfo());
     systeminfo::BuildStorageUnitInfo(*it, unit.get());
-    results.push_back(unit);
+    info_.push_back(unit);
   }
-  info->swap(results);
-
-  return true;
 }
 
 void StorageInfoProvider::AddObserver(StorageFreeSpaceObserver* obs) {
@@ -128,6 +129,20 @@ void StorageInfoProvider::StopWatching(const std::string& transient_id) {
                  this, transient_id));
 }
 
+void StorageInfoProvider::StartWatchingAllStorages() {
+  for (StorageUnitInfoList::const_iterator it = info_.begin();
+       it != info_.end(); ++it) {
+    StartWatching((*it)->id);
+  }
+}
+
+void StorageInfoProvider::StopWatchingAllStorages() {
+  for (StorageUnitInfoList::const_iterator it = info_.begin();
+       it != info_.end(); ++it) {
+    StopWatching((*it)->id);
+  }
+}
+
 int64 StorageInfoProvider::GetStorageFreeSpaceFromTransientId(
     const std::string& transient_id) {
   std::vector<chrome::StorageInfo> storage_list = GetAllStorages();
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 cfa7397..a6138dd 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
@@ -33,9 +33,10 @@ void BuildStorageUnitInfo(const chrome::StorageInfo& info,
 }  // namespace systeminfo
 
 typedef std::vector<linked_ptr<
-    api::experimental_system_info_storage::StorageUnitInfo> > StorageInfo;
+    api::experimental_system_info_storage::StorageUnitInfo> >
+        StorageUnitInfoList;
 
-class StorageInfoProvider : public SystemInfoProvider<StorageInfo> {
+class StorageInfoProvider : public SystemInfoProvider<StorageUnitInfoList> {
  public:
   StorageInfoProvider();
 
@@ -47,12 +48,21 @@ class StorageInfoProvider : public SystemInfoProvider<StorageInfo> {
   void RemoveObserver(StorageFreeSpaceObserver* obs);
 
   // Start and stop watching the given storage |transient_id|.
-  virtual void StartWatching(const std::string& transient_id);
-  virtual void StopWatching(const std::string& transient_id);
+  void StartWatching(const std::string& transient_id);
+  void StopWatching(const std::string& transient_id);
+
+  // Start and stop watching all available storages.
+  void StartWatchingAllStorages();
+  void StopWatchingAllStorages();
 
   // Returns all available storages, including fixed and removable.
   virtual std::vector<chrome::StorageInfo> GetAllStorages() const;
 
+  // SystemInfoProvider implementations
+  virtual void PrepareQueryOnUIThread() OVERRIDE;
+  virtual void InitializeProvider(const base::Closure& do_query_info_callback)
+      OVERRIDE;
+
   // Get the amount of storage free space from |transient_id|, or -1 on failure.
   virtual int64 GetStorageFreeSpaceFromTransientId(
       const std::string& transient_id);
@@ -62,27 +72,26 @@ class StorageInfoProvider : public SystemInfoProvider<StorageInfo> {
   virtual std::string GetDeviceIdForTransientId(
       const std::string& transient_id) const;
 
+  const StorageUnitInfoList& storage_unit_info_list() const;
+
  protected:
   virtual ~StorageInfoProvider();
 
   // TODO(Haojian): Put this method in a testing subclass rather than here.
   void SetWatchingIntervalForTesting(size_t ms) { watching_interval_ = ms; }
 
-  // SystemInfoProvider implementations.
-  virtual bool QueryInfo(StorageInfo* info) OVERRIDE;
-  virtual void StartQueryInfoImpl() OVERRIDE;
+  // Put all available storages' information into |info_|.
+  virtual void GetAllStoragesIntoInfoList();
 
  private:
   typedef std::map<std::string, double> StorageTransientIdToSizeMap;
 
-  // Query the available capacity of all known storage devices on the blocking
-  // pool, including fixed and removable.
-  void QueryAvailableCapacityOnBlockingPool();
+  // SystemInfoProvider implementations.
+  // Override to query the available capacity of all known storage devices on
+  // the blocking pool, including fixed and removable devices.
+  virtual bool QueryInfo() OVERRIDE;
   // Query the new attached removable storage info on the blocking pool.
   void QueryAttachedStorageInfoOnBlockingPool(const std::string& transient_id);
-  // Run the QueryInfo() implementation after the StorageMonitor has been
-  // initialized.
-  void QueryInfoImplOnUIThread();
 
   // Posts a task to check for free space changes on the blocking pool.
   // Should be called on the UI thread.
diff --git a/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.cc b/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.cc
index cc9d905..b126c65 100644
--- a/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.cc
+++ b/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.cc
@@ -20,12 +20,11 @@ bool SystemInfoStorageGetFunction::RunImpl() {
   return true;
 }
 
-void SystemInfoStorageGetFunction::OnGetStorageInfoCompleted(
-    const StorageInfo& info, bool success) {
-
+void SystemInfoStorageGetFunction::OnGetStorageInfoCompleted(bool success) {
   if (success) {
     results_ =
-      api::experimental_system_info_storage::Get::Results::Create(info);
+      api::experimental_system_info_storage::Get::Results::Create(
+          StorageInfoProvider::Get()->storage_unit_info_list());
   } else {
     SetError("Error occurred when querying storage information.");
   }
diff --git a/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.h b/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.h
index 02a4ff8..8699717 100644
--- a/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.h
+++ b/chrome/browser/extensions/api/system_info_storage/system_info_storage_api.h
@@ -21,7 +21,7 @@ class SystemInfoStorageGetFunction : public AsyncExtensionFunction {
   virtual ~SystemInfoStorageGetFunction();
   virtual bool RunImpl() OVERRIDE;
 
-  void OnGetStorageInfoCompleted(const StorageInfo& info, bool success);
+  void OnGetStorageInfoCompleted(bool success);
 };
 
 class SystemInfoStorageAddWatchFunction : public AsyncExtensionFunction {
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 40a1f8f..3c87491 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
@@ -22,7 +22,7 @@ using chrome::test::TestStorageMonitor;
 using extensions::api::experimental_system_info_storage::ParseStorageUnitType;
 using extensions::api::experimental_system_info_storage::StorageUnitInfo;
 using extensions::StorageInfoProvider;
-using extensions::StorageInfo;
+using extensions::StorageUnitInfoList;
 using extensions::systeminfo::kStorageTypeFixed;
 using extensions::systeminfo::kStorageTypeRemovable;
 using extensions::systeminfo::kStorageTypeUnknown;
diff --git a/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.cc b/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.cc
index 884bbf2..f90d2e1e 100644
--- a/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.cc
+++ b/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.cc
@@ -40,18 +40,16 @@ chrome::StorageInfo TestStorageInfoProvider::BuildStorageInfo(
   return info;
 }
 
-bool TestStorageInfoProvider::QueryInfo(extensions::StorageInfo* info) {
-  info->clear();
+void TestStorageInfoProvider::GetAllStoragesIntoInfoList() {
+  info_.clear();
   for (size_t i = 0; i < testing_data_.size(); ++i) {
     linked_ptr<StorageUnitInfo> unit(new StorageUnitInfo());
     unit->id = testing_data_[i].transient_id;
     unit->name = testing_data_[i].name;
     unit->type = ParseStorageUnitType(testing_data_[i].type);
     unit->capacity = testing_data_[i].capacity;
-    unit->available_capacity = testing_data_[i].available_capacity;
-    info->push_back(unit);
+    info_.push_back(unit);
   }
-  return true;
 }
 
 std::vector<chrome::StorageInfo>
diff --git a/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.h b/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.h
index 1d2f42b..0ab3d4c 100644
--- a/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.h
+++ b/chrome/browser/extensions/api/system_info_storage/test_storage_info_provider.h
@@ -48,7 +48,7 @@ class TestStorageInfoProvider : public extensions::StorageInfoProvider {
   virtual ~TestStorageInfoProvider();
 
   // StorageInfoProvider implementations.
-  virtual bool QueryInfo(extensions::StorageInfo* info) OVERRIDE;
+  virtual void GetAllStoragesIntoInfoList() OVERRIDE;
   virtual std::vector<chrome::StorageInfo> GetAllStorages() const OVERRIDE;
   virtual int64 GetStorageFreeSpaceFromTransientId(
       const std::string& transient_id) OVERRIDE;