chromium-os:6108 - about:system should use FILE thread to get system info.

Replaced SyslogsLibrary::GetSyslogs with asynchronous RequestSyslogs.
Modified BugReportHandler and AboutSource in browser_about_handler.cc appropriately.

BUG=http://code.google.com/p/chromium-os/issues/detail?id=6108
TEST=Test chrome:system, and bug reporter. Both should always be responsive. Test multiple instances of each, closing tabs quickly, and sending a bug quickly. Veryify results of bug report. Confirm that about:system no longer shows anything.

Review URL: http://codereview.chromium.org/3295024

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

3 files changed, 89 insertions, 22 deletions

diff --git a/chrome/browser/chromeos/cros/syslogs_library.cc b/chrome/browser/chromeos/cros/syslogs_library.cc
index 7c5f4fa..9fed176 100644
--- a/chrome/browser/chromeos/cros/syslogs_library.cc
+++ b/chrome/browser/chromeos/cros/syslogs_library.cc
@@ -15,12 +15,42 @@ class SyslogsLibraryImpl : public SyslogsLibrary {
   SyslogsLibraryImpl() {}
   virtual ~SyslogsLibraryImpl() {}
 
-  LogDictionaryType* GetSyslogs(FilePath* tmpfilename) {
+  virtual Handle RequestSyslogs(FilePath* tmpfilename,
+                                CancelableRequestConsumerBase* consumer,
+                                ReadCompleteCallback* callback) {
+    // Register the callback request.
+    scoped_refptr<CancelableRequest<ReadCompleteCallback> > request(
+           new CancelableRequest<ReadCompleteCallback>(callback));
+    AddRequest(request, consumer);
+
+    // Schedule a task on the FILE thread which will then trigger a request
+    // callback on the calling thread (e.g. UI) when complete.
+    ChromeThread::PostTask(
+        ChromeThread::FILE, FROM_HERE,
+        NewRunnableMethod(
+            this, &SyslogsLibraryImpl::ReadSyslogs, request, tmpfilename));
+
+    return request->handle();
+  }
+
+ private:
+  // Called from FILE thread.
+  void ReadSyslogs(
+      scoped_refptr<CancelableRequest<ReadCompleteCallback> > request,
+      FilePath* tmpfilename) {
+    if (request->canceled())
+      return;
+
+    LogDictionaryType* logs = NULL;
     if (CrosLibrary::Get()->EnsureLoaded()) {
-      return chromeos::GetSystemLogs(tmpfilename);
+      logs = chromeos::GetSystemLogs(tmpfilename);
     }
-    return NULL;
+
+    // Will call the callback on the calling thread.
+    request->ForwardResult(Tuple1<LogDictionaryType*>(logs));
   }
+
+  DISALLOW_COPY_AND_ASSIGN(SyslogsLibraryImpl);
 };
 
 class SyslogsLibraryStubImpl : public SyslogsLibrary {
@@ -28,12 +58,14 @@ class SyslogsLibraryStubImpl : public SyslogsLibrary {
   SyslogsLibraryStubImpl() {}
   virtual ~SyslogsLibraryStubImpl() {}
 
-  LogDictionaryType* GetSyslogs(FilePath* tmpfilename) {
-    return &log_dictionary_;
-  }
+  virtual Handle RequestSyslogs(FilePath* tmpfilename,
+                                CancelableRequestConsumerBase* consumer,
+                                ReadCompleteCallback* callback) {
+    if (callback)
+      callback->Run(Tuple1<LogDictionaryType*>(NULL));
 
- private:
-  LogDictionaryType log_dictionary_;
+    return 0;
+  }
 };
 
 // static
@@ -45,3 +77,7 @@ SyslogsLibrary* SyslogsLibrary::GetImpl(bool stub) {
 }
 
 }  // namespace chromeos
+
+// Allows InvokeLater without adding refcounting. SyslogsLibraryImpl is a
+// Singleton and won't be deleted until it's last InvokeLater is run.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(chromeos::SyslogsLibraryImpl);
diff --git a/chrome/browser/chromeos/cros/syslogs_library.h b/chrome/browser/chromeos/cros/syslogs_library.h
index 85c88cc..56a07f7 100644
--- a/chrome/browser/chromeos/cros/syslogs_library.h
+++ b/chrome/browser/chromeos/cros/syslogs_library.h
@@ -7,18 +7,29 @@
 #pragma once
 
 #include "base/singleton.h"
+#include "chrome/browser/cancelable_request.h"
 #include "cros/chromeos_syslogs.h"
 
+class CancelableRequestConsumerBase;
+
 namespace chromeos {
 
 // This interface defines interaction with the ChromeOS syslogs APIs.
-class SyslogsLibrary {
+class SyslogsLibrary : public CancelableRequestProvider {
  public:
+  typedef Callback1<LogDictionaryType*>::Type ReadCompleteCallback;
+
   SyslogsLibrary() {}
   virtual ~SyslogsLibrary() {}
 
-  // System logs gathered for userfeedback
-  virtual LogDictionaryType* GetSyslogs(FilePath* tmpfilename)  = 0;
+  // Request system logs. Read happens on the FILE thread and callback is
+  // called on the thread this is called from (via ForwardResult).
+  // Logs are owned by callback function (use delete when done with them).
+  // Returns the request handle. Call CancelRequest(Handle) to cancel
+  // the request before the callback gets called.
+  virtual Handle RequestSyslogs(FilePath* tmpfilename,
+                                CancelableRequestConsumerBase* consumer,
+                                ReadCompleteCallback* callback) = 0;
 
   // Factory function, creates a new instance and returns ownership.
   // For normal usage, access the singleton via CrosLibrary::Get().
diff --git a/chrome/browser/chromeos/dom_ui/system_info_ui.cc b/chrome/browser/chromeos/dom_ui/system_info_ui.cc
index 01172833..a15f8d80 100644
--- a/chrome/browser/chromeos/dom_ui/system_info_ui.cc
+++ b/chrome/browser/chromeos/dom_ui/system_info_ui.cc
@@ -46,6 +46,14 @@ class SystemInfoUIHTMLSource : public ChromeURLDataManager::DataSource {
  private:
   ~SystemInfoUIHTMLSource() {}
 
+  void SyslogsComplete(chromeos::LogDictionaryType* sys_info);
+
+  CancelableRequestConsumer consumer_;
+
+  // Stored data from StartDataRequest()
+  std::string path_;
+  int request_id_;
+
   DISALLOW_COPY_AND_ASSIGN(SystemInfoUIHTMLSource);
 };
 
@@ -71,12 +79,29 @@ class SystemInfoHandler : public DOMMessageHandler,
 ////////////////////////////////////////////////////////////////////////////////
 
 SystemInfoUIHTMLSource::SystemInfoUIHTMLSource()
-    : DataSource(chrome::kChromeUISystemInfoHost, MessageLoop::current()) {
+    : DataSource(chrome::kChromeUISystemInfoHost, MessageLoop::current()),
+      request_id_(0) {
 }
 
 void SystemInfoUIHTMLSource::StartDataRequest(const std::string& path,
                                               bool is_off_the_record,
                                               int request_id) {
+  path_ = path;
+  request_id_ = request_id;
+
+  chromeos::SyslogsLibrary* syslogs_lib =
+      chromeos::CrosLibrary::Get()->GetSyslogsLibrary();
+  if (syslogs_lib) {
+    FilePath* tmpfilename = NULL;  // use default filepath.
+    syslogs_lib->RequestSyslogs(
+        tmpfilename,
+        &consumer_,
+        NewCallback(this, &SystemInfoUIHTMLSource::SyslogsComplete));
+  }
+}
+
+void SystemInfoUIHTMLSource::SyslogsComplete(
+    chromeos::LogDictionaryType* sys_info) {
   DictionaryValue strings;
   strings.SetString("title", l10n_util::GetStringUTF16(IDS_ABOUT_SYS_TITLE));
   strings.SetString("description",
@@ -93,24 +118,19 @@ void SystemInfoUIHTMLSource::StartDataRequest(const std::string& path,
                     l10n_util::GetStringUTF16(IDS_ABOUT_SYS_COLLAPSE));
   SetFontAndTextDirection(&strings);
 
-  chromeos::SyslogsLibrary* syslogs_lib =
-      chromeos::CrosLibrary::Get()->GetSyslogsLibrary();
-  scoped_ptr<chromeos::LogDictionaryType> sys_info;
-  if (syslogs_lib)
-    sys_info.reset(syslogs_lib->GetSyslogs(new FilePath()));
-  if (sys_info.get()) {
+  if (sys_info) {
      ListValue* details = new ListValue();
      strings.Set("details", details);
      chromeos::LogDictionaryType::iterator it;
-     for (it = sys_info.get()->begin(); it != sys_info.get()->end(); ++it) {
+     for (it = sys_info->begin(); it != sys_info->end(); ++it) {
        DictionaryValue* val = new DictionaryValue;
        val->SetString("stat_name", it->first);
        val->SetString("stat_value", it->second);
        details->Append(val);
      }
-     strings.SetString("anchor", path);
+     strings.SetString("anchor", path_);
+     delete sys_info;
   }
-
   static const base::StringPiece systeminfo_html(
       ResourceBundle::GetSharedInstance().GetRawDataResource(
           IDR_ABOUT_SYS_HTML));
@@ -121,7 +141,7 @@ void SystemInfoUIHTMLSource::StartDataRequest(const std::string& path,
   html_bytes->data.resize(full_html.size());
   std::copy(full_html.begin(), full_html.end(), html_bytes->data.begin());
 
-  SendResponse(request_id, html_bytes);
+  SendResponse(request_id_, html_bytes);
 }
 
 ////////////////////////////////////////////////////////////////////////////////