Add --log-file=PATH switch to more executables

The --log-file switch lets users specify the logging location. It was
added as a generic flag and implemented in app_shell in
crrev.com/c/647314.

This CL expands the switch to other logging configurers, notably
including chrome and content_shell.

In Chrome, --log-file=<PATH> will override the $CHROME_LOG_FILE
environmental variable if both are present.

Bug: 760431
Change-Id: I72448b1a8cdea5b5818badd340594a2f9d6d8cd2
Reviewed-on: https://chromium-review.googlesource.com/685997Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505287}

chrome/browser/chrome_content_browser_client.cc

@@ -3467,8 +3467,9 @@ ChromeContentBrowserClient::GetTaskSchedulerInitParams() {
   return task_scheduler_util::GetBrowserTaskSchedulerInitParamsFromVariations();
 }
 
-base::FilePath ChromeContentBrowserClient::GetLoggingFileName() {
-  return logging::GetLogFileName();
+base::FilePath ChromeContentBrowserClient::GetLoggingFileName(
+    const base::CommandLine& command_line) {
+  return logging::GetLogFileName(command_line);
 }
 
 std::vector<std::unique_ptr<content::URLLoaderThrottle>>

chrome/browser/chrome_content_browser_client.h

@@ -343,7 +343,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
 
   std::unique_ptr<base::TaskScheduler::InitParams> GetTaskSchedulerInitParams()
       override;
-  base::FilePath GetLoggingFileName() override;
+  base::FilePath GetLoggingFileName(
+      const base::CommandLine& command_line) override;
   std::vector<std::unique_ptr<content::URLLoaderThrottle>>
   CreateURLLoaderThrottles(
       const base::Callback<content::WebContents*()>& wc_getter) override;

chrome/browser/chrome_content_browser_client_unittest.cc

@@ -346,9 +346,20 @@ TEST_F(InstantNTPURLRewriteTest, UberURLHandler_InstantExtendedNewTabPage) {
 class ChromeContentBrowserClientGetLoggingFileTest : public testing::Test {};
 
 TEST_F(ChromeContentBrowserClientGetLoggingFileTest, GetLoggingFile) {
+  base::CommandLine cmd_line(base::CommandLine::NO_PROGRAM);
   ChromeContentBrowserClient client;
   base::FilePath log_file_name;
-  EXPECT_FALSE(client.GetLoggingFileName().empty());
+  EXPECT_FALSE(client.GetLoggingFileName(cmd_line).empty());
+}
+
+TEST_F(ChromeContentBrowserClientGetLoggingFileTest,
+       GetLoggingFileFromCommandLine) {
+  base::CommandLine cmd_line(base::CommandLine::NO_PROGRAM);
+  cmd_line.AppendSwitchASCII(switches::kLogFile, "test_log.txt");
+  ChromeContentBrowserClient client;
+  base::FilePath log_file_name;
+  EXPECT_EQ(base::FilePath(FILE_PATH_LITERAL("test_log.txt")).value(),
+            client.GetLoggingFileName(cmd_line).value());
 }
 
 class TestChromeContentBrowserClient : public ChromeContentBrowserClient {

chrome/browser/logging_chrome_unittest.cc

@@ -4,10 +4,12 @@
 
 #include <memory>
 
+#include "base/command_line.h"
 #include "base/environment.h"
 #include "base/files/file_path.h"
 #include "chrome/common/env_vars.h"
 #include "chrome/common/logging_chrome.h"
+#include "content/public/common/content_switches.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class ChromeLoggingTest : public testing::Test {
@@ -29,15 +31,23 @@ class ChromeLoggingTest : public testing::Test {
     env->SetVar(env_vars::kLogFileName, environment_filename_);
   }
 
+  void SetLogFileFlag(const std::string& value) {
+    cmd_line_.AppendSwitchASCII(switches::kLogFile, value);
+  }
+
+  const base::CommandLine& cmd_line() { return cmd_line_; }
+
  private:
   std::string environment_filename_;  // Saves real environment value.
+  base::CommandLine cmd_line_ =
+      base::CommandLine(base::CommandLine::NO_PROGRAM);
 };
 
 // Tests the log file name getter without an environment variable.
 TEST_F(ChromeLoggingTest, LogFileName) {
   SaveEnvironmentVariable(std::string());
 
-  base::FilePath filename = logging::GetLogFileName();
+  base::FilePath filename = logging::GetLogFileName(cmd_line());
   ASSERT_NE(base::FilePath::StringType::npos,
             filename.value().find(FILE_PATH_LITERAL("chrome_debug.log")));
 
@@ -46,10 +56,32 @@ TEST_F(ChromeLoggingTest, LogFileName) {
 
 // Tests the log file name getter with an environment variable.
 TEST_F(ChromeLoggingTest, EnvironmentLogFileName) {
-  SaveEnvironmentVariable("test value");
+  SaveEnvironmentVariable("test env value");
+
+  base::FilePath filename = logging::GetLogFileName(cmd_line());
+  ASSERT_EQ(base::FilePath(FILE_PATH_LITERAL("test env value")).value(),
+            filename.value());
+
+  RestoreEnvironmentVariable();
+}
+
+// Tests the log file name getter with a command-line flag.
+TEST_F(ChromeLoggingTest, FlagLogFileName) {
+  SetLogFileFlag("test flag value");
+
+  base::FilePath filename = logging::GetLogFileName(cmd_line());
+  ASSERT_EQ(base::FilePath(FILE_PATH_LITERAL("test flag value")).value(),
+            filename.value());
+}
+
+// Tests the log file name getter with with an environment variable and a
+// command-line flag. The flag takes precedence.
+TEST_F(ChromeLoggingTest, EnvironmentAndFlagLogFileName) {
+  SaveEnvironmentVariable("test env value");
+  SetLogFileFlag("test flag value");
 
-  base::FilePath filename = logging::GetLogFileName();
-  ASSERT_EQ(base::FilePath(FILE_PATH_LITERAL("test value")).value(),
+  base::FilePath filename = logging::GetLogFileName(cmd_line());
+  ASSERT_EQ(base::FilePath(FILE_PATH_LITERAL("test flag value")).value(),
             filename.value());
 
   RestoreEnvironmentVariable();

chrome/chrome_watcher/chrome_watcher_main.cc

@@ -140,10 +140,11 @@ bool GetLogsPath(base::FilePath* result) {
 #endif  // NDEBUG
 }
 
-base::FilePath GetLogFileName() {
-  std::string filename;
-  std::unique_ptr<base::Environment> env(base::Environment::Create());
-  if (env->GetVar(env_vars::kLogFileName, &filename) && !filename.empty())
+base::FilePath GetLogFileName(const base::CommandLine& command_line) {
+  std::string filename = command_line.GetSwitchValueASCII(switches::kLogFile);
+  if (filename.empty())
+    base::Environment::Create()->GetVar(env_vars::kLogFileName, &filename);
+  if (!filename.empty())
     return base::FilePath::FromUTF8Unsafe(filename);
 
   const base::FilePath log_filename(FILE_PATH_LITERAL("chrome_debug.log"));
@@ -173,7 +174,7 @@ void InitChromeWatcherLogging(const base::CommandLine& command_line,
   // Don't resolve the log path unless we need to. Otherwise we leave an open
   // ALPC handle after sandbox lockdown on Windows.
   if ((logging_dest & LOG_TO_FILE) != 0) {
-    log_path = GetLogFileName();
+    log_path = GetLogFileName(command_line);
 
   } else {
     log_locking_state = DONT_LOCK_LOG_FILE;

chrome/common/logging_chrome.cc

@@ -252,7 +252,8 @@ base::FilePath GetSessionLogDir(const base::CommandLine& command_line) {
 }
 
 base::FilePath GetSessionLogFile(const base::CommandLine& command_line) {
-  return GetSessionLogDir(command_line).Append(GetLogFileName().BaseName());
+  return GetSessionLogDir(command_line)
+      .Append(GetLogFileName(command_line).BaseName());
 }
 
 #endif  // OS_CHROMEOS
@@ -271,7 +272,7 @@ void InitChromeLogging(const base::CommandLine& command_line,
   // Don't resolve the log path unless we need to. Otherwise we leave an open
   // ALPC handle after sandbox lockdown on Windows.
   if ((logging_dest & LOG_TO_FILE) != 0) {
-    log_path = GetLogFileName();
+    log_path = GetLogFileName(command_line);
 
 #if defined(OS_CHROMEOS)
     // For BWSI (Incognito) logins, we want to put the logs in the user
@@ -387,10 +388,11 @@ void CleanupChromeLogging() {
   chrome_logging_redirected_ = false;
 }
 
-base::FilePath GetLogFileName() {
-  std::string filename;
-  std::unique_ptr<base::Environment> env(base::Environment::Create());
-  if (env->GetVar(env_vars::kLogFileName, &filename) && !filename.empty())
+base::FilePath GetLogFileName(const base::CommandLine& command_line) {
+  std::string filename = command_line.GetSwitchValueASCII(switches::kLogFile);
+  if (filename.empty())
+    base::Environment::Create()->GetVar(env_vars::kLogFileName, &filename);
+  if (!filename.empty())
     return base::FilePath::FromUTF8Unsafe(filename);
 
   const base::FilePath log_filename(FILE_PATH_LITERAL("chrome_debug.log"));

chrome/common/logging_chrome.h

@@ -55,7 +55,7 @@ base::FilePath GetSessionLogFile(const base::CommandLine& command_line);
 void CleanupChromeLogging();
 
 // Returns the fully-qualified name of the log file.
-base::FilePath GetLogFileName();
+base::FilePath GetLogFileName(const base::CommandLine& command_line);
 
 // Returns true when error/assertion dialogs are not to be shown, false
 // otherwise.

content/browser/sandbox_parameters_mac.mm

@@ -70,7 +70,7 @@ void SetupRendererSandboxParameters(sandbox::SeatbeltExecClient* client) {
                              std::to_string(getpid())));
 
   std::string logging_path =
-      GetContentClient()->browser()->GetLoggingFileName().value();
+      GetContentClient()->browser()->GetLoggingFileName(*command_line).value();
   CHECK(client->SetParameter(Sandbox::kSandboxLoggingPathAsLiteral,
                              logging_path));
 

content/public/browser/content_browser_client.cc

@@ -171,7 +171,8 @@ const gfx::ImageSkia* ContentBrowserClient::GetDefaultFavicon() {
   return empty;
 }
 
-base::FilePath ContentBrowserClient::GetLoggingFileName() {
+base::FilePath ContentBrowserClient::GetLoggingFileName(
+    const base::CommandLine& command_line) {
   return base::FilePath();
 }
 

content/public/browser/content_browser_client.h

@@ -356,7 +356,8 @@ class CONTENT_EXPORT ContentBrowserClient {
 
   // Returns the fully qualified path to the log file name, or an empty path.
   // This function is used by the sandbox to allow write access to the log.
-  virtual base::FilePath GetLoggingFileName();
+  virtual base::FilePath GetLoggingFileName(
+      const base::CommandLine& command_line);
 
   // Allow the embedder to control if an AppCache can be used for the given url.
   // This is called on the IO thread.

content/shell/app/shell_main_delegate.cc

@@ -103,10 +103,16 @@ const GUID kContentShellProviderName = {
         { 0x84, 0x13, 0xec, 0x94, 0xd8, 0xc2, 0xa4, 0xb6 } };
 #endif
 
-void InitLogging() {
+void InitLogging(const base::CommandLine& command_line) {
   base::FilePath log_filename;
-  PathService::Get(base::DIR_EXE, &log_filename);
-  log_filename = log_filename.AppendASCII("content_shell.log");
+  std::string filename = command_line.GetSwitchValueASCII(switches::kLogFile);
+  if (filename.empty()) {
+    PathService::Get(base::DIR_EXE, &log_filename);
+    log_filename = log_filename.AppendASCII("content_shell.log");
+  } else {
+    log_filename = base::FilePath::FromUTF8Unsafe(filename);
+  }
+
   logging::LoggingSettings settings;
   settings.logging_dest = logging::LOG_TO_ALL;
   settings.log_file = log_filename.value().c_str();
@@ -149,7 +155,7 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
   EnsureCorrectResolutionSettings();
 #endif  // OS_MACOSX
 
-  InitLogging();
+  InitLogging(command_line);
   if (command_line.HasSwitch(switches::kCheckLayoutTestSysDeps)) {
     // If CheckLayoutSystemDeps succeeds, we don't exit early. Instead we
     // continue and try to load the fonts in BlinkTestPlatformInitialize