[SystemInfo API] Finish TODOs in SystemInfoProvider

chrome/browser/extensions/api/system_info/system_info_provider.h

Index: chrome/browser/extensions/api/system_info/system_info_provider.h
diff --git a/chrome/browser/extensions/api/system_info/system_info_provider.h b/chrome/browser/extensions/api/system_info/system_info_provider.h
index 4b8ad9ae97e2aa9a165cb964bfc8b3d37b8e800f..ba4835ffddd23bc197789fe2bcca07db7b3151e1 100644
--- a/chrome/browser/extensions/api/system_info/system_info_provider.h
+++ b/chrome/browser/extensions/api/system_info/system_info_provider.h
@@ -40,14 +40,12 @@ class SystemInfoProvider
   // two arguments. The first one is the information got already, the second
   // one indicates whether its contents are valid, for example, no error
   // occurs in querying the information.
-  typedef base::Callback<void(const T&, bool)> QueryInfoCompletionCallback;
+  typedef base::Callback<void(bool)> QueryInfoCompletionCallback;
   typedef std::queue<QueryInfoCompletionCallback> CallbackQueue;
 
   SystemInfoProvider()
-    : is_waiting_for_completion_(false) {
-    worker_pool_token_ =
-      content::BrowserThread::GetBlockingPool()->GetSequenceToken();
-  }
+    : is_waiting_for_completion_(false),
+      success_(false) {}
 
   virtual ~SystemInfoProvider() {}
 
@@ -77,50 +75,30 @@ class SystemInfoProvider
  protected:
   // Default implementation of querying system information.
   virtual void StartQueryInfoImpl() {
-    base::Closure callback =
-        base::Bind(&SystemInfoProvider<T>::QueryOnWorkerPool, this);
-    PostQueryTaskToBlockingPool(FROM_HERE, callback);
-  }
-
-  // Post a task to blocking pool for information querying.
-  void PostQueryTaskToBlockingPool(const tracked_objects::Location& from_here,
-                                   const base::Closure& query_callback) {
-    DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-    base::SequencedWorkerPool* worker_pool =
-        content::BrowserThread::GetBlockingPool();
-    // The query task posted to the worker pool won't block shutdown, and any
-    // running query task at shutdown time will be ignored.
-    worker_pool->PostSequencedWorkerTaskWithShutdownBehavior(
-          worker_pool_token_, from_here, query_callback,
-          base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+     // Post a task to blocking pool for information querying and reply with
+     // OnQueryCompleted.
+    content::BrowserThread::PostBlockingPoolTaskAndReply(FROM_HERE,
+        base::Bind(&SystemInfoProvider<T>::QueryInfo, this),
+        base::Bind(&SystemInfoProvider<T>::OnQueryCompleted, this));
   }
 
-  // Query the system information synchronously and output the result to the
-  // |info| parameter. The |info| contents MUST be reset firstly in its
-  // platform specific implementation. Return true if it succeeds, otherwise
-  // false is returned.
-  // TODO(Haojian): Remove the parameter T-typed pointer.
-  virtual bool QueryInfo(T* info) = 0;
-
-  // TODO(Haojian): Use PostBlockingPoolTaskAndReply to avoid unnecessary
-  // trampolines trip.
-  virtual void QueryOnWorkerPool() {
-    bool success = QueryInfo(&info_);
-    content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-        base::Bind(&SystemInfoProvider<T>::OnQueryCompleted, this, success));
-  }
+  // Query the system information synchronously and the result is info_.
+  virtual void QueryInfo() = 0;
 
   // Called on UI thread. The |success| parameter means whether it succeeds
   // to get the information.
-  virtual void OnQueryCompleted(bool success) {
+  virtual void OnQueryCompleted() {
     DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
     while (!callbacks_.empty()) {
       QueryInfoCompletionCallback callback = callbacks_.front();
-      callback.Run(info_, success);
+      callback.Run(success_);
       callbacks_.pop();
     }
 
+    // Reset success_ flag.
+    success_ = false;

[Hongbo Min, 2013/07/01 10:23:41]

why to keep a member for success flag instead of the return value of QueryInfo? 

QueryInfo returning a boolean type seems to be a bit reasonable.

[Haojian Wu, 2013/07/01 15:16:42]

Since we are using PostBlockingPoolTaskAndReply function (see
system_info_provider.h line 80). The function second and third parameters' type
are base::Closure which actually is Callback<void(void)>. If QueryInfo returns a
boolean type, then will cause a failed compile.

[Hongbo Min, 2013/07/02 00:57:58]

If that is the only reason to let QueryInfo return void, I suggest that you
could try to base::PostTaskAndReplyWithResult instead. The pattern to use it is:

scoped_refptr<base::SequencedWorkerPool> pool =
    content::BrowserThread::GetBlockingPool();
blocking_task_runner_ = pool->GetSequencedTaskRunner(pool->GetSequenceToken());
base::PostTaskAndReplyWithResult(
    blocking_task_runner_,
    FROM_HERE,
    base::Bind(&SystemInfoProvider<T>::QueryInfo, this),
    base::Bind(&SystemInfoProvider<T>::OnQueryCompleted, this));

Note the input parameter of OnQueryCompleted should be the return value of
QueryInfo.
    

[Haojian Wu, 2013/07/05 03:01:34]

Done.

+
     is_waiting_for_completion_ = false;
  }
 
@@ -138,6 +116,9 @@ class SystemInfoProvider
   // type generated by IDL parser.
   T info_;
 
+  // Record the last QueryInfo function call whether processed successfully.
+  bool success_;
+
  private:
   // The single shared provider instance. We create it only when needed.
   static typename base::LazyInstance<
@@ -150,10 +131,6 @@ class SystemInfoProvider
   // Indicates if it is waiting for the querying completion.
   bool is_waiting_for_completion_;
 
-  // Unqiue sequence token so that the operation of querying inforation can
-  // be executed in order.
-  base::SequencedWorkerPool::SequenceToken worker_pool_token_;
-
   DISALLOW_COPY_AND_ASSIGN(SystemInfoProvider<T>);
 };