Rename SingleLogSource to SingleLogFileLogSource

Since there is now a class SingleDebugDaemonLogSource, the old
name "SingleLogSource" is ambiguous. Rename it to indicate that the
logs come from a log file.

Also refactored the use of SingleLogSource in feedbackPrivate API:
- Use the base interface class SystemLogsSource instead, in
  LogSourceAccessManager.
- Convert from feedback_private::LogSource directly to a
  SystemLogSource in SingleLogSourceFactory. Conversion of enums is
  internal to SingleLogSourceFactory.

BUG=733846

Review-Url: https://codereview.chromium.org/2956513004
Cr-Commit-Position: refs/heads/master@{#485283}

chrome/browser/chromeos/BUILD.gn

@@ -1439,8 +1439,8 @@ source_set("chromeos") {
     "system_logs/lsb_release_log_source.h",
     "system_logs/single_debug_daemon_log_source.cc",
     "system_logs/single_debug_daemon_log_source.h",
-    "system_logs/single_log_source.cc",
-    "system_logs/single_log_source.h",
+    "system_logs/single_log_file_log_source.cc",
+    "system_logs/single_log_file_log_source.h",
     "system_logs/touch_log_source.h",
     "system_logs/touch_log_source_ozone.cc",
     "system_logs/touch_log_source_x11.cc",
@@ -1870,7 +1870,7 @@ source_set("unit_tests") {
     "system/automatic_reboot_manager_unittest.cc",
     "system/device_disabling_manager_unittest.cc",
     "system_logs/single_debug_daemon_log_source_unittest.cc",
-    "system_logs/single_log_source_unittest.cc",
+    "system_logs/single_log_file_log_source_unittest.cc",
     "tether/tether_service_unittest.cc",
     "ui/accessibility_focus_ring_controller_unittest.cc",
     "ui/idle_app_name_notification_view_unittest.cc",

chrome/browser/chromeos/system_logs/single_log_source.cc → chrome/browser/chromeos/system_logs/single_log_file_log_source.cc

@@ -2,7 +2,7 @@
 // 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 "chrome/browser/chromeos/system_logs/single_log_file_log_source.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ -34,13 +34,13 @@ const base::Time* g_chrome_start_time_for_test = nullptr;
 // base system log directory path. In the future, if non-file source types are
 // added, this function should return an empty file path.
 base::FilePath GetLogFileSourceRelativeFilePath(
-    SingleLogSource::SupportedSource source) {
+    SingleLogFileLogSource::SupportedSource source) {
   switch (source) {
-    case SingleLogSource::SupportedSource::kMessages:
+    case SingleLogFileLogSource::SupportedSource::kMessages:
       return base::FilePath("messages");
-    case SingleLogSource::SupportedSource::kUiLatest:
+    case SingleLogFileLogSource::SupportedSource::kUiLatest:
       return base::FilePath("ui/ui.LATEST");
-    case SingleLogSource::SupportedSource::kAtrusLog:
+    case SingleLogFileLogSource::SupportedSource::kAtrusLog:
       return base::FilePath("atrus.log");
   }
   NOTREACHED();
@@ -134,7 +134,7 @@ size_t GetFirstFileOffsetWithTime(const base::FilePath& path,
 
 }  // namespace
 
-SingleLogSource::SingleLogSource(SupportedSource source_type)
+SingleLogFileLogSource::SingleLogFileLogSource(SupportedSource source_type)
     : SystemLogsSource(GetLogFileSourceRelativeFilePath(source_type).value()),
       source_type_(source_type),
       log_file_dir_path_(kDefaultSystemLogDirPath),
@@ -142,15 +142,15 @@ SingleLogSource::SingleLogSource(SupportedSource source_type)
       file_inode_(0),
       weak_ptr_factory_(this) {}
 
-SingleLogSource::~SingleLogSource() {}
+SingleLogFileLogSource::~SingleLogFileLogSource() {}
 
 // static
-void SingleLogSource::SetChromeStartTimeForTesting(
+void SingleLogFileLogSource::SetChromeStartTimeForTesting(
     const base::Time* start_time) {
   g_chrome_start_time_for_test = start_time;
 }
 
-void SingleLogSource::Fetch(const SysLogsSourceCallback& callback) {
+void SingleLogFileLogSource::Fetch(const SysLogsSourceCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!callback.is_null());
 
@@ -158,17 +158,18 @@ void SingleLogSource::Fetch(const SysLogsSourceCallback& callback) {
   base::PostTaskWithTraitsAndReply(
       FROM_HERE,
       base::TaskTraits(base::MayBlock(), base::TaskPriority::BACKGROUND),
-      base::Bind(&SingleLogSource::ReadFile, weak_ptr_factory_.GetWeakPtr(),
+      base::Bind(&SingleLogFileLogSource::ReadFile,
+                 weak_ptr_factory_.GetWeakPtr(),
                  kMaxNumAllowedLogRotationsDuringFileRead, response),
       base::Bind(callback, base::Owned(response)));
 }
 
-base::FilePath SingleLogSource::GetLogFilePath() const {
+base::FilePath SingleLogFileLogSource::GetLogFilePath() const {
   return base::FilePath(log_file_dir_path_).Append(source_name());
 }
 
-void SingleLogSource::ReadFile(size_t num_rotations_allowed,
-                               SystemLogsResponse* result) {
+void SingleLogFileLogSource::ReadFile(size_t num_rotations_allowed,
+                                      SystemLogsResponse* result) {
   // Attempt to open the file if it was not previously opened.
   if (!file_.IsValid()) {
     file_.Initialize(GetLogFilePath(),

chrome/browser/chromeos/system_logs/single_log_source.h → chrome/browser/chromeos/system_logs/single_log_file_log_source.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROMEOS_SYSTEM_LOGS_SINGLE_LOG_SOURCE_H_
-#define CHROME_BROWSER_CHROMEOS_SYSTEM_LOGS_SINGLE_LOG_SOURCE_H_
+#ifndef CHROME_BROWSER_CHROMEOS_SYSTEM_LOGS_SINGLE_LOG_FILE_LOG_SOURCE_H_
+#define CHROME_BROWSER_CHROMEOS_SYSTEM_LOGS_SINGLE_LOG_FILE_LOG_SOURCE_H_
 
 #include <stddef.h>
 #include <sys/types.h>
@@ -21,7 +21,7 @@ class Time;
 namespace system_logs {
 
 // Gathers log data from a single source, possibly incrementally.
-class SingleLogSource : public SystemLogsSource {
+class SingleLogFileLogSource : public SystemLogsSource {
  public:
   enum class SupportedSource {
     // For /var/log/messages.
@@ -34,8 +34,8 @@ class SingleLogSource : public SystemLogsSource {
     kAtrusLog,
   };
 
-  explicit SingleLogSource(SupportedSource source);
-  ~SingleLogSource() override;
+  explicit SingleLogFileLogSource(SupportedSource source);
+  ~SingleLogFileLogSource() override;
 
   // During testing, use this to set a custom Chrome start time to override the
   // actual start time. Does not take ownership of |start_time|. Call this again
@@ -46,7 +46,7 @@ class SingleLogSource : public SystemLogsSource {
   void Fetch(const SysLogsSourceCallback& callback) override;
 
  private:
-  friend class SingleLogSourceTest;
+  friend class SingleLogFileLogSourceTest;
 
   // Returns the full path of the log file.
   base::FilePath GetLogFilePath() const;
@@ -85,11 +85,11 @@ class SingleLogSource : public SystemLogsSource {
   // For removing PII from log results.
   feedback::AnonymizerTool anonymizer_;
 
-  base::WeakPtrFactory<SingleLogSource> weak_ptr_factory_;
+  base::WeakPtrFactory<SingleLogFileLogSource> weak_ptr_factory_;
 
-  DISALLOW_COPY_AND_ASSIGN(SingleLogSource);
+  DISALLOW_COPY_AND_ASSIGN(SingleLogFileLogSource);
 };
 
 }  // namespace system_logs
 
-#endif  // CHROME_BROWSER_CHROMEOS_SYSTEM_LOGS_SINGLE_LOG_SOURCE_H_
+#endif  // CHROME_BROWSER_CHROMEOS_SYSTEM_LOGS_SINGLE_LOG_FILE_LOG_SOURCE_H_

chrome/browser/extensions/api/feedback_private/feedback_private_api_chromeos_unittest.cc

@@ -18,12 +18,12 @@ namespace extensions {
 
 namespace {
 
+using api::feedback_private::LogSource;
 using api::feedback_private::ReadLogSourceResult;
 using api::feedback_private::ReadLogSourceParams;
 using base::TimeDelta;
-using system_logs::SingleLogSource;
 using system_logs::SystemLogsResponse;
-using SupportedSource = system_logs::SingleLogSource::SupportedSource;
+using system_logs::SystemLogsSource;
 
 std::unique_ptr<KeyedService> ApiResourceManagerTestFactory(
     content::BrowserContext* context) {
@@ -41,12 +41,12 @@ std::string ParamsToJSON(const ReadLogSourceParams& params) {
   return params_json_string;
 }
 
-// A dummy SingleLogSource that does not require real system logs to be
+// A dummy SystemLogsSource that does not require real system logs to be
 // available during testing.
-class TestSingleLogSource : public SingleLogSource {
+class TestSingleLogSource : public SystemLogsSource {
  public:
-  explicit TestSingleLogSource(SupportedSource type)
-      : SingleLogSource(type), call_count_(0) {}
+  explicit TestSingleLogSource(LogSource type)
+      : SystemLogsSource(ToString(type)), call_count_(0) {}
 
   ~TestSingleLogSource() override = default;
 
@@ -66,7 +66,7 @@ class TestSingleLogSource : public SingleLogSource {
 
     // Do not directly pass the result to the callback, because that's not how
     // log sources actually work. Instead, simulate the asynchronous operation
-    // of a SingleLogSource by invoking the callback separately.
+    // of a SystemLogsSource by invoking the callback separately.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::BindOnce(callback, base::Owned(result_map)));
   }
@@ -74,7 +74,7 @@ class TestSingleLogSource : public SingleLogSource {
   // Instantiates a new instance of this class. Does not retain ownership. Used
   // to create a Callback that can be used to override the default behavior of
   // SingleLogSourceFactory.
-  static std::unique_ptr<SingleLogSource> Create(SupportedSource type) {
+  static std::unique_ptr<SystemLogsSource> Create(LogSource type) {
     return base::MakeUnique<TestSingleLogSource>(type);
   }
 

chrome/browser/extensions/api/feedback_private/log_source_access_manager.cc

@@ -20,9 +20,7 @@ namespace extensions {
 namespace {
 
 namespace feedback_private = api::feedback_private;
-using feedback_private::LogSource;
-using SingleLogSource = system_logs::SingleLogSource;
-using SupportedSource = system_logs::SingleLogSource::SupportedSource;
+
 using SystemLogsResponse = system_logs::SystemLogsResponse;
 
 const int kMaxReadersPerSource = 10;
@@ -40,22 +38,6 @@ base::TimeDelta GetMinTimeBetweenReads() {
              : base::TimeDelta::FromMilliseconds(kDefaultRateLimitingTimeoutMs);
 }
 
-// Converts from feedback_private::LogSource to SupportedSource.
-SupportedSource GetSupportedSourceType(LogSource source) {
-  switch (source) {
-    case feedback_private::LOG_SOURCE_MESSAGES:
-      return SupportedSource::kMessages;
-    case feedback_private::LOG_SOURCE_UILATEST:
-      return SupportedSource::kUiLatest;
-    case feedback_private::LOG_SOURCE_NONE:
-    default:
-      NOTREACHED() << "Unknown log source type.";
-      return SingleLogSource::SupportedSource::kMessages;
-  }
-  NOTREACHED();
-  return SingleLogSource::SupportedSource::kMessages;
-}
-
 // SystemLogsResponse is a map of strings -> strings. The map value has the
 // actual log contents, a string containing all lines, separated by newlines.
 // This function extracts the individual lines and converts them into a vector
@@ -155,7 +137,7 @@ void LogSourceAccessManager::RemoveSource(const SourceAndExtension& key) {
 }
 
 LogSourceAccessManager::SourceAndExtension::SourceAndExtension(
-    api::feedback_private::LogSource source,
+    feedback_private::LogSource source,
     const std::string& extension_id)
     : source(source), extension_id(extension_id) {}
 
@@ -173,8 +155,7 @@ int LogSourceAccessManager::CreateResource(const SourceAndExtension& key) {
   std::unique_ptr<LogSourceResource> new_resource =
       base::MakeUnique<LogSourceResource>(
           key.extension_id,
-          SingleLogSourceFactory::CreateSingleLogSource(
-              GetSupportedSourceType(key.source)),
+          SingleLogSourceFactory::CreateSingleLogSource(key.source),
           base::Bind(&LogSourceAccessManager::RemoveSource,
                      weak_factory_.GetWeakPtr(), key));
 
@@ -206,7 +187,7 @@ base::TimeTicks LogSourceAccessManager::GetLastExtensionAccessTime(
 }
 
 size_t LogSourceAccessManager::GetNumActiveResourcesForSource(
-    api::feedback_private::LogSource source) const {
+    feedback_private::LogSource source) const {
   size_t count = 0;
   // The stored entries are sorted first by source type, then by extension ID.
   // We can take advantage of this fact to avoid iterating over all elements.

chrome/browser/extensions/api/feedback_private/log_source_access_manager_chromeos_unittest.cc

@@ -19,10 +19,10 @@ namespace {
 
 using api::feedback_private::LOG_SOURCE_MESSAGES;
 using api::feedback_private::LOG_SOURCE_UILATEST;
+using api::feedback_private::LogSource;
 using api::feedback_private::ReadLogSourceResult;
 using api::feedback_private::ReadLogSourceParams;
-using system_logs::SingleLogSource;
-using SupportedSource = system_logs::SingleLogSource::SupportedSource;
+using system_logs::SystemLogsSource;
 
 std::unique_ptr<KeyedService> ApiResourceManagerTestFactory(
     content::BrowserContext* context) {
@@ -32,11 +32,12 @@ std::unique_ptr<KeyedService> ApiResourceManagerTestFactory(
 // Dummy function used as a callback for FetchFromSource().
 void OnFetchedFromSource(const ReadLogSourceResult& result) {}
 
-// A dummy SingleLogSource that does not require real system logs to be
+// A dummy SystemLogsSource that does not require real system logs to be
 // available during testing. Always returns an empty result.
-class EmptySingleLogSource : public SingleLogSource {
+class EmptySingleLogSource : public system_logs::SystemLogsSource {
  public:
-  explicit EmptySingleLogSource(SupportedSource type) : SingleLogSource(type) {}
+  explicit EmptySingleLogSource(LogSource type)
+      : SystemLogsSource(api::feedback_private::ToString(type)) {}
 
   ~EmptySingleLogSource() override = default;
 
@@ -47,7 +48,7 @@ class EmptySingleLogSource : public SingleLogSource {
 
     // Do not directly pass the result to the callback, because that's not how
     // log sources actually work. Instead, simulate the asynchronous operation
-    // of a SingleLogSource by invoking the callback separately.
+    // of a SystemLogsSource by invoking the callback separately.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::BindOnce(callback, base::Owned(result_map)));
   }
@@ -55,7 +56,7 @@ class EmptySingleLogSource : public SingleLogSource {
   // Instantiates a new instance of this class. Does not retain ownership. Used
   // to create a Callback that can be used to override the default behavior of
   // SingleLogSourceFactory.
-  static std::unique_ptr<SingleLogSource> Create(SupportedSource type) {
+  static std::unique_ptr<SystemLogsSource> Create(LogSource type) {
     return base::MakeUnique<EmptySingleLogSource>(type);
   }
 

chrome/browser/extensions/api/feedback_private/log_source_resource.cc

@@ -22,7 +22,7 @@ ApiResourceManager<LogSourceResource>::GetFactoryInstance() {
 
 LogSourceResource::LogSourceResource(
     const std::string& extension_id,
-    std::unique_ptr<system_logs::SingleLogSource> source,
+    std::unique_ptr<system_logs::SystemLogsSource> source,
     base::Closure unregister_callback)
     : ApiResource(extension_id),
       source_(source.release()),

chrome/browser/extensions/api/feedback_private/log_source_resource.h

@@ -9,14 +9,14 @@
 
 #include "base/callback_helpers.h"
 #include "base/macros.h"
-#include "chrome/browser/chromeos/system_logs/single_log_source.h"
 #include "chrome/browser/extensions/api/feedback_private/log_source_access_manager.h"
+#include "chrome/browser/feedback/system_logs/system_logs_fetcher.h"
 #include "extensions/browser/api/api_resource.h"
 #include "extensions/browser/api/api_resource_manager.h"
 
 namespace extensions {
 
-// Holds a SingleLogSource object that is used by an extension using the
+// Holds a SystemLogsSource object that is used by an extension using the
 // feedbackPrivate API.
 class LogSourceResource : public ApiResource {
  public:
@@ -24,18 +24,18 @@ class LogSourceResource : public ApiResource {
       content::BrowserThread::UI;
 
   LogSourceResource(const std::string& extension_id,
-                    std::unique_ptr<system_logs::SingleLogSource> source,
+                    std::unique_ptr<system_logs::SystemLogsSource> source,
                     base::Closure unregister_callback_);
 
   ~LogSourceResource() override;
 
-  system_logs::SingleLogSource* GetLogSource() const { return source_.get(); }
+  system_logs::SystemLogsSource* GetLogSource() const { return source_.get(); }
 
  private:
   friend class ApiResourceManager<LogSourceResource>;
   static const char* service_name() { return "LogSourceResource"; }
 
-  std::unique_ptr<system_logs::SingleLogSource> source_;
+  std::unique_ptr<system_logs::SystemLogsSource> source_;
 
   // This unregisters the LogSourceResource from a LogSourceAccessManager when
   // this resource is cleaned up. Just pass in a base::Closure to the

chrome/browser/extensions/api/feedback_private/single_log_source_factory.cc

@@ -5,23 +5,40 @@
 #include "chrome/browser/extensions/api/feedback_private/single_log_source_factory.h"
 
 #include "base/memory/ptr_util.h"
+#include "chrome/browser/chromeos/system_logs/single_log_file_log_source.h"
 
 namespace extensions {
 
 namespace {
 
-using system_logs::SingleLogSource;
+namespace feedback_private = api::feedback_private;
+
+using system_logs::SingleLogFileLogSource;
+using system_logs::SystemLogsSource;
 
 SingleLogSourceFactory::CreateCallback* g_callback = nullptr;
 
 }  // namespace
 
 // static
-std::unique_ptr<SingleLogSource> SingleLogSourceFactory::CreateSingleLogSource(
-    SingleLogSource::SupportedSource type) {
+std::unique_ptr<SystemLogsSource> SingleLogSourceFactory::CreateSingleLogSource(
+    feedback_private::LogSource source_type) {
   if (g_callback)
-    return g_callback->Run(type);
-  return base::MakeUnique<SingleLogSource>(type);
+    return g_callback->Run(source_type);
+
+  switch (source_type) {
+    case feedback_private::LOG_SOURCE_MESSAGES:
+      return base::MakeUnique<system_logs::SingleLogFileLogSource>(
+          SingleLogFileLogSource::SupportedSource::kMessages);
+    case feedback_private::LOG_SOURCE_UILATEST:
+      return base::MakeUnique<system_logs::SingleLogFileLogSource>(
+          SingleLogFileLogSource::SupportedSource::kUiLatest);
+    case feedback_private::LOG_SOURCE_NONE:
+    default:
+      NOTREACHED() << "Unknown log source type.";
+      break;
+  }
+  return std::unique_ptr<SystemLogsSource>(nullptr);
 }
 
 // static

chrome/browser/extensions/api/feedback_private/single_log_source_factory.h

@@ -8,22 +8,23 @@
 #include <memory>
 
 #include "base/callback.h"
-#include "chrome/browser/chromeos/system_logs/single_log_source.h"
+#include "chrome/browser/feedback/system_logs/system_logs_fetcher.h"
+#include "chrome/common/extensions/api/feedback_private.h"
 
 namespace extensions {
 
-// Provides a way to override the creation of a new SingleLogSource during
+// Provides a way to override the creation of a new Single*LogSource during
 // testing.
 class SingleLogSourceFactory {
  public:
   using CreateCallback =
-      base::Callback<std::unique_ptr<system_logs::SingleLogSource>(
-          system_logs::SingleLogSource::SupportedSource)>;
+      base::Callback<std::unique_ptr<system_logs::SystemLogsSource>(
+          api::feedback_private::LogSource)>;
 
-  // Returns a SingleLogSource with source type of |type|. The caller must take
-  // ownership of the returned object.
-  static std::unique_ptr<system_logs::SingleLogSource> CreateSingleLogSource(
-      system_logs::SingleLogSource::SupportedSource type);
+  // Returns a Single*LogSource with source type corresponding to |type|. The
+  // caller must takeownership of the returned object.
+  static std::unique_ptr<system_logs::SystemLogsSource> CreateSingleLogSource(
+      api::feedback_private::LogSource type);
 
   // Pass in a callback that gets executed instead of the default behavior of
   // CreateSingleLogSource. Does not take ownership of |callback|. When done