Additional methods for chrome.logPrivate API

chrome/browser/chromeos/system_logs/debug_log_writer.cc

+// Copyright (c) 2014 The Chromium Authors. All rights reserved.

[Zachary Kuznia, 2014/06/25 00:05:53]

nit: no (c)

[zel, 2014/06/27 19:31:00]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/system_logs/debug_log_writer.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/file_util.h"
+#include "base/files/file.h"
+#include "chrome/common/logging_chrome.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/debug_daemon_client.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace chromeos {
+
+namespace {
+
+// Called upon completion of |WriteDebugLogToFile|. Closes file
+// descriptor, deletes log file in the case of failure and calls
+// |callback|.
+void WriteDebugLogToFileCompleted(
+    const DebugLogWriter::StoreDebugLogsCallback& callback,
+    const base::FilePath& file_path,
+    bool succeeded) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!succeeded) {
+    bool posted = content::BrowserThread::PostBlockingPoolTaskAndReply(
+        FROM_HERE,
+        base::Bind(base::IgnoreResult(&base::DeleteFile), file_path, false),
+        base::Bind(callback, file_path, false));
+    DCHECK(posted);
+    return;
+  }
+  callback.Run(file_path, true);
+}
+
+// Stores into |file_path| debug logs in the .tgz format. Calls
+// |callback| upon completion.
+void WriteDebugLogToFile(
+    const DebugLogWriter::StoreDebugLogsCallback& callback,
+    base::File* file,
+    const base::FilePath& file_path) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!file->IsValid()) {
+    LOG(ERROR) <<

[Zachary Kuznia, 2014/06/25 00:05:53]

I think we currently prefer VLOG(1)

[zel, 2014/06/27 19:31:00]

VLOG(1) gets recorded nowhere. This is really an error that indicates serious
issues.

+        "Can't create debug log file: " << file_path.AsUTF8Unsafe() << ", " <<
+        "error: " << file->error_details();
+    return;
+  }
+  chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->GetDebugLogs(
+      file->Pass(),
+      base::Bind(&WriteDebugLogToFileCompleted, callback, file_path));
+}
+
+}  // namespace
+
+// static.
+void DebugLogWriter::StoreLogs(
+    const base::FilePath& fileshelf,
+    const StoreDebugLogsCallback& callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(!callback.is_null());
+
+  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;

[xiaowenx, 2014/06/25 05:30:44]

Two spaces here after "=", intentional?

[zel, 2014/06/27 19:31:00]

Done.

+  base::File* file = new base::File;

[Zachary Kuznia, 2014/06/25 00:05:53]

Is there a reason not to make this a scoped_ptr?  I think WriteDebugLogToFile
should at least take a scoped_ptr, so that the ownership is clear.

+  bool posted = content::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);
+}
+
+}  // namespace chromeos