Revert "Browser shutdown cleanups (no functional changes)."

This reverts commit 11936b3.

Reason for revert: Crashy.

Original change's description:
> Browser shutdown cleanups (no functional changes).
> 
> - Make ShutdownType an enum class.
> - Introduce browser_shutdown::HasShutdownStarted as a convenience.
> - Move some Win-specific relaunch code from AttemptRestart, which runs
>   fairly early in a restart that may never actually happen, into
>   upgrade_util::RelaunchChromeBrowserImpl, which performs the actual
>   restart.
> - Switch from UMA macros to functions for one-shot metrics.
> 
> BUG=958865
> 
> Change-Id: I75cd08c712d8bb7c674b4b5e413d79abc874bc39
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1910098
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Commit-Queue: Greg Thompson <grt@chromium.org>
> Auto-Submit: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#715730}

TBR=treib@chromium.org,bcwhite@chromium.org,holte@chromium.org,grt@chromium.org

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

(cherry picked from commit 0328367)

Bug: 958865,1025638,1025634
Change-Id: Ia97829abe61fcfb362720ce74741a5d375bb811e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1921973
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#716146}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919766
Cr-Commit-Position: refs/branch-heads/3970@{#8}
Cr-Branched-From: 55f4451-refs/heads/master@{#716057}

chrome/browser/background/background_mode_optimizer.cc

@@ -76,7 +76,7 @@ void BackgroundModeOptimizer::TryBrowserRestart() {
 
   // If the application is already shutting down, do not turn it into a restart.
   if (browser_shutdown::IsTryingToQuit() ||
-      browser_shutdown::HasShutdownStarted()) {
+      browser_shutdown::GetShutdownType() != browser_shutdown::NOT_VALID) {
     DVLOG(1) << "TryBrowserRestart: Cancelled because we are shutting down.";
     return;
   }

chrome/browser/chrome_content_browser_client.cc

@@ -1270,7 +1270,7 @@ void ChromeContentBrowserClient::SetBrowserStartupIsCompleteForTesting() {
 }
 
 bool ChromeContentBrowserClient::IsShuttingDown() {
-  return browser_shutdown::HasShutdownStarted();
+  return browser_shutdown::GetShutdownType() != browser_shutdown::NOT_VALID;
 }
 
 std::string ChromeContentBrowserClient::GetStoragePartitionIdForSite(

chrome/browser/chromeos/login/ui/login_display_host_webui.cc

@@ -714,7 +714,7 @@ void LoginDisplayHostWebUI::Observe(
 
 void LoginDisplayHostWebUI::RenderProcessGone(base::TerminationStatus status) {
   // Do not try to restore on shutdown
-  if (browser_shutdown::HasShutdownStarted())
+  if (browser_shutdown::GetShutdownType() != browser_shutdown::NOT_VALID)
     return;
 
   crash_count_++;

chrome/browser/first_run/upgrade_util_win.cc

@@ -32,9 +32,7 @@
 #include "build/branding_buildflags.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/first_run/upgrade_util_win.h"
-#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/shell_integration.h"
-#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/install_static/install_util.h"
@@ -101,21 +99,6 @@ bool InvokeGoogleUpdateForRename() {
 namespace upgrade_util {
 
 bool RelaunchChromeBrowserImpl(const base::CommandLine& command_line) {
-  // Breakpad will upload crash reports if the breakpad pipe name environment
-  // variable is defined. So we undefine this environment variable before
-  // restarting, as the restarted processes will inherit their environment
-  // variables from ours, thus suppressing crash uploads.
-  if (!ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled()) {
-    HMODULE exe_module = GetModuleHandle(chrome::kBrowserProcessExecutableName);
-    if (exe_module) {
-      typedef void(__cdecl * ClearBreakpadPipeEnvVar)();
-      ClearBreakpadPipeEnvVar clear = reinterpret_cast<ClearBreakpadPipeEnvVar>(
-          GetProcAddress(exe_module, "ClearBreakpadPipeEnvironmentVariable"));
-      if (clear)
-        clear();
-    }
-  }
-
   base::FilePath chrome_exe;
   if (!base::PathService::Get(base::FILE_EXE, &chrome_exe)) {
     NOTREACHED();

chrome/browser/lifetime/application_lifetime.cc

@@ -7,6 +7,10 @@
 #include <memory>
 #include <string>
 
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/process/process.h"
@@ -23,6 +27,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/buildflags.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/keep_alive_registry/keep_alive_registry.h"
 #include "components/language/core/browser/pref_names.h"
@@ -56,10 +61,10 @@
 
 #if defined(OS_WIN)
 #include "base/win/win_util.h"
+#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #endif
 
 namespace chrome {
-
 namespace {
 
 #if !defined(OS_ANDROID)
@@ -224,6 +229,23 @@ void AttemptUserExit() {
 // The Android implementation is in application_lifetime_android.cc
 #if !defined(OS_ANDROID)
 void AttemptRestart() {
+#if defined(OS_WIN)
+  // On Windows, Breakpad will upload crash reports if the breakpad pipe name
+  // environment variable is defined. So we undefine this environment variable
+  // before restarting, as the restarted processes will inherit their
+  // environment variables from ours, thus suppressing crash uploads.
+  if (!ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled()) {
+    HMODULE exe_module = GetModuleHandle(kBrowserProcessExecutableName);
+    if (exe_module) {
+      typedef void (__cdecl *ClearBreakpadPipeEnvVar)();
+      ClearBreakpadPipeEnvVar clear = reinterpret_cast<ClearBreakpadPipeEnvVar>(
+          GetProcAddress(exe_module, "ClearBreakpadPipeEnvironmentVariable"));
+      if (clear)
+        clear();
+    }
+  }
+#endif  // defined(OS_WIN)
+
   // TODO(beng): Can this use ProfileManager::GetLoadedProfiles instead?
   for (auto* browser : *BrowserList::GetInstance())
     content::BrowserContext::SaveSessionState(browser->profile());
@@ -285,9 +307,10 @@ void ExitIgnoreUnloadHandlers() {
   // In this case, AreAllBrowsersCloseable()
   // can be false in following cases. a) power-off b) signout from
   // screen locker.
-  browser_shutdown::OnShutdownStarting(
-      AreAllBrowsersCloseable() ? browser_shutdown::ShutdownType::kBrowserExit
-                                : browser_shutdown::ShutdownType::kEndSession);
+  if (!AreAllBrowsersCloseable())
+    browser_shutdown::OnShutdownStarting(browser_shutdown::END_SESSION);
+  else
+    browser_shutdown::OnShutdownStarting(browser_shutdown::BROWSER_EXIT);
 #endif
   AttemptExitInternal(true);
 }
@@ -323,8 +346,7 @@ void SessionEnding() {
   ShutdownWatcherHelper shutdown_watcher;
   shutdown_watcher.Arm(base::TimeDelta::FromSeconds(90));
 
-  browser_shutdown::OnShutdownStarting(
-      browser_shutdown::ShutdownType::kEndSession);
+  browser_shutdown::OnShutdownStarting(browser_shutdown::END_SESSION);
 
   // In a clean shutdown, browser_shutdown::OnShutdownStarting sets
   // g_shutdown_type, and browser_shutdown::ShutdownPreThreadsStop calls
@@ -344,7 +366,6 @@ void SessionEnding() {
 #if defined(OS_WIN)
   base::win::SetShouldCrashOnProcessDetach(false);
 #endif
-
   // On Windows 7 and later, the system will consider the process ripe for
   // termination as soon as it hides or destroys its windows. Since any
   // execution past that point will be non-deterministically cut short, we

chrome/browser/lifetime/browser_close_manager.cc

@@ -51,8 +51,7 @@ BrowserCloseManager::~BrowserCloseManager() {
 void BrowserCloseManager::StartClosingBrowsers() {
   // If the session is ending, skip straight to closing the browsers. There's no
   // time to wait for beforeunload dialogs.
-  if (browser_shutdown::GetShutdownType() ==
-      browser_shutdown::ShutdownType::kEndSession) {
+  if (browser_shutdown::GetShutdownType() == browser_shutdown::END_SESSION) {
     // Tell everyone that we are shutting down.
     browser_shutdown::SetTryingToQuit(true);
     CloseBrowsers();
@@ -163,8 +162,8 @@ void BrowserCloseManager::CloseBrowsers() {
             BrowserList::GetInstance()->end(),
             std::back_inserter(browser_list_copy));
 
-  bool session_ending = browser_shutdown::GetShutdownType() ==
-                        browser_shutdown::ShutdownType::kEndSession;
+  bool session_ending =
+      browser_shutdown::GetShutdownType() == browser_shutdown::END_SESSION;
 
   for (auto* browser : browser_list_copy) {
     browser->window()->Close();

chrome/browser/lifetime/browser_shutdown.cc

@@ -14,7 +14,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
-#include "base/metrics/histogram_functions.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
@@ -76,7 +76,7 @@ namespace {
 bool g_trying_to_quit = false;
 
 base::Time* g_shutdown_started = nullptr;
-ShutdownType g_shutdown_type = ShutdownType::kNotValid;
+ShutdownType g_shutdown_type = NOT_VALID;
 int g_shutdown_num_processes;
 int g_shutdown_num_processes_slow;
 
@@ -90,14 +90,14 @@ base::FilePath GetShutdownMsPath() {
 
 const char* ToShutdownTypeString(ShutdownType type) {
   switch (type) {
-    case ShutdownType::kNotValid:
+    case NOT_VALID:
       NOTREACHED();
-      break;
-    case ShutdownType::kWindowClose:
+      return "";
+    case WINDOW_CLOSE:
       return "close";
-    case ShutdownType::kBrowserExit:
+    case BROWSER_EXIT:
       return "exit";
-    case ShutdownType::kEndSession:
+    case END_SESSION:
       return "end";
   }
   return "";
@@ -106,15 +106,18 @@ const char* ToShutdownTypeString(ShutdownType type) {
 }  // namespace
 
 void RegisterPrefs(PrefRegistrySimple* registry) {
-  registry->RegisterIntegerPref(prefs::kShutdownType,
-                                static_cast<int>(ShutdownType::kNotValid));
+  registry->RegisterIntegerPref(prefs::kShutdownType, NOT_VALID);
   registry->RegisterIntegerPref(prefs::kShutdownNumProcesses, 0);
   registry->RegisterIntegerPref(prefs::kShutdownNumProcessesSlow, 0);
   registry->RegisterBooleanPref(prefs::kRestartLastSessionOnShutdown, false);
 }
 
+ShutdownType GetShutdownType() {
+  return g_shutdown_type;
+}
+
 void OnShutdownStarting(ShutdownType type) {
-  if (g_shutdown_type != ShutdownType::kNotValid)
+  if (g_shutdown_type != NOT_VALID)
     return;
 
   static crash_reporter::CrashKeyString<8> shutdown_type_key("shutdown-type");
@@ -142,14 +145,6 @@ void OnShutdownStarting(ShutdownType type) {
   }
 }
 
-bool HasShutdownStarted() {
-  return g_shutdown_type != ShutdownType::kNotValid;
-}
-
-ShutdownType GetShutdownType() {
-  return g_shutdown_type;
-}
-
 #if !defined(OS_ANDROID)
 bool ShutdownPreThreadsStop() {
 #if defined(OS_CHROMEOS)
@@ -188,11 +183,10 @@ bool ShutdownPreThreadsStop() {
 
 bool RecordShutdownInfoPrefs() {
   PrefService* prefs = g_browser_process->local_state();
-  if (g_shutdown_type != ShutdownType::kNotValid &&
-      g_shutdown_num_processes > 0) {
+  if (g_shutdown_type > NOT_VALID && g_shutdown_num_processes > 0) {
     // Record the shutdown info so that we can put it into a histogram at next
     // startup.
-    prefs->SetInteger(prefs::kShutdownType, static_cast<int>(g_shutdown_type));
+    prefs->SetInteger(prefs::kShutdownType, g_shutdown_type);
     prefs->SetInteger(prefs::kShutdownNumProcesses, g_shutdown_num_processes);
     prefs->SetInteger(prefs::kShutdownNumProcessesSlow,
                       g_shutdown_num_processes_slow);
@@ -223,7 +217,7 @@ void ShutdownPostThreadsStop(RestartMode restart_mode) {
 
 #if defined(OS_WIN)
   if (!browser_util::IsBrowserAlreadyRunning() &&
-      g_shutdown_type != ShutdownType::kEndSession) {
+      g_shutdown_type != END_SESSION) {
     upgrade_util::SwapNewChromeExeIfPresent();
   }
 #endif
@@ -269,8 +263,7 @@ void ShutdownPostThreadsStop(RestartMode restart_mode) {
 #endif  // defined(OS_CHROMEOS)
   }
 
-  if (g_shutdown_type != ShutdownType::kNotValid &&
-      g_shutdown_num_processes > 0) {
+  if (g_shutdown_type > NOT_VALID && g_shutdown_num_processes > 0) {
     // Measure total shutdown time as late in the process as possible
     // and then write it to a file to be read at startup.
     // We can't use prefs since all services are shutdown at this point.
@@ -305,40 +298,29 @@ void ReadLastShutdownFile(ShutdownType type,
     base::StringToInt64(shutdown_ms_str, &shutdown_ms);
   base::DeleteFile(shutdown_ms_file, false);
 
-  if (shutdown_ms == 0 || num_procs == 0)
+  if (type == NOT_VALID || shutdown_ms == 0 || num_procs == 0)
     return;
 
-  const char* time2_metric_name = nullptr;
-  const char* per_proc_metric_name = nullptr;
-
-  switch (type) {
-    case ShutdownType::kNotValid:
-      break;
-
-    case ShutdownType::kWindowClose:
-      time2_metric_name = "Shutdown.window_close.time2";
-      per_proc_metric_name = "Shutdown.window_close.time_per_process";
-      break;
-
-    case ShutdownType::kBrowserExit:
-      time2_metric_name = "Shutdown.browser_exit.time2";
-      per_proc_metric_name = "Shutdown.browser_exit.time_per_process";
-      break;
-
-    case ShutdownType::kEndSession:
-      time2_metric_name = "Shutdown.end_session.time2";
-      per_proc_metric_name = "Shutdown.end_session.time_per_process";
-      break;
+  if (type == WINDOW_CLOSE) {
+    UMA_HISTOGRAM_MEDIUM_TIMES("Shutdown.window_close.time2",
+                               TimeDelta::FromMilliseconds(shutdown_ms));
+    UMA_HISTOGRAM_TIMES("Shutdown.window_close.time_per_process",
+                        TimeDelta::FromMilliseconds(shutdown_ms / num_procs));
+  } else if (type == BROWSER_EXIT) {
+    UMA_HISTOGRAM_MEDIUM_TIMES("Shutdown.browser_exit.time2",
+                               TimeDelta::FromMilliseconds(shutdown_ms));
+    UMA_HISTOGRAM_TIMES("Shutdown.browser_exit.time_per_process",
+                        TimeDelta::FromMilliseconds(shutdown_ms / num_procs));
+  } else if (type == END_SESSION) {
+    UMA_HISTOGRAM_MEDIUM_TIMES("Shutdown.end_session.time2",
+                               TimeDelta::FromMilliseconds(shutdown_ms));
+    UMA_HISTOGRAM_TIMES("Shutdown.end_session.time_per_process",
+                        TimeDelta::FromMilliseconds(shutdown_ms / num_procs));
+  } else {
+    NOTREACHED();
   }
-  if (!time2_metric_name)
-    return;
-
-  base::UmaHistogramMediumTimes(time2_metric_name,
-                                TimeDelta::FromMilliseconds(shutdown_ms));
-  base::UmaHistogramTimes(per_proc_metric_name,
-                          TimeDelta::FromMilliseconds(shutdown_ms / num_procs));
-  base::UmaHistogramCounts100("Shutdown.renderers.total", num_procs);
-  base::UmaHistogramCounts100("Shutdown.renderers.slow", num_procs_slow);
+  UMA_HISTOGRAM_COUNTS_100("Shutdown.renderers.total", num_procs);
+  UMA_HISTOGRAM_COUNTS_100("Shutdown.renderers.slow", num_procs_slow);
 }
 
 void ReadLastShutdownInfo() {
@@ -348,12 +330,11 @@ void ReadLastShutdownInfo() {
   int num_procs = prefs->GetInteger(prefs::kShutdownNumProcesses);
   int num_procs_slow = prefs->GetInteger(prefs::kShutdownNumProcessesSlow);
   // clear the prefs immediately so we don't pick them up on a future run
-  prefs->SetInteger(prefs::kShutdownType,
-                    static_cast<int>(ShutdownType::kNotValid));
+  prefs->SetInteger(prefs::kShutdownType, NOT_VALID);
   prefs->SetInteger(prefs::kShutdownNumProcesses, 0);
   prefs->SetInteger(prefs::kShutdownNumProcessesSlow, 0);
 
-  base::UmaHistogramEnumeration("Shutdown.ShutdownType", type);
+  UMA_HISTOGRAM_ENUMERATION("Shutdown.ShutdownType", type, kNumShutdownTypes);
 
   base::PostTask(
       FROM_HERE,

chrome/browser/lifetime/browser_shutdown.h

@@ -41,30 +41,25 @@ enum class RestartMode {
 
 #endif  // !defined(OS_ANDROID)
 
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum class ShutdownType {
-  // An uninitialized value.
-  kNotValid = 0,
-  // The last browser window was closed.
-  kWindowClose = 1,
-  // The user clicked on the Exit menu item.
-  kBrowserExit = 2,
-  // User logoff or system shutdown.
-  kEndSession = 3,
-  kMaxValue = kEndSession
+enum ShutdownType {
+  // an uninitialized value
+  NOT_VALID = 0,
+  // the last browser window was closed
+  WINDOW_CLOSE,
+  // user clicked on the Exit menu item
+  BROWSER_EXIT,
+  // windows is logging off or shutting down
+  END_SESSION
 };
 
+constexpr int kNumShutdownTypes = END_SESSION + 1;
+
 void RegisterPrefs(PrefRegistrySimple* registry);
 
 // Called when the browser starts shutting down so that we can measure shutdown
 // time.
 void OnShutdownStarting(ShutdownType type);
 
-// Returns true if OnShutdownStarting has been called to note that shutdown has
-// started.
-bool HasShutdownStarted();
-
 // Get the current shutdown type.
 ShutdownType GetShutdownType();