Use base::File for DebugDaemonClient::GetDebugLogs

There is no need to keep the file descriptor open in net_internals_ui.cc as libdbus duplicates the FD when appropriate.

Use base::File instead of PlatformFile.
Use blocking pool instead of WorkerPool.

BUG=369183
TEST=Can save debug logs in "Chrome OS" section of chrome://net-internals on a Chromebook.

Review URL: https://codereview.chromium.org/270853002

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

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

@@ -27,7 +27,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/cancelable_task_tracker.h"
-#include "base/threading/worker_pool.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
@@ -209,48 +208,6 @@ content::WebUIDataSource* CreateNetInternalsHTMLSource() {
 }
 
 #if defined(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(base::File file,
-                              const base::FilePath& file_path)>
-      DebugLogFileCallback;
-
-  DebugLogFileHelper() {}
-
-  ~DebugLogFileHelper() {}
-
-  void DoWork(const base::FilePath& fileshelf) {
-    const base::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::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE;
-    file_.Initialize(file_path_, flags);
-  }
-
-  void Reply(const DebugLogFileCallback& callback) {
-    DCHECK(!callback.is_null());
-    callback.Run(file_.Pass(), file_path_);
-  }
-
- private:
-  base::File file_;
-  base::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.
 
@@ -260,56 +217,39 @@ class DebugLogFileHelper {
 typedef base::Callback<void(const base::FilePath& log_path,
                             bool succeded)> StoreDebugLogsCallback;
 
-// Closes file handle, so, should be called on the WorkerPool thread.
-void CloseDebugLogFile(base::File file) {
-  file.Close();
-}
-
-// Closes file handle and deletes debug log file, so, should be called
-// on the WorkerPool thread.
-void CloseAndDeleteDebugLogFile(base::File file,
-                                const base::FilePath& file_path) {
-  file.Close();
-  base::DeleteFile(file_path, false);
-}
-
 // Called upon completion of |WriteDebugLogToFile|. Closes file
 // descriptor, deletes log file in the case of failure and calls
 // |callback|.
 void WriteDebugLogToFileCompleted(const StoreDebugLogsCallback& callback,
-                                  base::File file,
                                   const base::FilePath& file_path,
                                   bool succeeded) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (!succeeded) {
-    bool posted = base::WorkerPool::PostTaskAndReply(FROM_HERE,
-        base::Bind(&CloseAndDeleteDebugLogFile, Passed(&file), file_path),
-        base::Bind(callback, file_path, false), false);
+    bool posted = BrowserThread::PostBlockingPoolTaskAndReply(
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&base::DeleteFile), file_path, false),
+        base::Bind(callback, file_path, false));
     DCHECK(posted);
     return;
   }
-  bool posted = base::WorkerPool::PostTaskAndReply(FROM_HERE,
-      base::Bind(&CloseDebugLogFile, Passed(&file)),
-      base::Bind(callback, file_path, true), false);
-  DCHECK(posted);
+  callback.Run(file_path, true);
 }
 
 // Stores into |file_path| debug logs in the .tgz format. Calls
 // |callback| upon completion.
 void WriteDebugLogToFile(const StoreDebugLogsCallback& callback,
-                         base::File file,
+                         base::File* file,
                          const base::FilePath& file_path) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (!file.IsValid()) {
+  if (!file->IsValid()) {
     LOG(ERROR) <<
         "Can't create debug log file: " << file_path.AsUTF8Unsafe() << ", " <<
-        "error: " << file.error_details();
+        "error: " << file->error_details();
     return;
   }
   chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->GetDebugLogs(
-      file.GetPlatformFile(),
-      base::Bind(&WriteDebugLogToFileCompleted,
-                 callback, Passed(&file), file_path));
+      file->Pass(),
+      base::Bind(&WriteDebugLogToFileCompleted, callback, file_path));
 }
 
 // Stores debug logs in the .tgz archive on the |fileshelf|. The file
