Fixed GetDebugLogs functionality.

chrome/browser/ui/webui/net_internals/net_internals_ui.cc

 // 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.
 
 #include "chrome/browser/ui/webui/net_internals/net_internals_ui.h"
 
 #include <algorithm>
 #include <list>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/message_loop_helpers.h"
 #include "base/path_service.h"
 #include "base/platform_file.h"
 #include "base/string_number_conversions.h"
 #include "base/string_piece.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
+#include "base/threading/worker_pool.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data_remover.h"
+#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/connection_tester.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/pref_member.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/browser/ui/webui/chrome_web_ui_data_source.h"
@@ skipping 33 matching lines @@
 #include "chrome/browser/chromeos/cros/cros_library.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/system/syslogs_provider.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon_client.h"
 #endif
 #ifdef OS_WIN
 #include "chrome/browser/net/service_providers_win.h"
 #endif
 
+using base::PassPlatformFile;
 using base::PlatformFile;
 using base::PlatformFileError;
 using content::BrowserThread;
 using content::WebContents;
 using content::WebUIMessageHandler;
 
 namespace {
 
 // Delay between when an event occurs and when it is passed to the Javascript
 // page.  All events that occur during this period are grouped together and
@@ skipping 54 matching lines @@
 
   source->set_default_resource(IDR_NET_INTERNALS_INDEX_HTML);
   source->add_resource_path("help.html", IDR_NET_INTERNALS_HELP_HTML);
   source->add_resource_path("help.js", IDR_NET_INTERNALS_HELP_JS);
   source->add_resource_path("index.js", IDR_NET_INTERNALS_INDEX_JS);
   source->set_json_path("strings.js");
   return source;
 }
 
 #ifdef OS_CHROMEOS
+// Small helper class used to create temporary log file and pass its
+// handle and error status to callback.
+// Use case:
+// DebugLogFileHelper* helper = new DebugLogFileHelper();
+// base::WorkerPool::PostTaskAndReply(FROM_HERE,
+//     base::Bind(&DebugLogFileHelper::DoWork, base::Unretained(helper), ...),
+//     base::Bind(&DebugLogFileHelper::Reply, base::Owned(helper), ...),
+//     false);
+class DebugLogFileHelper {
+ public:
+  typedef base::Callback<void(PassPlatformFile pass_platform_file,
+                              bool created,
+                              PlatformFileError error,
+                              const FilePath& file_path)> DebugLogFileCallback;
+
+  DebugLogFileHelper()
+      : file_handle_(base::kInvalidPlatformFileValue),
+        created_(false),
+        error_(base::PLATFORM_FILE_OK) {
+  }
+
+  ~DebugLogFileHelper() {
+  }
+
+  void DoWork(const FilePath& fileshelf) {
+    const FilePath::CharType kLogFileName[] =
+        FILE_PATH_LITERAL("debug-log.tgz");
+
+    file_path_ = fileshelf.Append(kLogFileName);
+    file_path_ = logging::GenerateTimestampedName(file_path_,
+                                                  base::Time::Now());
+
+    int flags = base::PLATFORM_FILE_CREATE_ALWAYS |
+        base::PLATFORM_FILE_WRITE;

[mmenke, 2012/05/15 17:25:28]

nit:  For legibility, may want to align the two flags at base::

[ygorshenin1, 2012/05/17 07:48:46]

Done.

+    file_handle_ = base::CreatePlatformFile(file_path_, flags,
+                                            &created_, &error_);
+  }
+
+  void Reply(const DebugLogFileCallback& callback) {
+    DCHECK(!callback.is_null());
+    callback.Run(PassPlatformFile(&file_handle_), created_, error_, file_path_);
+  }
+
+ private:
+  PlatformFile file_handle_;
+  bool created_;
+  PlatformFileError error_;
+  FilePath file_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(DebugLogFileHelper);
+};
+
 // Following functions are used for getting debug logs. Logs are
 // fetched from /var/log/* and put on the fileshelf.
 
 // Called once StoreDebugLogs is complete. Takes two parameters:
 // - log_path: where the log file was saved in the case of success;
 // - succeeded: was the log file saved successfully.
 typedef base::Callback<void(const FilePath& log_path,
                             bool succeded)> StoreDebugLogsCallback;
 
-// Called once creation of the debug log file is completed. If
-// creation failed, deletes the file by |log_path|. So, this function
-// should be called on the FILE thread. After all, calls |callback| on
-// the UI thread.
-void CreateDebugLogFileCompleted(const StoreDebugLogsCallback& callback,
-                                 const FilePath& log_path,
-                                 bool succeeded) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  if (!succeeded)
-    file_util::Delete(log_path, false);
-
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                          base::Bind(callback, log_path, succeeded));
+// Closes file handle, so, should be called on the FILE thread.

[Sam Leffler, 2012/05/15 16:42:28]

nit: called on a workerpool thread so comment needs adjusting

[ygorshenin1, 2012/05/15 17:13:39]

Done.

+void CloseDebugLogFile(PassPlatformFile pass_platform_file) {
+  base::ClosePlatformFile(pass_platform_file.ReleaseValue());
 }
 
