Revert "Reland "[Logging] Disable always-log-ERRORs-to-stderr unless LOG_TO_FILE is set.""

This reverts commit eb26f201.

Reason for revert: This was relanded without any changes to address the
LOG(ERROR) debugging issues that prompted it to be reverted the first time.

Original change's description:
> Reland "[Logging] Disable always-log-ERRORs-to-stderr unless LOG_TO_FILE is set."
> 
> This reverts commit c6b8b53b.
> 
> Reason for revert: Fixing the debugging issues.
> 
> Original change's description:
> > Revert "[Logging] Disable always-log-ERRORs-to-stderr unless LOG_TO_FILE is set."
> > 
> > This reverts commit ffa05b99.
> > 
> > Reason for revert: This breaks a lot of debugging workflows.
> > 
> > Original change's description:
> > > [Logging] Disable always-log-ERRORs-to-stderr unless LOG_TO_FILE is set.
> > > 
> > > logging::LogMessage has a special-case to write LOG_ERROR messages
> > > to stderr, even if LOG_TO_STDERR is not enabled, to make ERROR and
> > > FATAL messages more visible, especially in tests.
> > > 
> > > Restrict this behaviour to apply only if LOG_TO_FILE is set, so that
> > > binaries which only LOG_TO_SYSTEM_DEBUG_LOG will never emit logspam
> > > to stderr.
> > > 
> > > Tested by adding a unit test for this case.
> > > 
> > > Bug: 968387
> > > Change-Id: Icfe094e7d7bf9697a70a589eab48edbe79c36091
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638782
> > > Commit-Queue: Sharon Yang <yangsharon@chromium.org>
> > > Reviewed-by: Wez <wez@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#666507}
> > 
> > TBR=wez@chromium.org,kmarshall@chromium.org,yangsharon@chromium.org
> > 
> > Change-Id: I857fd9bc95afedf204a5e981e6a362cd8990b4a8
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 968387
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1647361
> > Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> > Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#666753}
> 
> TBR=dcheng@chromium.org,wez@chromium.org,kmarshall@chromium.org,yangsharon@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 968387
> Change-Id: Id656bff53e3b53e4df981ca8e819f33900c966f4
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1682954
> Reviewed-by: Will Harris <wfh@chromium.org>
> Commit-Queue: Aidan Wolter <awolter@chromium.org>
> Auto-Submit: Aidan Wolter <awolter@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#677030}

TBR=wez@chromium.org,agl@chromium.org,wfh@chromium.org,awolter@chromium.org,yangsharon@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 968387, 984379
Change-Id: I46779692208bef6db0fb442df22868fdeecf932e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1704681Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#677994}

base/logging.cc

@@ -482,21 +482,13 @@ bool ShouldCreateLogMessage(int severity) {
   if (severity < g_min_log_level)
     return false;
 
-  // Return true here unless we know ~LogMessage won't do anything.
+  // Return true here unless we know ~LogMessage won't do anything. Note that
+  // ~LogMessage writes to stderr if severity_ >= kAlwaysPrintErrorLevel, even
+  // when g_logging_destination is LOG_NONE.
   return g_logging_destination != LOG_NONE || log_message_handler ||
          severity >= kAlwaysPrintErrorLevel;
 }
 
-// Returns true when logging destination LOG_TO_STDERR flag is set. Also when
-// configured to only LOG_TO_FILE, continue to log higher-severity messages to
-// stderr as well to better detect and diagnose problems with unit tests,
-// especially on the buildbots.
-bool ShouldLogToStderr(int severity) {
-  return ((g_logging_destination & LOG_TO_STDERR) != 0) ||
-         (severity >= kAlwaysPrintErrorLevel &&
-          g_logging_destination == LOG_TO_FILE);
-}
-
 int GetVlogVerbosity() {
   return std::max(-1, LOG_INFO - GetMinLogLevel());
 }
@@ -858,7 +850,11 @@ LogMessage::~LogMessage() {
 #endif  // OS_FUCHSIA
   }
 
-  if (ShouldLogToStderr(severity_)) {
+  if ((g_logging_destination & LOG_TO_STDERR) != 0 ||
+      severity_ >= kAlwaysPrintErrorLevel) {
+    // Write logs with destination LOG_TO_STDERR to stderr. Also output to
+    // stderr for logs above a certain log level to better detect and diagnose
+    // problems with unit tests, especially on the buildbots.
     ignore_result(fwrite(str_newline.data(), str_newline.size(), 1, stderr));
     fflush(stderr);
   }

base/logging_unittest.cc

@@ -8,8 +8,6 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
@@ -238,60 +236,6 @@ TEST_F(LoggingTest, LogToStdErrFlag) {
   LOG(INFO) << mock_log_source_stderr.Log();
 }
 
-// Check that when logging to file and message severity is
-// kAlwaysPrintErrorLevel or higher that the message is also written to stderr.
-// When severity is lower, make sure the message is only written to file.
-// Relies on POSIX specific APIs. Though this feature is cross-platform, testing
-// on POSIX platforms is sufficient.
-#if defined(OS_POSIX) || defined(OS_FUCHSIA)
-TEST_F(LoggingTest, LogToFileAndStderr) {
-  const char kInfoLogMessage[] = "This is an INFO level message";
-  const char kErrorLogMessage[] = "Here we have a message of level ERROR";
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-
-  // Set up logging to log to a file.
-  base::FilePath file_logs_path = temp_dir.GetPath().Append("file.log");
-  LoggingSettings settings;
-  settings.logging_dest = LOG_TO_FILE;
-  settings.log_file = file_logs_path.value().c_str();
-  InitLogging(settings);
-
-  // Create a file and change stderr to write to that file, to easily check
-  // contents.
-  base::FilePath stderr_logs_path = temp_dir.GetPath().Append("stderr.log");
-  base::File stderr_logs = base::File(
-      stderr_logs_path,
-      base::File::FLAG_CREATE | base::File::FLAG_WRITE | base::File::FLAG_READ);
-  base::ScopedFD stderr_backup = base::ScopedFD(dup(STDERR_FILENO));
-  int dup_result = dup2(stderr_logs.GetPlatformFile(), STDERR_FILENO);
-  ASSERT_GE(dup_result, 0);
-
-  LOG(INFO) << kInfoLogMessage;
-  LOG(ERROR) << kErrorLogMessage;
-
-  // Restore the original stderr logging destination.
-  dup_result = dup2(stderr_backup.get(), STDERR_FILENO);
-  ASSERT_EQ(dup_result, STDERR_FILENO);
-
-  // Check that both log messages are written to "file.log".
-  std::string written_file_logs;
-  ASSERT_TRUE(base::ReadFileToString(file_logs_path, &written_file_logs));
-  std::size_t found_info_log = written_file_logs.find(kInfoLogMessage);
-  EXPECT_FALSE(found_info_log == std::string::npos);
-  std::size_t found_error_log = written_file_logs.find(kErrorLogMessage);
-  EXPECT_FALSE(found_error_log == std::string::npos);
-
-  // Check that the kErrorLogMessage is written to |stderr.log| and
-  // kInfoLogMessage is not.
-  ASSERT_TRUE(base::ReadFileToString(stderr_logs_path, &written_file_logs));
-  found_info_log = written_file_logs.find(kInfoLogMessage);
-  EXPECT_TRUE(found_info_log == std::string::npos);
-  found_error_log = written_file_logs.find(kErrorLogMessage);
-  EXPECT_FALSE(found_error_log == std::string::npos);
-}
-#endif
-
 // Official builds have CHECKs directly call BreakDebugger.
 #if !defined(OFFICIAL_BUILD)