feat: bring --enable-logging functionality in line with Chromium (#25089)

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

shell/app/electron_main_delegate.cc

@@ -32,6 +32,7 @@
 #include "shell/browser/relauncher.h"
 #include "shell/common/application_info.h"
 #include "shell/common/electron_paths.h"
+#include "shell/common/logging.h"
 #include "shell/common/options_switches.h"
 #include "shell/common/platform_util.h"
 #include "shell/renderer/electron_renderer_client.h"
@@ -71,6 +72,10 @@ namespace {
 
 const char* kRelauncherProcess = "relauncher";
 
+constexpr base::StringPiece kElectronDisableSandbox("ELECTRON_DISABLE_SANDBOX");
+constexpr base::StringPiece kElectronEnableStackDumping(
+    "ELECTRON_ENABLE_STACK_DUMPING");
+
 bool IsBrowserProcess(base::CommandLine* cmd) {
   std::string process_type = cmd->GetSwitchValueASCII(::switches::kProcessType);
   return process_type.empty();
@@ -236,7 +241,6 @@ const size_t ElectronMainDelegate::kNonWildcardDomainNonPortSchemesSize =
 bool ElectronMainDelegate::BasicStartupComplete(int* exit_code) {
   auto* command_line = base::CommandLine::ForCurrentProcess();
 
-  logging::LoggingSettings settings;
 #if defined(OS_WIN)
   v8_crashpad_support::SetUp();
 
@@ -244,43 +248,16 @@ bool ElectronMainDelegate::BasicStartupComplete(int* exit_code) {
   // prevent output in the same line as the prompt.
   if (IsBrowserProcess(command_line))
     std::wcout << std::endl;
-#if defined(DEBUG)
-  // Print logging to debug.log on Windows
-  settings.logging_dest = logging::LOG_TO_ALL;
-  base::FilePath log_filename;
-  base::PathService::Get(base::DIR_EXE, &log_filename);
-  log_filename = log_filename.AppendASCII("debug.log");
-  settings.log_file_path = log_filename.value().c_str();
-  settings.lock_log = logging::LOCK_LOG_FILE;
-  settings.delete_old = logging::DELETE_OLD_LOG_FILE;
-#else
-  settings.logging_dest =
-      logging::LOG_TO_SYSTEM_DEBUG_LOG | logging::LOG_TO_STDERR;
-#endif  // defined(DEBUG)
-#else   // defined(OS_WIN)
-  settings.logging_dest =
-      logging::LOG_TO_SYSTEM_DEBUG_LOG | logging::LOG_TO_STDERR;
 #endif  // !defined(OS_WIN)
 
-  // Only enable logging when --enable-logging is specified.
   auto env = base::Environment::Create();
-  if (!command_line->HasSwitch(::switches::kEnableLogging) &&
-      !env->HasVar("ELECTRON_ENABLE_LOGGING")) {
-    settings.logging_dest = logging::LOG_NONE;
-    logging::SetMinLogLevel(logging::LOGGING_NUM_SEVERITIES);
-  }
-
-  logging::InitLogging(settings);
-
-  // Logging with pid and timestamp.
-  logging::SetLogItems(true, false, true, false);
 
   // Enable convenient stack printing. This is enabled by default in
   // non-official builds.
-  if (env->HasVar("ELECTRON_ENABLE_STACK_DUMPING"))
+  if (env->HasVar(kElectronEnableStackDumping))
     base::debug::EnableInProcessStackDumping();
 
-  if (env->HasVar("ELECTRON_DISABLE_SANDBOX"))
+  if (env->HasVar(kElectronDisableSandbox))
     command_line->AppendSwitch(sandbox::policy::switches::kNoSandbox);
 
   tracing_sampler_profiler_ =
@@ -353,6 +330,19 @@ void ElectronMainDelegate::PreSandboxStartup() {
                                                  user_data_dir, false, true);
   }
 
+#if !defined(OS_WIN)
+  // For windows we call InitLogging later, after the sandbox is initialized.
+  //
+  // On Linux, we force a "preinit" in the zygote (i.e. never log to a default
+  // log file), because the zygote is booted prior to JS running, so it can't
+  // know the correct user-data directory. (And, further, accessing the
+  // application name on Linux can cause glib calls that end up spawning
+  // threads, which if done before the zygote is booted, causes a CHECK().)
+  logging::InitElectronLogging(*command_line,
+                               /* is_preinit = */ process_type.empty() ||
+                                   process_type == ::switches::kZygoteProcess);
+#endif
+
 #if !defined(MAS_BUILD)
   crash_reporter::InitializeCrashKeys();
 #endif
@@ -400,6 +390,13 @@ void ElectronMainDelegate::PreSandboxStartup() {
   }
 }
 
+void ElectronMainDelegate::SandboxInitialized(const std::string& process_type) {
+#if defined(OS_WIN)
+  logging::InitElectronLogging(*base::CommandLine::ForCurrentProcess(),
+                               /* is_preinit = */ process_type.empty());
+#endif
+}
+
 void ElectronMainDelegate::PreBrowserMain() {
   // This is initialized early because the service manager reads some feature
   // flags and we need to make sure the feature list is initialized before the

shell/app/electron_main_delegate.h

@@ -30,6 +30,7 @@ class ElectronMainDelegate : public content::ContentMainDelegate {
   // content::ContentMainDelegate:
   bool BasicStartupComplete(int* exit_code) override;
   void PreSandboxStartup() override;
+  void SandboxInitialized(const std::string& process_type) override;
   void PreBrowserMain() override;
   content::ContentBrowserClient* CreateContentBrowserClient() override;
   content::ContentGpuClient* CreateContentGpuClient() override;

shell/browser/electron_browser_client.cc

@@ -100,6 +100,7 @@
 #include "shell/common/api/api.mojom.h"
 #include "shell/common/application_info.h"
 #include "shell/common/electron_paths.h"
+#include "shell/common/logging.h"
 #include "shell/common/options_switches.h"
 #include "shell/common/platform_util.h"
 #include "third_party/blink/public/common/loader/url_loader_throttle.h"
@@ -542,6 +543,15 @@ void ElectronBrowserClient::AppendExtraCommandLineSwitches(
   }
 #endif
 
+  // The zygote process is booted before JS runs, so DIR_USER_DATA isn't usable
+  // at that time. It doesn't need --user-data-dir to be correct anyway, since
+  // the zygote itself doesn't access anything in that directory.
+  if (process_type != ::switches::kZygoteProcess) {
+    base::FilePath user_data_dir;
+    if (base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
+      command_line->AppendSwitchPath(::switches::kUserDataDir, user_data_dir);
+  }
+
   if (process_type == ::switches::kUtilityProcess ||
       process_type == ::switches::kRendererProcess) {
     // Copy following switches to child process.
@@ -794,6 +804,11 @@ bool ElectronBrowserClient::ArePersistentMediaDeviceIDsAllowed(
   return true;
 }
 
+base::FilePath ElectronBrowserClient::GetLoggingFileName(
+    const base::CommandLine& cmd_line) {
+  return logging::GetLogFileName(cmd_line);
+}
+
 void ElectronBrowserClient::SiteInstanceDeleting(
     content::SiteInstance* site_instance) {
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)

shell/browser/electron_browser_client.h

@@ -263,6 +263,7 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
       const GURL& scope,
       const GURL& site_for_cookies,
       const absl::optional<url::Origin>& top_frame_origin) override;
+  base::FilePath GetLoggingFileName(const base::CommandLine& cmd_line) override;
 
   // content::RenderProcessHostObserver:
   void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;

shell/browser/electron_browser_context.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/barrier_closure.h"
+#include "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/no_destructor.h"

shell/browser/electron_browser_main_parts.cc

@@ -45,6 +45,7 @@
 #include "shell/common/application_info.h"
 #include "shell/common/electron_paths.h"
 #include "shell/common/gin_helper/trackable_object.h"
+#include "shell/common/logging.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -269,6 +270,11 @@ void ElectronBrowserMainParts::PostEarlyInitialization() {
   // Initialize field trials.
   InitializeFieldTrials();
 
+  // Reinitialize logging now that the app has had a chance to set the app name
+  // and/or user data directory.
+  logging::InitElectronLogging(*base::CommandLine::ForCurrentProcess(),
+                               /* is_preinit = */ false);
+
   // Initialize after user script environment creation.
   fake_browser_process_->PostEarlyInitialization();
 }

shell/common/api/electron_api_testing.cc

@@ -0,0 +1,48 @@
+// Copyright (c) 2021 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "base/dcheck_is_on.h"
+#include "base/logging.h"
+#include "shell/common/gin_helper/dictionary.h"
+#include "shell/common/node_includes.h"
+#include "v8/include/v8.h"
+
+#if DCHECK_IS_ON()
+namespace {
+
+void Log(int severity, std::string text) {
+  switch (severity) {
+    case logging::LOGGING_VERBOSE:
+      VLOG(1) << text;
+      break;
+    case logging::LOGGING_INFO:
+      LOG(INFO) << text;
+      break;
+    case logging::LOGGING_WARNING:
+      LOG(WARNING) << text;
+      break;
+    case logging::LOGGING_ERROR:
+      LOG(ERROR) << text;
+      break;
+    case logging::LOGGING_FATAL:
+      LOG(FATAL) << text;
+      break;
+    default:
+      LOG(ERROR) << "Unrecognized severity: " << severity;
+      break;
+  }
+}
+
+void Initialize(v8::Local<v8::Object> exports,
+                v8::Local<v8::Value> unused,
+                v8::Local<v8::Context> context,
+                void* priv) {
+  gin_helper::Dictionary dict(context->GetIsolate(), exports);
+  dict.SetMethod("log", &Log);
+}
+
+}  // namespace
+
+NODE_LINKED_MODULE_CONTEXT_AWARE(electron_common_testing, Initialize)
+#endif

shell/common/logging.cc

@@ -0,0 +1,141 @@
+// Copyright (c) 2021 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/common/logging.h"
+
+#include <string>
+
+#include "base/base_switches.h"
+#include "base/command_line.h"
+#include "base/environment.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/common/chrome_paths.h"
+#include "content/public/common/content_switches.h"
+#include "shell/common/electron_paths.h"
+
+namespace logging {
+
+constexpr base::StringPiece kLogFileName("ELECTRON_LOG_FILE");
+constexpr base::StringPiece kElectronEnableLogging("ELECTRON_ENABLE_LOGGING");
+
+base::FilePath GetLogFileName(const base::CommandLine& command_line) {
+  std::string filename = command_line.GetSwitchValueASCII(switches::kLogFile);
+  if (filename.empty())
+    base::Environment::Create()->GetVar(kLogFileName, &filename);
+  if (!filename.empty())
+    return base::FilePath::FromUTF8Unsafe(filename);
+
+  const base::FilePath log_filename(FILE_PATH_LITERAL("electron_debug.log"));
+  base::FilePath log_path;
+
+  if (base::PathService::Get(chrome::DIR_LOGS, &log_path)) {
+    log_path = log_path.Append(log_filename);
+    return log_path;
+  } else {
+    // error with path service, just use some default file somewhere
+    return log_filename;
+  }
+}
+
+bool HasExplicitLogFile(const base::CommandLine& command_line) {
+  std::string filename = command_line.GetSwitchValueASCII(switches::kLogFile);
+  if (filename.empty())
+    base::Environment::Create()->GetVar(kLogFileName, &filename);
+  return !filename.empty();
+}
+
+LoggingDestination DetermineLoggingDestination(
+    const base::CommandLine& command_line,
+    bool is_preinit) {
+  bool enable_logging = false;
+  std::string logging_destination;
+  if (command_line.HasSwitch(::switches::kEnableLogging)) {
+    enable_logging = true;
+    logging_destination =
+        command_line.GetSwitchValueASCII(switches::kEnableLogging);
+  } else {
+    auto env = base::Environment::Create();
+    if (env->HasVar(kElectronEnableLogging)) {
+      enable_logging = true;
+      env->GetVar(kElectronEnableLogging, &logging_destination);
+    }
+  }
+  if (!enable_logging)
+    return LOG_NONE;
+
+  // --enable-logging logs to stderr, --enable-logging=file logs to a file.
+  // NB. this differs from Chromium, in which --enable-logging logs to a file
+  // and --enable-logging=stderr logs to stderr, because that's how Electron
+  // used to work, so in order to not break anyone who was depending on
+  // --enable-logging logging to stderr, we preserve the old behavior by
+  // default.
+  // If --log-file or ELECTRON_LOG_FILE is specified along with
+  // --enable-logging, return LOG_TO_FILE.
+  // If we're in the pre-init phase, before JS has run, we want to avoid
+  // logging to the default log file, which is inside the user data directory,
+  // because we aren't able to accurately determine the user data directory
+  // before JS runs. Instead, log to stderr unless there's an explicit filename
+  // given.
+  if (HasExplicitLogFile(command_line) ||
+      (logging_destination == "file" && !is_preinit))
+    return LOG_TO_FILE;
+  return LOG_TO_SYSTEM_DEBUG_LOG | LOG_TO_STDERR;
+}
+
+void InitElectronLogging(const base::CommandLine& command_line,
+                         bool is_preinit) {
+  const std::string process_type =
+      command_line.GetSwitchValueASCII(::switches::kProcessType);
+  LoggingDestination logging_dest =
+      DetermineLoggingDestination(command_line, is_preinit);
+  LogLockingState log_locking_state = LOCK_LOG_FILE;
+  base::FilePath log_path;
+
+  if (command_line.HasSwitch(::switches::kLoggingLevel) &&
+      GetMinLogLevel() >= 0) {
+    std::string log_level =
+        command_line.GetSwitchValueASCII(::switches::kLoggingLevel);
+    int level = 0;
+    if (base::StringToInt(log_level, &level) && level >= 0 &&
+        level < LOGGING_NUM_SEVERITIES) {
+      SetMinLogLevel(level);
+    } else {
+      DLOG(WARNING) << "Bad log level: " << log_level;
+    }
+  }
+
+  // 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(command_line);
+  } else {
+    log_locking_state = DONT_LOCK_LOG_FILE;
+  }
+
+  // On Windows, having non canonical forward slashes in log file name causes
+  // problems with sandbox filters, see https://crbug.com/859676
+  log_path = log_path.NormalizePathSeparators();
+
+  LoggingSettings settings;
+  settings.logging_dest = logging_dest;
+  settings.log_file_path = log_path.value().c_str();
+  settings.lock_log = log_locking_state;
+  // If we're logging to an explicit file passed with --log-file, we don't want
+  // to delete the log file on our second initialization.
+  settings.delete_old =
+      process_type.empty() && (is_preinit || !HasExplicitLogFile(command_line))
+          ? DELETE_OLD_LOG_FILE
+          : APPEND_TO_OLD_LOG_FILE;
+  bool success = InitLogging(settings);
+  if (!success) {
+    PLOG(FATAL) << "Failed to init logging";
+  }
+
+  SetLogItems(true /* pid */, false, true /* timestamp */, false);
+}
+
+}  // namespace logging

shell/common/logging.h

@@ -0,0 +1,22 @@
+// Copyright (c) 2021 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_LOGGING_H_
+#define SHELL_COMMON_LOGGING_H_
+
+namespace base {
+class CommandLine;
+class FilePath;
+}  // namespace base
+
+namespace logging {
+
+void InitElectronLogging(const base::CommandLine& command_line,
+                         bool is_preinit);
+
+base::FilePath GetLogFileName(const base::CommandLine& command_line);
+
+}  // namespace logging
+
+#endif  // SHELL_COMMON_LOGGING_H_

shell/common/node_bindings.cc

@@ -88,6 +88,8 @@
 
 #define ELECTRON_DESKTOP_CAPTURER_MODULE(V) V(electron_browser_desktop_capturer)
 
+#define ELECTRON_TESTING_MODULE(V) V(electron_common_testing)
+
 // This is used to load built-in modules. Instead of using
 // __attribute__((constructor)), we call the _register_<modname>
 // function for each built-in modules explicitly. This is only
@@ -101,6 +103,9 @@ ELECTRON_VIEWS_MODULES(V)
 #if BUILDFLAG(ENABLE_DESKTOP_CAPTURER)
 ELECTRON_DESKTOP_CAPTURER_MODULE(V)
 #endif
+#if DCHECK_IS_ON()
+ELECTRON_TESTING_MODULE(V)
+#endif
 #undef V
 
 namespace {
@@ -329,6 +334,9 @@ void NodeBindings::RegisterBuiltinModules() {
 #if BUILDFLAG(ENABLE_DESKTOP_CAPTURER)
   ELECTRON_DESKTOP_CAPTURER_MODULE(V)
 #endif
+#if DCHECK_IS_ON()
+  ELECTRON_TESTING_MODULE(V)
+#endif
 #undef V
 }