Add SingleLogSource to system_logs sources

chrome/browser/chromeos/system_logs/single_log_source.cc

Index: chrome/browser/chromeos/system_logs/single_log_source.cc
diff --git a/chrome/browser/chromeos/system_logs/single_log_source.cc b/chrome/browser/chromeos/system_logs/single_log_source.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f1e460f0335f1d4b281a39f860811f6fd88c9bfa
--- /dev/null
+++ b/chrome/browser/chromeos/system_logs/single_log_source.cc
@@ -0,0 +1,101 @@
+// Copyright 2017 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/chromeos/system_logs/single_log_source.h"
+
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/task_scheduler/post_task.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace system_logs {
+
+namespace {
+
+// Defines a pair: log source name and log filename.
+struct LogSource {
+  const char* name;
+  const char* filename;
+};
+
+// Whitelist of log sources that SingleLogSource is permitted to access.
+const LogSource kLogFileSources[] = {
+    {"messages", "/var/log/messages"},
+    {"ui_latest", "/var/log/ui/ui.LATEST"},
+};
+
+// Given a valid log source name from |kLogFileSources|, returns the
+// corresponding log filename. If the source name is not in the whitelist,
+// returns an empty filename.
+std::string GetLogFileSourceFilename(const std::string& source_name) {

[afakhry, 2017/05/06 01:12:13]

This can now be a simple switch statement if we use enum class. The above struct
and kLogFileSources[] will not be needed.

You'd probably need another member variable to store source_name_.

[Simon Que, 2017/05/06 14:19:42]

Done.

+  for (const LogSource& source : kLogFileSources) {
+    if (source.name == source_name)
+      return source.filename;
+  }
+  return "";
+}
+
+}  // namespace
+
+SingleLogSource::SingleLogSource() : SystemLogsSource(""), num_bytes_read_(0) {}

[afakhry, 2017/05/06 01:12:13]

Please remove this default constructor. It seems useless. One can't modify the
object afterwards if this constructor was called.

[Simon Que, 2017/05/06 14:19:42]

Done.

+
+SingleLogSource::SingleLogSource(const std::string& source_name)
+    : SystemLogsSource(source_name), num_bytes_read_(0) {}
+
+SingleLogSource::~SingleLogSource() {}
+
+void SingleLogSource::Fetch(const SysLogsSourceCallback& callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(!callback.is_null());
+
+  SystemLogsResponse* response = new SystemLogsResponse;
+  base::PostTaskWithTraitsAndReply(
+      FROM_HERE,
+      base::TaskTraits().MayBlock().WithPriority(
+          base::TaskPriority::BACKGROUND),
+      base::Bind(&SingleLogSource::ReadFile, base::Unretained(this), response),
+      base::Bind(callback, base::Owned(response)));
+}
+
+void SingleLogSource::ReadFile(SystemLogsResponse* result) {
+  // Attempt to open the file if it was not previously opened.
+  if (!file_.IsValid()) {
+    file_.Initialize(base::FilePath(GetLogFileSourceFilename(source_name())),
+                     base::File::FLAG_OPEN | base::File::FLAG_READ);
+    if (!file_.IsValid())
+      return;
+  }
+
+  // Check for file size reset.
+  size_t length = file_.GetLength();

[afakhry, 2017/05/06 01:12:13]

Nit: const here and everywhere below please.

[Simon Que, 2017/05/06 14:19:42]

Done.

+  if (length < num_bytes_read_) {
+    num_bytes_read_ = 0;
+    file_.Seek(base::File::FROM_BEGIN, 0);
+  }
+
+  // Read from file until end.
+  size_t size_to_read = length - num_bytes_read_;
+  std::vector<char> read_result(size_to_read);
+  size_t size_read = file_.ReadAtCurrentPos(read_result.data(), size_to_read);
+  read_result.resize(size_read);
+
+  // Create a string from the read results.
+  std::string result_string;

[afakhry, 2017/05/06 01:12:13]

Why don't you read into the string right away and avoid the copying?

[Simon Que, 2017/05/06 14:19:43]

ReadAtCurrentPos() writes to a char*, but std::string does not expose a mutable
char*, only const char*.

[afakhry, 2017/05/08 19:20:47]

You can use &result_string[0]. C++17 should provide 

CharT* data(); [ http://en.cppreference.com/w/cpp/string/basic_string/data ]

But until then, you can use &string[0] as long as your sting is resized
properly. This way you're using the string as a wrapper for your buffer.

Please avoid unnecessary copying as much as possible. Those log files are big
and it doesn't make sense to copy them around.

[afakhry, 2017/05/09 23:46:18]

I see you missed this comment. Please take a look. not lgtm until then.

+
+  // The reader may only read complete lines.
+  if (!read_result.empty() && read_result.back() == '\n') {
+    result_string.reserve(read_result.size());
+    result_string.assign(read_result.begin(), read_result.end());
+    num_bytes_read_ += size_read;
+  } else {
+    // If an incomplete line was read, reset the file read offset to before the
+    // most recent read.
+    file_.Seek(base::File::FROM_CURRENT, -size_read);
+  }
+
+  // Pass it back to the callback.
+  (*result)[source_name()] = anonymizer_.Anonymize(result_string);

[afakhry, 2017/05/06 01:12:13]

result->emplace(source_name_, anonymizer_.Anonymize(result_string));

[Simon Que, 2017/05/06 14:19:43]

Done.

+}
+
+}  // namespace system_logs