Feedback: Provide list of user log files in Chrome directly

Chrome used to call into debugd to get the list of user log
files that it should read itself. Those files are generated
by Chrome itself, and hence they should be listed in Chrome
itself without the need to ask debugd to list them.

BUG=907193
TEST=File feedback report, make sure the following entries
     are part of the system logs:
* chrome_user_log,
* libassistant_user_log,
* login-times,
* logout-times.

Change-Id: I2e164db999edfc6e7089a0ae905cc6db57121621
Reviewed-on: https://chromium-review.googlesource.com/c/1352681Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615788}

chrome/browser/chromeos/system_logs/debug_daemon_log_source.cc

@@ -24,15 +24,51 @@
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_thread.h"
 
-const char kNotAvailable[] = "<not available>";
-const char kRoutesKeyName[] = "routes";
-const char kNetworkStatusKeyName[] = "network-status";
-const char kModemStatusKeyName[] = "modem-status";
-const char kWiMaxStatusKeyName[] = "wimax-status";
-const char kUserLogFileKeyName[] = "user_log_files";
-
 namespace system_logs {
 
+namespace {
+
+constexpr char kNotAvailable[] = "<not available>";
+constexpr char kRoutesKeyName[] = "routes";
+constexpr char kNetworkStatusKeyName[] = "network-status";
+constexpr char kModemStatusKeyName[] = "modem-status";
+constexpr char kWiMaxStatusKeyName[] = "wimax-status";
+
+// List of user log files that Chrome reads directly as these logs are generated
+// by Chrome itself.
+constexpr struct UserLogs {
+  // A string key used as a title for this log in feedback reports.
+  const char* log_key;
+
+  // The log file's path relative to the user's profile directory.
+  const char* log_file_relative_path;
+} kUserLogs[] = {
+    {"chrome_user_log", "log/chrome"},
+    {"libassistant_user_log", "log/libassistant.log"},
+    {"login-times", "login-times"},
+    {"logout-times", "logout-times"},
+};
+
+// Reads the contents of the user log files listed in |kUserLogs| and adds them
+// to the |response| parameter.
+void ReadUserLogFiles(const std::vector<base::FilePath>& profile_dirs,
+                      SystemLogsResponse* response) {
+  for (size_t i = 0; i < profile_dirs.size(); ++i) {
+    std::string profile_prefix = "Profile[" + base::UintToString(i) + "] ";
+    for (const auto& log : kUserLogs) {
+      std::string value;
+      const bool read_success = base::ReadFileToString(
+          profile_dirs[i].Append(log.log_file_relative_path), &value);
+
+      response->emplace(
+          profile_prefix + log.log_key,
+          (read_success && !value.empty()) ? std::move(value) : kNotAvailable);
+    }
+  }
+}
+
+}  // namespace
+
 DebugDaemonLogSource::DebugDaemonLogSource(bool scrub)
     : SystemLogsSource("DebugDemon"),
       response_(new SystemLogsResponse()),
@@ -122,69 +158,36 @@ void DebugDaemonLogSource::OnGetLogs(bool /* succeeded */,
   RequestCompleted();
 }
 
-void DebugDaemonLogSource::OnGetUserLogFiles(
-    bool succeeded,
-    const KeyValueMap& user_log_files) {
+void DebugDaemonLogSource::GetLoggedInUsersLogFiles() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (succeeded) {
-    auto response = std::make_unique<SystemLogsResponse>();
-    SystemLogsResponse* response_ptr = response.get();
-
-    const user_manager::UserList& users =
-        user_manager::UserManager::Get()->GetLoggedInUsers();
-    std::vector<base::FilePath> profile_dirs;
-    for (user_manager::UserList::const_iterator it = users.begin();
-         it != users.end();
-         ++it) {
-      if ((*it)->username_hash().empty())
-        continue;
-      profile_dirs.push_back(
-          chromeos::ProfileHelper::GetProfilePathByUserIdHash(
-              (*it)->username_hash()));
-    }
 
-    base::PostTaskWithTraitsAndReply(
-        FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
-        base::BindOnce(&DebugDaemonLogSource::ReadUserLogFiles, user_log_files,
-                       profile_dirs, response_ptr),
-        base::BindOnce(&DebugDaemonLogSource::MergeUserLogFilesResponse,
-                       weak_ptr_factory_.GetWeakPtr(), std::move(response)));
-  } else {
-    (*response_)[kUserLogFileKeyName] = kNotAvailable;
-    auto response = std::make_unique<SystemLogsResponse>();
-    std::swap(response, response_);
-    DCHECK(!callback_.is_null());
-    std::move(callback_).Run(std::move(response));
+  // List all logged-in users' profile directories.
+  std::vector<base::FilePath> profile_dirs;
+  const user_manager::UserList& users =
+      user_manager::UserManager::Get()->GetLoggedInUsers();
+  for (const auto* user : users) {
+    if (user->username_hash().empty())
+      continue;
+
+    profile_dirs.emplace_back(
+        chromeos::ProfileHelper::GetProfilePathByUserIdHash(
+            user->username_hash()));
   }
-}
 
-// static
-void DebugDaemonLogSource::ReadUserLogFiles(
-    const KeyValueMap& user_log_files,
-    const std::vector<base::FilePath>& profile_dirs,
-    SystemLogsResponse* response) {
-  for (size_t i = 0; i < profile_dirs.size(); ++i) {
-    std::string profile_prefix = "Profile[" + base::UintToString(i) + "] ";
-    for (KeyValueMap::const_iterator it = user_log_files.begin();
-         it != user_log_files.end();
-         ++it) {
-      std::string key = it->first;
-      std::string value;
-      std::string filename = it->second;
-      bool read_success = base::ReadFileToString(
-          profile_dirs[i].Append(filename), &value);
-
-      if (read_success && !value.empty())
-        (*response)[profile_prefix + key] = value;
-      else
-        (*response)[profile_prefix + filename] = kNotAvailable;
-    }
-  }
+  auto response = std::make_unique<SystemLogsResponse>();
+  SystemLogsResponse* response_ptr = response.get();
+  base::PostTaskWithTraitsAndReply(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
+      base::BindOnce(&ReadUserLogFiles, profile_dirs, response_ptr),
+      base::BindOnce(&DebugDaemonLogSource::MergeUserLogFilesResponse,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(response)));
 }
 
 void DebugDaemonLogSource::MergeUserLogFilesResponse(
     std::unique_ptr<SystemLogsResponse> response) {
-  response_->insert(response->begin(), response->end());
+  for (auto& pair : *response)
+    response_->emplace(pair.first, std::move(pair.second));
+
   auto response_to_return = std::make_unique<SystemLogsResponse>();
   std::swap(response_to_return, response_);
   DCHECK(!callback_.is_null());
@@ -198,12 +201,10 @@ void DebugDaemonLogSource::RequestCompleted() {
   --num_pending_requests_;
   if (num_pending_requests_ > 0)
     return;
+
   // When all other logs are collected, fetch the user logs, because any errors
   // fetching the other logs is reported in the user logs.
-  chromeos::DebugDaemonClient* client =
-      chromeos::DBusThreadManager::Get()->GetDebugDaemonClient();
-  client->GetUserLogFiles(base::Bind(&DebugDaemonLogSource::OnGetUserLogFiles,
-                                     weak_ptr_factory_.GetWeakPtr()));
+  GetLoggedInUsersLogFiles();
 }
 
 }  // namespace system_logs

chrome/browser/chromeos/system_logs/debug_daemon_log_source.h

@@ -40,16 +40,16 @@ class DebugDaemonLogSource : public SystemLogsSource {
   void OnGetWiMaxStatus(base::Optional<std::string> status);
   void OnGetLogs(bool succeeded,
                  const KeyValueMap& logs);
+
+  // Reads the logged-in users' log files that have to be read by Chrome as
+  // debugd has no access to them. The contents of these logs are appended to
+  // |response_|. This is called at the end when all debugd logs are collected
+  // so that we can see any debugd related errors surface in feedback reports.
+  void GetLoggedInUsersLogFiles();
+
   void OnGetUserLogFiles(bool succeeded,
                          const KeyValueMap& logs);
 
-  // Read the contents of the specified user logs files and adds it to
-  // the response parameter.
-  static void ReadUserLogFiles(
-      const KeyValueMap& user_log_files,
-      const std::vector<base::FilePath>& profile_dirs,
-      SystemLogsResponse* response);
-
   // Merge the responses from ReadUserLogFiles into the main response dict and
   // invoke the callback_.Run method with the assumption that all other logs
   // have already been collected.