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

4 files changed, 83 insertions, 73 deletions

diff --git a/base/logging.cc b/base/logging.cc
index 49623bf..3b78387 100644
--- 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
index 045ecd4..859f260 100644
--- 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
index 07b9964..a4b1780 100644
--- 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
index 5180333..e044322 100644
--- 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