Lacros: Direct stdout/err from lacros to lacros.log.

BrowserManager will launch lacros-chrome process with stdout/err redirected to
/home/chronos/user/lacros/lacros.log. Logs will stop being written directly to
the log file and will be written to stderr.

Test: Deploy ash-chrome and observe /home/chronos/user/lacros/lacros.log and /var/log/chrome/chrome

Bug: 1121031
Change-Id: I977ec48d745f87f32b2b57bfd9127094c38145e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2371443
Commit-Queue: Yuta Hijikata <ythjkt@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802164}

chrome/browser/chromeos/crosapi/browser_manager.cc

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/chromeos/crosapi/browser_manager.h"
 
+#include <fcntl.h>
+#include <unistd.h>
+
 #include <string>
 #include <utility>
 #include <vector>
@@ -15,6 +18,7 @@
 #include "base/logging.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
+#include "base/posix/eintr_wrapper.h"
 #include "base/process/launch.h"
 #include "base/process/process_handle.h"
 #include "base/strings/string_number_conversions.h"
@@ -52,6 +56,29 @@ base::FilePath LacrosLogPath() {
   return browser_util::GetUserDataDir().Append("lacros.log");
 }
 
+base::ScopedFD CreateLogFile() {
+  base::FilePath::StringType log_path = LacrosLogPath().value();
+
+  // Delete old log file if exists.
+  if (unlink(log_path.c_str()) != 0) {
+    if (errno != ENOENT) {
+      // unlink() failed for reason other than the file not existing.
+      PLOG(ERROR) << "Failed to unlink the log file " << log_path;
+      return base::ScopedFD();
+    }
+  }
+
+  int fd =
+      HANDLE_EINTR(open(log_path.c_str(), O_WRONLY | O_CREAT | O_EXCL, 644));
+
+  if (fd < 0) {
+    PLOG(ERROR) << "Failed to get file descriptor for " << log_path;
+    return base::ScopedFD();
+  }
+
+  return base::ScopedFD(fd);
+}
+
 std::string GetXdgRuntimeDir() {
   // If ash-chrome was given an environment variable, use it.
   std::unique_ptr<base::Environment> env = base::Environment::Create();
@@ -146,11 +173,14 @@ void BrowserManager::NewWindow() {
     return;
   }
 
+  if (state_ == State::CREATING_LOG_FILE) {
+    LOG(WARNING) << "lacros-chrome is in the process of launching";
+    return;
+  }
+
   if (state_ == State::STOPPED) {
     // If lacros-chrome is not running, launch it.
-    bool succeeded = Start();
-    LOG_IF(ERROR, !succeeded)
-        << "lacros-chrome failed to launch. Cannot open a window";
+    Start();
     return;
   }
 
@@ -158,10 +188,21 @@ void BrowserManager::NewWindow() {
   lacros_chrome_service_->NewWindow(base::DoNothing());
 }
 
-bool BrowserManager::Start() {
+void BrowserManager::Start() {
   DCHECK_EQ(state_, State::STOPPED);
   DCHECK(!lacros_path_.empty());
 
+  state_ = State::CREATING_LOG_FILE;
+
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE, base::BindOnce(&CreateLogFile),
+      base::BindOnce(&BrowserManager::StartWithLogFile,
+                     weak_factory_.GetWeakPtr()));
+}
+
+void BrowserManager::StartWithLogFile(base::ScopedFD logfd) {
+  DCHECK_EQ(state_, State::CREATING_LOG_FILE);
+
   std::string chrome_path = lacros_path_.MaybeAsASCII() + "/chrome";
   LOG(WARNING) << "Launching lacros-chrome at " << chrome_path;
 
@@ -210,9 +251,17 @@ bool BrowserManager::Start() {
     argv.push_back(flag);
   }
 
-  // Always enable logging.
-  argv.push_back("--enable-logging");
-  argv.push_back("--log-file=" + LacrosLogPath().value());
+  // If logfd is valid, enable logging and redirect stdout/stderr to logfd.
+  if (logfd.is_valid()) {
+    // The next flag will make chrome log only via stderr. See
+    // DetermineLoggingDestination in logging_chrome.cc.
+    argv.push_back("--enable-logging=stderr");
+
+    // These options will assign stdout/stderr fds to logfd in the fd table of
+    // the new process.
+    options.fds_to_remap.push_back(std::make_pair(logfd.get(), STDOUT_FILENO));
+    options.fds_to_remap.push_back(std::make_pair(logfd.get(), STDERR_FILENO));
+  }
 
   // Set up Mojo channel.
   base::CommandLine command_line(argv);
@@ -243,7 +292,8 @@ bool BrowserManager::Start() {
   lacros_process_ = base::LaunchProcess(command_line, options);
   if (!lacros_process_.IsValid()) {
     LOG(ERROR) << "Failed to launch lacros-chrome";
-    return false;
+    state_ = State::STOPPED;
+    return;
   }
   state_ = State::STARTING;
   LOG(WARNING) << "Launched lacros-chrome with pid " << lacros_process_.Pid();
@@ -253,7 +303,6 @@ bool BrowserManager::Start() {
   mojo::OutgoingInvitation::Send(std::move(invitation),
                                  lacros_process_.Handle(),
                                  channel.TakeLocalEndpoint());
-  return true;
 }
 
 void BrowserManager::OnAshChromeServiceReceiverReceived(

chrome/browser/chromeos/crosapi/browser_manager.h

@@ -52,10 +52,11 @@ class BrowserManager : public session_manager::SessionManagerObserver {
   void SetLoadCompleteCallback(LoadCompleteCallback callback);
 
   // Opens the browser window in lacros-chrome.
-  // If lacros-chrome is not yet launched, it triggers to launch.
-  // This needs to be called after loading. The condition can be checked
-  // IsReady(), and if not yet, SetLoadCompletionCallback can be used
-  // to wait for the loading.
+  // If lacros-chrome is not yet launched, it triggers to launch. If this is
+  // called again during the setup phase of the launch process, it will be
+  // ignored. This needs to be called after loading. The condition can be
+  // checked IsReady(), and if not yet, SetLoadCompletionCallback can be used to
+  // wait for the loading.
   // TODO(crbug.com/1101676): Notify callers the result of opening window
   // request. Because of asynchronous operations crossing processes,
   // there's no guarantee that the opening window request succeeds.
@@ -83,6 +84,9 @@ class BrowserManager : public session_manager::SessionManagerObserver {
     // Lacros-chrome is loaded and ready for launching.
     STOPPED,
 
+    // Lacros-chrome is creating a new log file to log to.
+    CREATING_LOG_FILE,
+
     // Lacros-chrome is launching.
     STARTING,
 
@@ -94,10 +98,12 @@ class BrowserManager : public session_manager::SessionManagerObserver {
     TERMINATING,
   };
 
-  // Starts the lacros-chrome process. Returns whether the subprocess is
-  // created. Note that the subprocess may be crashed immediately, even if this
-  // returns true. This can be called only in STOPPED state.
-  bool Start();
+  // Posts CreateLogFile() and StartWithLogFile() to the thread pooll.
+  void Start();
+
+  // Starts the lacros-chrome process and redirects stdout/err to file pointed
+  // by logfd.
+  void StartWithLogFile(base::ScopedFD logfd);
 
   // Called when PendingReceiver of AshChromeService is passed from
   // lacros-chrome.