Define a LoggingSettings struct to use for InitLogging()

Update all callers of InitLogging() to use LoggingSettings, only setting fields that need a non-default value. Turn LoggingDestination enum into a bit field and define add LOG_DEFAULT and LOG_ALL constants. Fix erroneous comment saying that the default was to not lock the log file. BUG=247594 TBR=brettw@chromium.org, cpu@chromium.org, gene@chromium.org, jam@chromium.org, rch@chromium.org, scherkus@chromium.org, sergeyu@chromium.org, sky@chromium.org, tkent@chromium.org, yfriedman@chromium.org, zea@chromium.org Review URL: https://codereview.chromium.org/16519003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207920 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Jun 21, 2013 · 5e3f7c2 · 5e3f7c2
1 parent f25eb36
commit 5e3f7c2
Show file tree

Hide file tree

Showing 42 changed files with 251 additions and 309 deletions.
diff --git a/base/logging.cc b/base/logging.cc
@@ -85,15 +85,7 @@ const char* const log_severity_names[LOG_NUM_SEVERITIES] = {
 
 int min_log_level = 0;
 
-// The default set here for logging_destination will only be used if
-// InitLogging is not called.  On Windows, use a file next to the exe;
-// on POSIX platforms, where it may not even be possible to locate the
-// executable on disk, use stderr.
-#if defined(OS_WIN)
-LoggingDestination logging_destination = LOG_ONLY_TO_FILE;
-#elif defined(OS_POSIX)
-LoggingDestination logging_destination = LOG_ONLY_TO_SYSTEM_DEBUG_LOG;
-#endif
+LoggingDestination logging_destination = LOG_DEFAULT;
 
 // For LOG_ERROR and above, always print to stderr.
 const int kAlwaysPrintErrorLevel = LOG_ERROR;
@@ -323,8 +315,7 @@ bool InitializeLogFileHandle() {
     log_file_name = new PathString(GetDefaultLogFile());
   }
 
-  if (logging_destination == LOG_ONLY_TO_FILE ||
-      logging_destination == LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG) {
+  if ((logging_destination & LOG_TO_FILE) != 0) {
 #if defined(OS_WIN)
     log_file = CreateFile(log_file_name->c_str(), GENERIC_WRITE,
                           FILE_SHARE_READ | FILE_SHARE_WRITE, NULL,
@@ -352,17 +343,19 @@ bool InitializeLogFileHandle() {
 
 }  // namespace
 
+LoggingSettings::LoggingSettings()
+    : logging_dest(LOG_DEFAULT),
+      log_file(NULL),
+      lock_log(LOCK_LOG_FILE),
+      delete_old(APPEND_TO_OLD_LOG_FILE),
+      dcheck_state(DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS) {}
 
-bool BaseInitLoggingImpl(const PathChar* new_log_file,
-                         LoggingDestination logging_dest,
-                         LogLockingState lock_log,
-                         OldFileDeletionState delete_old,
-                         DcheckState dcheck_state) {
+bool BaseInitLoggingImpl(const LoggingSettings& settings) {
 #if defined(OS_NACL)
-  CHECK(logging_dest == LOG_NONE ||
-        logging_dest == LOG_ONLY_TO_SYSTEM_DEBUG_LOG);
+  // Can log only to the system debug log.
+  CHECK_EQ(settings.logging_dest & ~LOG_TO_SYSTEM_DEBUG_LOG, 0);
 #endif
-  g_dcheck_state = dcheck_state;
+  g_dcheck_state = settings.dcheck_state;
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   // Don't bother initializing g_vlog_info unless we use one of the
   // vlog switches.
@@ -380,7 +373,7 @@ bool BaseInitLoggingImpl(const PathChar* new_log_file,
                      &min_log_level);
   }
 
-  LoggingLock::Init(lock_log, new_log_file);
+  LoggingLock::Init(settings.lock_log, settings.log_file);
 
   LoggingLock logging_lock;
 
@@ -391,17 +384,16 @@ bool BaseInitLoggingImpl(const PathChar* new_log_file,
     log_file = NULL;
   }
 
-  logging_destination = logging_dest;
+  logging_destination = settings.logging_dest;
 
-  // ignore file options if logging is disabled or only to system
-  if (logging_destination == LOG_NONE ||
-      logging_destination == LOG_ONLY_TO_SYSTEM_DEBUG_LOG)
+  // ignore file options unless logging to file is set.
+  if ((logging_destination & LOG_TO_FILE) == 0)
     return true;
 
   if (!log_file_name)
     log_file_name = new PathString();
-  *log_file_name = new_log_file;
-  if (delete_old == DELETE_OLD_LOG_FILE)
+  *log_file_name = settings.log_file;
+  if (settings.delete_old == DELETE_OLD_LOG_FILE)
     DeleteFilePath(*log_file_name);
 
   return InitializeLogFileHandle();
@@ -578,14 +570,14 @@ LogMessage::~LogMessage() {
   std::string str_newline(stream_.str());
 
   // Give any log message handler first dibs on the message.
-  if (log_message_handler && log_message_handler(severity_, file_, line_,
-          message_start_, str_newline)) {
+  if (log_message_handler &&
+      log_message_handler(severity_, file_, line_,
+                          message_start_, str_newline)) {
     // The handler took care of it, no further processing.
     return;
   }
 
-  if (logging_destination == LOG_ONLY_TO_SYSTEM_DEBUG_LOG ||
-      logging_destination == LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG) {
+  if ((logging_destination & LOG_TO_SYSTEM_DEBUG_LOG) != 0) {
 #if defined(OS_WIN)
     OutputDebugStringA(str_newline.c_str());
 #elif defined(OS_ANDROID)
@@ -627,8 +619,7 @@ LogMessage::~LogMessage() {
   // thread at the beginning of execution.
   LoggingLock::Init(LOCK_LOG_FILE, NULL);
   // write to log file
-  if (logging_destination != LOG_NONE &&
-      logging_destination != LOG_ONLY_TO_SYSTEM_DEBUG_LOG) {
+  if ((logging_destination & LOG_TO_FILE) != 0) {
     LoggingLock logging_lock;
     if (InitializeLogFileHandle()) {
 #if defined(OS_WIN)

diff --git a/base/logging.h b/base/logging.h
@@ -146,22 +146,39 @@
 
 namespace logging {
 
-// Where to record logging output? A flat file and/or system debug log via
-// OutputDebugString. Defaults on Windows to LOG_ONLY_TO_FILE, and on
-// POSIX to LOG_ONLY_TO_SYSTEM_DEBUG_LOG (aka stderr).
-enum LoggingDestination { LOG_NONE,
-                          LOG_ONLY_TO_FILE,
-                          LOG_ONLY_TO_SYSTEM_DEBUG_LOG,
-                          LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG };
+// TODO(avi): do we want to do a unification of character types here?
+#if defined(OS_WIN)
+typedef wchar_t PathChar;
+#else
+typedef char PathChar;
+#endif
+
+// Where to record logging output? A flat file and/or system debug log
+// via OutputDebugString.
+enum LoggingDestination {
+  LOG_NONE                = 0,
+  LOG_TO_FILE             = 1 << 0,
+  LOG_TO_SYSTEM_DEBUG_LOG = 1 << 1,
+
+  LOG_TO_ALL = LOG_TO_FILE | LOG_TO_SYSTEM_DEBUG_LOG,
+
+  // On Windows, use a file next to the exe; on POSIX platforms, where
+  // it may not even be possible to locate the executable on disk, use
+  // stderr.
+#if defined(OS_WIN)
+  LOG_DEFAULT = LOG_TO_FILE,
+#elif defined(OS_POSIX)
+  LOG_DEFAULT = LOG_TO_SYSTEM_DEBUG_LOG,
+#endif
+};
 
 // Indicates that the log file should be locked when being written to.
-// Often, there is no locking, which is fine for a single threaded program.
-// If logging is being done from multiple threads or there can be more than
-// one process doing the logging, the file should be locked during writes to
-// make each log outut atomic. Other writers will block.
+// Unless there is only one single-threaded process that is logging to
+// the log file, the file should be locked during writes to make each
+// log outut atomic. Other writers will block.
 //
 // All processes writing to the log file must have their locking set for it to
-// work properly. Defaults to DONT_LOCK_LOG_FILE.
+// work properly. Defaults to LOCK_LOG_FILE.
 enum LogLockingState { LOCK_LOG_FILE, DONT_LOCK_LOG_FILE };
 
 // On startup, should we delete or append to an existing log file (if any)?
@@ -173,12 +190,26 @@ enum DcheckState {
   ENABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS
 };
 
-// TODO(avi): do we want to do a unification of character types here?
-#if defined(OS_WIN)
-typedef wchar_t PathChar;
-#else
-typedef char PathChar;
-#endif
+struct BASE_EXPORT LoggingSettings {
+  // The defaults values are:
+  //
+  //  logging_dest: LOG_DEFAULT
+  //  log_file:     NULL
+  //  lock_log:     LOCK_LOG_FILE
+  //  delete_old:   APPEND_TO_OLD_LOG_FILE
+  //  dcheck_state: DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS
+  LoggingSettings();
+
+  LoggingDestination logging_dest;
+
+  // The three settings below have an effect only when LOG_TO_FILE is
+  // set in |logging_dest|.
+  const PathChar* log_file;
+  LogLockingState lock_log;
+  OldFileDeletionState delete_old;
+
+  DcheckState dcheck_state;
+};
 
 // Define different names for the BaseInitLoggingImpl() function depending on
 // whether NDEBUG is defined or not so that we'll fail to link if someone tries
@@ -193,11 +224,7 @@ typedef char PathChar;
 // Implementation of the InitLogging() method declared below.  We use a
 // more-specific name so we can #define it above without affecting other code
 // that has named stuff "InitLogging".
-BASE_EXPORT bool BaseInitLoggingImpl(const PathChar* log_file,
-                                     LoggingDestination logging_dest,
-                                     LogLockingState lock_log,
-                                     OldFileDeletionState delete_old,
-                                     DcheckState dcheck_state);
+BASE_EXPORT bool BaseInitLoggingImpl(const LoggingSettings& settings);
 
 // Sets the log file name and other global logging state. Calling this function
 // is recommended, and is normally done at the beginning of application init.
@@ -213,13 +240,8 @@ BASE_EXPORT bool BaseInitLoggingImpl(const PathChar* log_file,
 // This function may be called a second time to re-direct logging (e.g after
 // loging in to a user partition), however it should never be called more than
 // twice.
-inline bool InitLogging(const PathChar* log_file,
-                        LoggingDestination logging_dest,
-                        LogLockingState lock_log,
-                        OldFileDeletionState delete_old,
-                        DcheckState dcheck_state) {
-  return BaseInitLoggingImpl(log_file, logging_dest, lock_log,
-                             delete_old, dcheck_state);
+inline bool InitLogging(const LoggingSettings& settings) {
+  return BaseInitLoggingImpl(settings);
 }
 
 // Sets the log level. Anything at or above this level will be written to the

diff --git a/base/test/test_suite.cc b/base/test/test_suite.cc
@@ -221,12 +221,11 @@ void TestSuite::Initialize() {
   base::FilePath exe;
   PathService::Get(base::FILE_EXE, &exe);
   base::FilePath log_filename = exe.ReplaceExtension(FILE_PATH_LITERAL("log"));
-  logging::InitLogging(
-      log_filename.value().c_str(),
-      logging::LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG,
-      logging::LOCK_LOG_FILE,
-      logging::DELETE_OLD_LOG_FILE,
-      logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
+  logging::LoggingSettings settings;
+  settings.logging_dest = logging::LOG_TO_ALL;
+  settings.log_file = log_filename.value().c_str();
+  settings.delete_old = logging::DELETE_OLD_LOG_FILE;
+  logging::InitLogging(settings);
   // We want process and thread IDs because we may have multiple processes.
   // Note: temporarily enabled timestamps in an effort to catch bug 6361.
   logging::SetLogItems(true, true, true, true);

diff --git a/base/test/test_support_android.cc b/base/test/test_support_android.cc
@@ -165,11 +165,9 @@ void InitPathProvider(int key) {
 namespace base {
 
 void InitAndroidTestLogging() {
-  logging::InitLogging(NULL,
-                       logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG,
-                       logging::DONT_LOCK_LOG_FILE,
-                       logging::DELETE_OLD_LOG_FILE,
-                       logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
+  logging::LoggingSettings settings;
+  settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG;
+  logging::InitLogging(settings);
   // To view log output with IDs and timestamps use "adb logcat -v threadtime".
   logging::SetLogItems(false,    // Process ID
                        false,    // Thread ID

diff --git a/chrome/common/logging_chrome.cc b/chrome/common/logging_chrome.cc
@@ -132,13 +132,11 @@ LoggingDestination DetermineLogMode(const CommandLine& command_line) {
 #ifdef NDEBUG
   bool enable_logging = false;
   const char *kInvertLoggingSwitch = switches::kEnableLogging;
-  const logging::LoggingDestination kDefaultLoggingMode =
-      logging::LOG_ONLY_TO_FILE;
+  const logging::LoggingDestination kDefaultLoggingMode = logging::LOG_TO_FILE;
 #else
   bool enable_logging = true;
   const char *kInvertLoggingSwitch = switches::kDisableLogging;
-  const logging::LoggingDestination kDefaultLoggingMode =
-      logging::LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG;
+  const logging::LoggingDestination kDefaultLoggingMode = logging::LOG_TO_ALL;
 #endif
 
   if (command_line.HasSwitch(kInvertLoggingSwitch))
@@ -149,7 +147,7 @@ LoggingDestination DetermineLogMode(const CommandLine& command_line) {
     // Let --enable-logging=stderr force only stderr, particularly useful for
     // non-debug builds where otherwise you can't get logs to stderr at all.
     if (command_line.GetSwitchValueASCII(switches::kEnableLogging) == "stderr")
-      log_mode = logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG;
+      log_mode = logging::LOG_TO_SYSTEM_DEBUG_LOG;
     else
       log_mode = kDefaultLoggingMode;
   } else {
@@ -252,11 +250,11 @@ void RedirectChromeLogging(const CommandLine& command_line) {
 
   // ChromeOS always logs through the symlink, so it shouldn't be
   // deleted if it already exists.
-  if (!InitLogging(log_path.value().c_str(),
-                   DetermineLogMode(command_line),
-                   logging::LOCK_LOG_FILE,
-                   logging::APPEND_TO_OLD_LOG_FILE,
-                   dcheck_state)) {
+  logging::LoggingSettings settings;
+  settings.logging_dest = DetermineLogMode(command_line);
+  settings.log_file = log_path.value().c_str();
+  settings.dcheck_state = dcheck_state;
+  if (!logging::InitLogging(settings)) {
     DLOG(ERROR) << "Unable to initialize logging to " << log_path.value();
     RemoveSymlinkAndLog(log_path, target_path);
   } else {
@@ -280,8 +278,7 @@ void InitChromeLogging(const 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_ONLY_TO_FILE ||
-      logging_dest == LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG) {
+  if ((logging_dest & LOG_TO_FILE) != 0) {
     log_path = GetLogFileName();
 
 #if defined(OS_CHROMEOS)
@@ -311,11 +308,13 @@ void InitChromeLogging(const CommandLine& command_line,
       logging::ENABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS :
       logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS;
 
-  bool success = InitLogging(log_path.value().c_str(),
-                             logging_dest,
-                             log_locking_state,
-                             delete_old_log_file,
-                             dcheck_state);
+  logging::LoggingSettings settings;
+  settings.logging_dest = logging_dest;
+  settings.log_file = log_path.value().c_str();
+  settings.lock_log = log_locking_state;
+  settings.delete_old = delete_old_log_file;
+  settings.dcheck_state = dcheck_state;
+  bool success = logging::InitLogging(settings);
 
 #if defined(OS_CHROMEOS)
   if (!success) {

diff --git a/chrome/installer/gcapi/gcapi_dll.cc b/chrome/installer/gcapi/gcapi_dll.cc
@@ -22,12 +22,9 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE instance,
   if (reason == DLL_PROCESS_ATTACH) {
     g_exit_manager = new base::AtExitManager();
     CommandLine::Init(0, NULL);
-    logging::InitLogging(
-        NULL,
-        logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG,
-        logging::LOCK_LOG_FILE,
-        logging::DELETE_OLD_LOG_FILE,
-        logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
+    logging::LoggingSettings settings;
+    settings.logging_dest = logging::LOG_TO_SYSTEM_DEBUG_LOG;
+    logging::InitLogging(settings);
   } else if (reason == DLL_PROCESS_DETACH) {
     CommandLine::Reset();
     delete g_exit_manager;

diff --git a/chrome/installer/tools/validate_installation_main.cc b/chrome/installer/tools/validate_installation_main.cc
@@ -61,12 +61,12 @@ logging::LogMessageHandlerFunction
 
 ConsoleLogHelper::ConsoleLogHelper() : log_file_path_(GetLogFilePath()) {
   LOG_ASSERT(old_message_handler_ == NULL);
-  logging::InitLogging(
-      log_file_path_.value().c_str(),
-      logging::LOG_ONLY_TO_FILE,
-      logging::DONT_LOCK_LOG_FILE,
-      logging::DELETE_OLD_LOG_FILE,
-      logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
+  logging::LoggingSettings settings;
+  settings.logging_dest = logging::LOG_TO_FILE;
+  settings.log_file = log_file_path_.value().c_str();
+  settings.lock_log = logging::DONT_LOCK_LOG_FILE;
+  settings.delete_old = logging::DELETE_OLD_LOG_FILE;
+  logging::InitLogging(settings);
 
   old_message_handler_ = logging::GetLogMessageHandler();
   logging::SetLogMessageHandler(&DumpLogMessage);

diff --git a/chrome/installer/util/logging_installer.cc b/chrome/installer/util/logging_installer.cc
@@ -89,12 +89,10 @@ void InitInstallerLogging(const installer::MasterPreferences& prefs) {
   base::FilePath log_file_path(GetLogFilePath(prefs));
   TruncateLogFileIfNeeded(log_file_path);
 
-  logging::InitLogging(
-      log_file_path.value().c_str(),
-      logging::LOG_ONLY_TO_FILE,
-      logging::LOCK_LOG_FILE,
-      logging::APPEND_TO_OLD_LOG_FILE,
-      logging::DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS);
+  logging::LoggingSettings settings;
+  settings.logging_dest = logging::LOG_TO_FILE;
+  settings.log_file = log_file_path.value().c_str();
+  logging::InitLogging(settings);
 
   if (prefs.GetBool(installer::master_preferences::kVerboseLogging,
                     &value) && value) {