-// Retrieves debug logs from DebugDaemon and puts them on the
-// fileshelf directory into .tgz archive. So, this function should be
-// called on the FILE thread. Calls CreateDebugLogFileCompleted on the
-// FILE thread when creation of archive is completed.
-void CreateDebugLogFile(const StoreDebugLogsCallback& callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+// Closes file handle and deletes debug log file, so, should be called
+// on the FILE thread.

[Sam Leffler, 2012/05/15 16:42:28]

workerpool thread

[ygorshenin1, 2012/05/15 17:13:39]

Done.

+void CloseAndDeleteDebugLogFile(PassPlatformFile pass_platform_file,
+                                const FilePath& file_path) {
+  CloseDebugLogFile(pass_platform_file);
+  file_util::Delete(file_path, false);
+}
 
-  const FilePath::CharType kLogFileName[] = FILE_PATH_LITERAL("debug-log.tgz");
-
-  FilePath fileshelf_dir;
-  if (!PathService::Get(chrome::DIR_DEFAULT_DOWNLOADS, &fileshelf_dir)) {
-    LOG(ERROR) << "Can't get fileshelf dir";
-    CreateDebugLogFileCompleted(callback, FilePath(), false);
+// Called after completion of |FillDebugLogFile|. Closes file
+// descriptor, deletes log file in the case of failure and calls
+// |callback|.
+void FillDebugLogFileCompleted(const StoreDebugLogsCallback& callback,
+                               PassPlatformFile pass_platform_file,
+                               const FilePath& file_path,
+                               bool succeeded) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!succeeded) {
+    bool posted = base::WorkerPool::PostTaskAndReply(FROM_HERE,
+        base::Bind(&CloseAndDeleteDebugLogFile, pass_platform_file, file_path),
+        base::Bind(callback, file_path, false), false);
+    DCHECK(posted);
     return;
   }
+  bool posted = base::WorkerPool::PostTaskAndReply(FROM_HERE,
+      base::Bind(&CloseDebugLogFile, pass_platform_file),
+      base::Bind(callback, file_path, true), false);
+  DCHECK(posted);
+}
 
-  FilePath log_path = fileshelf_dir.Append(kLogFileName);
-  log_path = logging::GenerateTimestampedName(log_path, base::Time::Now());
-
-  int flags = base::PLATFORM_FILE_CREATE_ALWAYS |
-      base::PLATFORM_FILE_WRITE;
-  bool created;
-  PlatformFileError error;
-  PlatformFile log_file = base::CreatePlatformFile(
-      log_path, flags, &created, &error);
-
+// Stores into |file_path| user logs in the .tgz format. Calls
+// |callback| upon completion.
+void FillDebugLogFile(const StoreDebugLogsCallback& callback,
+                      PassPlatformFile pass_platform_file,
+                      bool created,
+                      PlatformFileError error,
+                      const FilePath& file_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!created) {
     LOG(ERROR) <<
-        "Can't create log file: " << log_path.AsUTF8Unsafe() << ", " <<
+        "Can't create debug log file: " << file_path.AsUTF8Unsafe() << ", " <<
         "error: " << error;
-    CreateDebugLogFileCompleted(callback, log_path, false);
+    bool posted = base::WorkerPool::PostTaskAndReply(FROM_HERE,
+        base::Bind(&CloseDebugLogFile, pass_platform_file),
+        base::Bind(callback, file_path, false), false);
+    DCHECK(posted);
     return;
   }
-  chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->
-      GetDebugLogs(log_file,
-                   base::Bind(&CreateDebugLogFileCompleted,
-                              callback, log_path));
+  PlatformFile platform_file = pass_platform_file.ReleaseValue();
+  chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->GetDebugLogs(
+      platform_file,
+      base::Bind(&FillDebugLogFileCompleted,
+          callback, PassPlatformFile(&platform_file), file_path));
 }
 
-// Delegates the job of saving debug logs on the fileshelf to
-// CreateDebugLogsFile on the FILE thread. Calls |callback| on the UI
-// thread when saving is completed. This function should be called on
-// the UI thread.
+// Creates .tgz archive with user logs on the fileshelf. Calls
+// |callback| upon completion.
 void StoreDebugLogs(const StoreDebugLogsCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&CreateDebugLogFile, callback));
+  DCHECK(!callback.is_null());
+  const FilePath fileshelf = download_util::GetDefaultDownloadDirectory();
+  DebugLogFileHelper* helper = new DebugLogFileHelper();
+  bool posted = base::WorkerPool::PostTaskAndReply(FROM_HERE,
+      base::Bind(&DebugLogFileHelper::DoWork,
+          base::Unretained(helper), fileshelf),
+      base::Bind(&DebugLogFileHelper::Reply, base::Owned(helper),
+          base::Bind(&FillDebugLogFile, callback)), false);
+  DCHECK(posted);
 }
 #endif  // OS_CHROMEOS
 
 // This class receives javascript messages from the renderer.
 // Note that the WebUI infrastructure runs on the UI thread, therefore all of
 // this class's methods are expected to run on the UI thread.
 //
 // Since the network code we want to run lives on the IO thread, we proxy
 // almost everything over to NetInternalsMessageHandler::IOThreadImpl, which
 // runs on the IO thread.
@@ skipping 1426 matching lines @@
 }
 
 NetInternalsUI::NetInternalsUI(content::WebUI* web_ui)
     : WebUIController(web_ui) {
   web_ui->AddMessageHandler(new NetInternalsMessageHandler());
 
   // Set up the chrome://net-internals/ source.
   Profile* profile = Profile::FromWebUI(web_ui);
   ChromeURLDataManager::AddDataSource(profile, CreateNetInternalsHTMLSource());
 }