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;
+    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 WorkerPool thread.
+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 WorkerPool thread.
+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,

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

nit:  Think this function name is a little weird.  Maybe WriteDebugLogToFile? 
Or PopulateDebugLogFile, or somesuch.

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

Done.

+                      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.

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

Suggest an overview either here or up top.  Something along the lines of:

"The file is created on the worker pool, then writing to it is triggered from
the UI thread, and finally it is closed (on success) or deleted (on failure) on
the worker pool, prior to calling |callback|."

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

Done.

 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());
 }