Cleanup nits in ChromeOS system log code.

Change-Id: I2ad9adf9a3ca63a83a4a2f0991fabeac5f3cc182
Reviewed-on: https://chromium-review.googlesource.com/686274Reviewed-by: Polina Bondarenko <pbond@chromium.org>
Reviewed-by: Simon Que <sque@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505268}

chrome/browser/chromeos/policy/system_log_uploader.cc

@@ -26,12 +26,15 @@
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "net/http/http_request_headers.h"
 
+namespace policy {
+
 namespace {
+
 // The maximum number of successive retries.
 const int kMaxNumRetries = 1;
 
 // String constant defining the url tail we upload system logs to.
-const char* kSystemLogUploadUrlTail = "/upload";
+constexpr char kSystemLogUploadUrlTail[] = "/upload";
 
 // The cutoff point (in bytes) after which log contents are ignored.
 const size_t kLogCutoffSize = 50 * 1024 * 1024;  // 50 MiB.
@@ -61,16 +64,15 @@ std::string ReadAndAnonymizeLogFile(feedback::AnonymizerTool* anonymizer,
     data.erase(pos != std::string::npos ? pos + 1 : 0);
     data += "... [truncated]\n";
   }
-  return policy::SystemLogUploader::RemoveSensitiveData(anonymizer, data);
+  return SystemLogUploader::RemoveSensitiveData(anonymizer, data);
 }
 
 // Reads the system log files as binary files, anonymizes data, stores the files
 // as pairs (file name, data) and returns. Called on blocking thread.
-std::unique_ptr<policy::SystemLogUploader::SystemLogs> ReadFiles() {
-  std::unique_ptr<policy::SystemLogUploader::SystemLogs> system_logs(
-      new policy::SystemLogUploader::SystemLogs());
+std::unique_ptr<SystemLogUploader::SystemLogs> ReadFiles() {
+  auto system_logs = std::make_unique<SystemLogUploader::SystemLogs>();
   feedback::AnonymizerTool anonymizer;
-  for (auto* file_path : kSystemLogFileNames) {
+  for (const char* file_path : kSystemLogFileNames) {
     if (!base::PathExists(base::FilePath(file_path)))
       continue;
     system_logs->push_back(std::make_pair(
@@ -82,7 +84,7 @@ std::unique_ptr<policy::SystemLogUploader::SystemLogs> ReadFiles() {
 
 // An implementation of the |SystemLogUploader::Delegate|, that is used to
 // create an upload job and load system logs from the disk.
-class SystemLogDelegate : public policy::SystemLogUploader::Delegate {
+class SystemLogDelegate : public SystemLogUploader::Delegate {
  public:
   explicit SystemLogDelegate(
       scoped_refptr<base::SequencedTaskRunner> task_runner);
@@ -91,9 +93,9 @@ class SystemLogDelegate : public policy::SystemLogUploader::Delegate {
   // SystemLogUploader::Delegate:
   void LoadSystemLogs(const LogUploadCallback& upload_callback) override;
 
-  std::unique_ptr<policy::UploadJob> CreateUploadJob(
+  std::unique_ptr<UploadJob> CreateUploadJob(
       const GURL& upload_url,
-      policy::UploadJob::Delegate* delegate) override;
+      UploadJob::Delegate* delegate) override;
 
  private:
   // TaskRunner used for scheduling upload the upload task.
@@ -117,9 +119,9 @@ void SystemLogDelegate::LoadSystemLogs(
       base::Bind(&ReadFiles), upload_callback);
 }
 
-std::unique_ptr<policy::UploadJob> SystemLogDelegate::CreateUploadJob(
+std::unique_ptr<UploadJob> SystemLogDelegate::CreateUploadJob(
     const GURL& upload_url,
-    policy::UploadJob::Delegate* delegate) {
+    UploadJob::Delegate* delegate) {
   chromeos::DeviceOAuth2TokenService* device_oauth2_token_service =
       chromeos::DeviceOAuth2TokenServiceFactory::Get();
 
@@ -129,17 +131,17 @@ std::unique_ptr<policy::UploadJob> SystemLogDelegate::CreateUploadJob(
       device_oauth2_token_service->GetRobotAccountId();
 
   SYSLOG(INFO) << "Creating upload job for system log";
-  return std::unique_ptr<policy::UploadJob>(new policy::UploadJobImpl(
+  return std::make_unique<UploadJobImpl>(
       upload_url, robot_account_id, device_oauth2_token_service,
       system_request_context, delegate,
-      base::WrapUnique(new policy::UploadJobImpl::RandomMimeBoundaryGenerator),
-      task_runner_));
+      std::make_unique<UploadJobImpl::RandomMimeBoundaryGenerator>(),
+      task_runner_);
 }
 
 // Returns the system log upload frequency.
 base::TimeDelta GetUploadFrequency() {
   base::TimeDelta upload_frequency(base::TimeDelta::FromMilliseconds(
-      policy::SystemLogUploader::kDefaultUploadDelayMs));
+      SystemLogUploader::kDefaultUploadDelayMs));
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSystemLogUploadFrequency)) {
     std::string string_value =
@@ -154,14 +156,12 @@ base::TimeDelta GetUploadFrequency() {
 }
 
 std::string GetUploadUrl() {
-  return policy::BrowserPolicyConnector::GetDeviceManagementUrl() +
+  return BrowserPolicyConnector::GetDeviceManagementUrl() +
          kSystemLogUploadUrlTail;
 }
 
 }  // namespace
 
-namespace policy {
-
 // Determines the time between log uploads.
 const int64_t SystemLogUploader::kDefaultUploadDelayMs =
     12 * 60 * 60 * 1000;  // 12 hours
@@ -193,7 +193,7 @@ SystemLogUploader::SystemLogUploader(
       upload_enabled_(false),
       weak_factory_(this) {
   if (!syslog_delegate_)
-    syslog_delegate_.reset(new SystemLogDelegate(task_runner));
+    syslog_delegate_ = std::make_unique<SystemLogDelegate>(task_runner);
   DCHECK(syslog_delegate_);
   SYSLOG(INFO) << "Creating system log uploader.";
 
@@ -250,7 +250,7 @@ void SystemLogUploader::OnFailure(UploadJob::ErrorCode error_code) {
 
 // static
 std::string SystemLogUploader::RemoveSensitiveData(
-    feedback::AnonymizerTool* const anonymizer,
+    feedback::AnonymizerTool* anonymizer,
     const std::string& data) {
   return anonymizer->Anonymize(data);
 }
@@ -264,18 +264,17 @@ void SystemLogUploader::RefreshUploadSettings() {
   // If trusted values are not available, register this function to be called
   // back when they are available.
   chromeos::CrosSettings* settings = chromeos::CrosSettings::Get();
-  if (chromeos::CrosSettingsProvider::TRUSTED !=
-      settings->PrepareTrustedValues(
-          base::Bind(&SystemLogUploader::RefreshUploadSettings,
-                     weak_factory_.GetWeakPtr()))) {
+  auto trust_status = settings->PrepareTrustedValues(base::Bind(
+      &SystemLogUploader::RefreshUploadSettings, weak_factory_.GetWeakPtr()));
+  if (trust_status != chromeos::CrosSettingsProvider::TRUSTED)
     return;
-  }
 
   // CrosSettings are trusted - we want to use the last trusted values, by
   // default do not upload system logs.
   if (!settings->GetBoolean(chromeos::kSystemLogUploadEnabled,
-                            &upload_enabled_))
+                            &upload_enabled_)) {
     upload_enabled_ = false;
+  }
 }
 
 void SystemLogUploader::UploadSystemLogs(

chrome/browser/chromeos/policy/system_log_uploader.h

@@ -37,7 +37,12 @@ class SystemLogUploader : public UploadJob::Delegate {
  public:
   // Structure that stores the system log files as pairs: (file name, loaded
   // from the disk binary file data).
-  typedef std::vector<std::pair<std::string, std::string>> SystemLogs;
+  using SystemLogs = std::vector<std::pair<std::string, std::string>>;
+
+  // Remove lines from |data| that contain common PII (IP addresses, BSSIDs,
+  // SSIDs, URLs, e-mail addresses).
+  static std::string RemoveSensitiveData(feedback::AnonymizerTool* anonymizer,
+                                         const std::string& data);
 
   // Refresh constants.
   static const int64_t kDefaultUploadDelayMs;
@@ -53,8 +58,8 @@ class SystemLogUploader : public UploadJob::Delegate {
   // from the disk and create an upload job.
   class Delegate {
    public:
-    typedef base::Callback<void(std::unique_ptr<SystemLogs> system_logs)>
-        LogUploadCallback;
+    using LogUploadCallback =
+        base::Callback<void(std::unique_ptr<SystemLogs> system_logs)>;
 
     virtual ~Delegate() {}
 
@@ -70,7 +75,7 @@ class SystemLogUploader : public UploadJob::Delegate {
 
   // Constructor. Callers can inject their own Delegate. A nullptr can be passed
   // for |syslog_delegate| to use the default implementation.
-  explicit SystemLogUploader(
+  SystemLogUploader(
       std::unique_ptr<Delegate> syslog_delegate,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner);
 
@@ -80,20 +85,14 @@ class SystemLogUploader : public UploadJob::Delegate {
   // ever happened.
   base::Time last_upload_attempt() const { return last_upload_attempt_; }
 
+  void ScheduleNextSystemLogUploadImmediately();
+
   // UploadJob::Delegate:
   // Callbacks handle success and failure results of upload, destroy the
   // upload job.
   void OnSuccess() override;
   void OnFailure(UploadJob::ErrorCode error_code) override;
 
-  // Remove lines from |data| that contain common PII (IP addresses, BSSIDs,
-  // SSIDs, URLs, e-mail addresses).
-  static std::string RemoveSensitiveData(
-      feedback::AnonymizerTool* const anonymizer,
-      const std::string& data);
-
-  void ScheduleNextSystemLogUploadImmediately();
-
  private:
   // Updates the system log upload enabled field from settings.
   void RefreshUploadSettings();

chrome/browser/chromeos/system_logs/single_log_file_log_source.cc

@@ -35,28 +35,28 @@ const base::Time* g_chrome_start_time_for_test = nullptr;
 // Converts a logs source type to the corresponding file path, relative to the
 // 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(
+base::FilePath::StringType GetLogFileSourceRelativeFilePathValue(
     SingleLogFileLogSource::SupportedSource source) {
   switch (source) {
     case SupportedSource::kMessages:
-      return base::FilePath("messages");
+      return "messages";
     case SupportedSource::kUiLatest:
-      return base::FilePath("ui/ui.LATEST");
+      return "ui/ui.LATEST";
     case SupportedSource::kAtrusLog:
-      return base::FilePath("atrus.log");
+      return "atrus.log";
     case SupportedSource::kNetLog:
-      return base::FilePath("net.log");
+      return "net.log";
     case SupportedSource::kEventLog:
-      return base::FilePath("eventlog.txt");
+      return "eventlog.txt";
     case SupportedSource::kUpdateEngineLog:
-      return base::FilePath("update_engine.log");
+      return "update_engine.log";
     case SupportedSource::kPowerdLatest:
-      return base::FilePath("power_manager/powerd.LATEST");
+      return "power_manager/powerd.LATEST";
     case SupportedSource::kPowerdPrevious:
-      return base::FilePath("power_manager/powerd.PREVIOUS");
+      return "power_manager/powerd.PREVIOUS";
   }
   NOTREACHED();
-  return base::FilePath();
+  return base::FilePath::StringType();
 }
 
 // Returns the inode value of file at |path|, or 0 if it doesn't exist or is
@@ -168,7 +168,7 @@ size_t GetFirstFileOffsetWithTime(const base::FilePath& path,
 }  // namespace
 
 SingleLogFileLogSource::SingleLogFileLogSource(SupportedSource source_type)
-    : SystemLogsSource(GetLogFileSourceRelativeFilePath(source_type).value()),
+    : SystemLogsSource(GetLogFileSourceRelativeFilePathValue(source_type)),
       source_type_(source_type),
       log_file_dir_path_(kDefaultSystemLogDirPath),
       num_bytes_read_(0),
@@ -187,18 +187,19 @@ void SingleLogFileLogSource::Fetch(const SysLogsSourceCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!callback.is_null());
 
-  SystemLogsResponse* response = new SystemLogsResponse;
+  auto response = std::make_unique<SystemLogsResponse>();
+  auto* response_ptr = response.get();
   base::PostTaskWithTraitsAndReply(
       FROM_HERE,
       base::TaskTraits(base::MayBlock(), base::TaskPriority::BACKGROUND),
       base::Bind(&SingleLogFileLogSource::ReadFile,
                  weak_ptr_factory_.GetWeakPtr(),
-                 kMaxNumAllowedLogRotationsDuringFileRead, response),
-      base::Bind(callback, base::Owned(response)));
+                 kMaxNumAllowedLogRotationsDuringFileRead, response_ptr),
+      base::Bind(callback, base::Owned(response.release())));
 }
 
 base::FilePath SingleLogFileLogSource::GetLogFilePath() const {
-  return base::FilePath(log_file_dir_path_).Append(source_name());
+  return log_file_dir_path_.Append(source_name());
 }
 
 void SingleLogFileLogSource::ReadFile(size_t num_rotations_allowed,