@@ -320,12 +260,20 @@ void StoreDebugLogs(const base::FilePath& fileshelf,
                     const StoreDebugLogsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!callback.is_null());
-  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(&WriteDebugLogToFile, callback)), false);
+
+  const base::FilePath::CharType kLogFileName[] =
+      FILE_PATH_LITERAL("debug-log.tgz");
+
+  base::FilePath file_path = fileshelf.Append(kLogFileName);
+  file_path = logging::GenerateTimestampedName(file_path, base::Time::Now());
+
+  int flags =  base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE;
+  base::File* file = new base::File;
+  bool posted = BrowserThread::PostBlockingPoolTaskAndReply(
+      FROM_HERE,
+      base::Bind(&base::File::Initialize,
+                 base::Unretained(file), file_path, flags),
+      base::Bind(&WriteDebugLogToFile, callback, base::Owned(file), file_path));
   DCHECK(posted);
 }
 #endif  // defined(OS_CHROMEOS)

chromeos/dbus/debug_daemon_client.cc

@@ -12,10 +12,10 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/files/file_path.h"
 #include "base/location.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/message_loop/message_loop.h"
-#include "base/platform_file.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/string_util.h"
 #include "base/task_runner_util.h"
@@ -46,10 +46,11 @@ class DebugDaemonClientImpl : public DebugDaemonClient {
   virtual ~DebugDaemonClientImpl() {}
 
   // DebugDaemonClient override.
-  virtual void GetDebugLogs(base::PlatformFile file,
+  virtual void GetDebugLogs(base::File file,
                             const GetDebugLogsCallback& callback) OVERRIDE {
 
-    dbus::FileDescriptor* file_descriptor = new dbus::FileDescriptor(file);
+    dbus::FileDescriptor* file_descriptor = new dbus::FileDescriptor;
+    file_descriptor->PutValue(file.TakePlatformFile());
     // Punt descriptor validity check to a worker thread; on return we'll
     // issue the D-Bus request to stop tracing and collect results.
     base::WorkerPool::PostTaskAndReply(

chromeos/dbus/debug_daemon_client.h

@@ -6,8 +6,8 @@
 #define CHROMEOS_DBUS_DEBUG_DAEMON_CLIENT_H_
 
 #include "base/callback.h"
+#include "base/files/file.h"
 #include "base/memory/ref_counted_memory.h"
-#include "base/platform_file.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client.h"
 
@@ -30,7 +30,7 @@ class CHROMEOS_EXPORT DebugDaemonClient : public DBusClient {
 
   // Requests to store debug logs into |file| and calls |callback|
   // when completed. Debug logs will be stored in the .tgz format.
-  virtual void GetDebugLogs(base::PlatformFile file,
+  virtual void GetDebugLogs(base::File file,
                             const GetDebugLogsCallback& callback) = 0;
 
   // Called once SetDebugMode() is complete. Takes one parameter:

chromeos/dbus/fake_debug_daemon_client.cc

@@ -21,7 +21,7 @@ FakeDebugDaemonClient::~FakeDebugDaemonClient() {}
 
 void FakeDebugDaemonClient::Init(dbus::Bus* bus) {}
 
-void FakeDebugDaemonClient::GetDebugLogs(base::PlatformFile file,
+void FakeDebugDaemonClient::GetDebugLogs(base::File file,
                                          const GetDebugLogsCallback& callback) {
   callback.Run(false);
 }

chromeos/dbus/fake_debug_daemon_client.h

@@ -19,7 +19,7 @@ class FakeDebugDaemonClient : public DebugDaemonClient {
   virtual ~FakeDebugDaemonClient();
 
   virtual void Init(dbus::Bus* bus) OVERRIDE;
-  virtual void GetDebugLogs(base::PlatformFile file,
+  virtual void GetDebugLogs(base::File file,
                             const GetDebugLogsCallback& callback) OVERRIDE;
   virtual void SetDebugMode(const std::string& subsystem,
                             const SetDebugModeCallback& callback) OVERRIDE;