Various upgrade_util improvements.

- upgrade_util.{cc,h} and friends are now only included in the build for
  relevant platforms (desktop Chrome).
- upgrade_util::SetNewCommandLine now explicitly takes ownership of its
  argument.
- A test seam has been added to RelaunchChromeBrowser. Tests may now
  specify a callback to be run when relaunch takes place.
- Unexpected calls to RelaunchChromeBrowser in browser_tests and friends
  now cause test failures (they previously attempted to launch Chrome).
- ScopedRelaunchChromeBrowserOverride is now available for tests of
  scenarios that involved RelaunchChromeBrowser.

BUG=958893,989468

Change-Id: I8620e2bbe56e282934b94b3ea2ae52ef32f9d04a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1736707
Auto-Submit: Greg Thompson <grt@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685466}

chrome/browser/BUILD.gn

@@ -3023,11 +3023,6 @@ jumbo_split_static_library("browser") {
       "first_run/first_run_internal_linux.cc",
       "first_run/first_run_internal_mac.mm",
       "first_run/first_run_internal_win.cc",
-      "first_run/upgrade_util.h",
-      "first_run/upgrade_util_mac.h",
-      "first_run/upgrade_util_mac.mm",
-      "first_run/upgrade_util_win.cc",
-      "first_run/upgrade_util_win.h",
       "font_family_cache.cc",
       "font_family_cache.h",
       "hid/chrome_hid_delegate.cc",
@@ -3701,7 +3696,6 @@ jumbo_split_static_library("browser") {
       "browser_switcher/browser_switcher_service_win.h",
       "downgrade/user_data_downgrade.cc",
       "downgrade/user_data_downgrade.h",
-      "first_run/upgrade_util.cc",
       "notifications/win/notification_image_retainer.cc",
       "notifications/win/notification_image_retainer.h",
       "notifications/win/notification_template_builder.cc",
@@ -3867,7 +3861,6 @@ jumbo_split_static_library("browser") {
   if (is_desktop_linux) {
     # Desktop linux, doesn't count ChromeOS.
     sources += [
-      "first_run/upgrade_util.cc",
       "first_run/upgrade_util_linux.cc",
       "first_run/upgrade_util_linux.h",
       "icon_loader_auralinux.cc",
@@ -3983,6 +3976,12 @@ jumbo_split_static_library("browser") {
 
   if (!is_android && !is_chromeos) {
     sources += [
+      "first_run/upgrade_util.cc",
+      "first_run/upgrade_util.h",
+      "first_run/upgrade_util_mac.h",
+      "first_run/upgrade_util_mac.mm",
+      "first_run/upgrade_util_win.cc",
+      "first_run/upgrade_util_win.h",
       "lifetime/switch_utils.cc",
       "lifetime/switch_utils.h",
       "metrics/upgrade_metrics_provider.cc",

chrome/browser/browser_process_impl.cc

@@ -1458,7 +1458,7 @@ void BrowserProcessImpl::RestartBackgroundInstance() {
   DLOG(WARNING) << "Shutting down current instance of the browser.";
   chrome::AttemptExit();
 
-  upgrade_util::SetNewCommandLine(new_cl.release());
+  upgrade_util::SetNewCommandLine(std::move(new_cl));
 }
 
 void BrowserProcessImpl::OnAutoupdateTimer() {

chrome/browser/first_run/scoped_relaunch_chrome_browser_override.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/first_run/scoped_relaunch_chrome_browser_override.h"
+
+#include <utility>
+
+namespace upgrade_util {
+
+ScopedRelaunchChromeBrowserOverride::ScopedRelaunchChromeBrowserOverride(
+    RelaunchChromeBrowserCallback callback)
+    : previous_(
+          SetRelaunchChromeBrowserCallbackForTesting(std::move(callback))) {}
+
+ScopedRelaunchChromeBrowserOverride::~ScopedRelaunchChromeBrowserOverride() {
+  SetRelaunchChromeBrowserCallbackForTesting(std::move(previous_));
+}
+
+}  // namespace upgrade_util

chrome/browser/first_run/scoped_relaunch_chrome_browser_override.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_FIRST_RUN_SCOPED_RELAUNCH_CHROME_BROWSER_OVERRIDE_H_
+#define CHROME_BROWSER_FIRST_RUN_SCOPED_RELAUNCH_CHROME_BROWSER_OVERRIDE_H_
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "chrome/browser/first_run/upgrade_util.h"
+
+namespace upgrade_util {
+
+// A test helper that overrides RelaunchChromeBrowser with a given callback for
+// the lifetime of an instance. The previous callback (or none) is restored
+// upon deletion.
+class ScopedRelaunchChromeBrowserOverride {
+ public:
+  explicit ScopedRelaunchChromeBrowserOverride(
+      RelaunchChromeBrowserCallback callback);
+  ~ScopedRelaunchChromeBrowserOverride();
+
+ private:
+  RelaunchChromeBrowserCallback previous_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedRelaunchChromeBrowserOverride);
+};
+
+}  // namespace upgrade_util
+
+#endif  // CHROME_BROWSER_FIRST_RUN_SCOPED_RELAUNCH_CHROME_BROWSER_OVERRIDE_H_

chrome/browser/first_run/upgrade_util.cc

@@ -4,19 +4,44 @@
 
 #include "chrome/browser/first_run/upgrade_util.h"
 
+#include <utility>
+
+#include "base/callback.h"
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "build/build_config.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace {
 
-base::CommandLine* command_line;
+#if !defined(OS_MACOSX)
+base::CommandLine* command_line = nullptr;
+#endif
+
+// A test seam for whole-browser tests to override browser relaunch.
+upgrade_util::RelaunchChromeBrowserCallback*
+    relaunch_chrome_browser_callback_for_testing = nullptr;
 
 }  // namespace
 
 namespace upgrade_util {
 
-void SetNewCommandLine(base::CommandLine* new_command_line) {
-  command_line = new_command_line;
+// Forward-declaration of the platform-specific implementation.
+bool RelaunchChromeBrowserImpl(const base::CommandLine& command_line);
+
+bool RelaunchChromeBrowser(const base::CommandLine& command_line) {
+  if (relaunch_chrome_browser_callback_for_testing)
+    return relaunch_chrome_browser_callback_for_testing->Run(command_line);
+
+  return RelaunchChromeBrowserImpl(command_line);
+}
+
+#if !defined(OS_MACOSX)
+
+void SetNewCommandLine(std::unique_ptr<base::CommandLine> new_command_line) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  delete command_line;
+  command_line = new_command_line.release();
 }
 
 void RelaunchChromeBrowserWithNewCommandLineIfNeeded() {
@@ -27,8 +52,33 @@ void RelaunchChromeBrowserWithNewCommandLineIfNeeded() {
       DLOG(WARNING) << "Launched a new instance of the browser.";
     }
     delete command_line;
-    command_line = NULL;
+    command_line = nullptr;
   }
 }
 
+#endif  // !defined(OS_MACOSX)
+
+RelaunchChromeBrowserCallback SetRelaunchChromeBrowserCallbackForTesting(
+    RelaunchChromeBrowserCallback callback) {
+  // Take ownership of the current test callback so it can be returned.
+  RelaunchChromeBrowserCallback previous =
+      relaunch_chrome_browser_callback_for_testing
+          ? std::move(*relaunch_chrome_browser_callback_for_testing)
+          : RelaunchChromeBrowserCallback();
+
+  // Move the caller's callback into the global, alloc'ing or freeing as needed.
+  auto memory = base::WrapUnique(relaunch_chrome_browser_callback_for_testing);
+  if (callback) {
+    if (!memory)
+      memory = std::make_unique<RelaunchChromeBrowserCallback>();
+    *memory = std::move(callback);
+  } else if (memory) {
+    memory.reset();
+  }
+  relaunch_chrome_browser_callback_for_testing = memory.release();
+
+  // Return the previous callback.
+  return previous;
+}
+
 }  // namespace upgrade_util

chrome/browser/first_run/upgrade_util.h

@@ -5,16 +5,15 @@
 #ifndef CHROME_BROWSER_FIRST_RUN_UPGRADE_UTIL_H_
 #define CHROME_BROWSER_FIRST_RUN_UPGRADE_UTIL_H_
 
+#include <memory>
+
+#include "base/callback_forward.h"
 #include "build/build_config.h"
 
 #if defined(OS_ANDROID) || defined(OS_CHROMEOS)
 #error Not used on Android or ChromeOS
 #endif
 
-#if defined(OS_WIN)
-#include <string>
-#endif
-
 namespace base {
 class CommandLine;
 }
@@ -27,10 +26,12 @@ bool RelaunchChromeBrowser(const base::CommandLine& command_line);
 
 #if !defined(OS_MACOSX)
 
-void SetNewCommandLine(base::CommandLine* new_command_line);
+// Sets a command line to be used to relaunch the browser upon exit.
+void SetNewCommandLine(std::unique_ptr<base::CommandLine> new_command_line);
 
-// Launches a new instance of the browser if the current instance in persistent
-// mode an upgrade is detected.
+// Launches a new instance of the browser using a command line previously
+// provided to SetNewCommandLine. This is typically used to finalize an in-use
+// update that was detected while the browser was in persistent mode.
 void RelaunchChromeBrowserWithNewCommandLineIfNeeded();
 
 // Windows:
@@ -42,6 +43,15 @@ bool IsUpdatePendingRestart();
 
 #endif  // !defined(OS_MACOSX)
 
+using RelaunchChromeBrowserCallback =
+    base::RepeatingCallback<bool(const base::CommandLine&)>;
+
+// Sets |callback| to be run to process a RelaunchChromeBrowser request. This
+// is a test seam for whole-browser tests. See
+// ScopedRelaunchChromeBrowserOverride for convenience.
+RelaunchChromeBrowserCallback SetRelaunchChromeBrowserCallbackForTesting(
+    RelaunchChromeBrowserCallback callback);
+
 }  // namespace upgrade_util
 
 #endif  // CHROME_BROWSER_FIRST_RUN_UPGRADE_UTIL_H_

chrome/browser/first_run/upgrade_util_linux.cc

@@ -21,7 +21,7 @@ double saved_last_modified_time_of_exe = 0;
 
 namespace upgrade_util {
 
-bool RelaunchChromeBrowser(const base::CommandLine& command_line) {
+bool RelaunchChromeBrowserImpl(const base::CommandLine& command_line) {
   base::LaunchOptions options;
   // Don't set NO_NEW_PRIVS on the relaunched browser process.
   options.allow_new_privs = true;

chrome/browser/first_run/upgrade_util_mac.mm

@@ -286,7 +286,7 @@ bool ShouldContinueToRelaunchForUpgrade() {
   return !other_instances_exist;
 }
 
-bool RelaunchChromeBrowser(const base::CommandLine& command_line) {
+bool RelaunchChromeBrowserImpl(const base::CommandLine& command_line) {
   upgrade_util::ThisAndOtherUserCounts counts =
       upgrade_util::GetCountOfOtherInstancesOfThisBinary();
   const int other_instances = counts.this_user_count + counts.other_user_count;

chrome/browser/first_run/upgrade_util_win.cc

@@ -95,7 +95,7 @@ bool InvokeGoogleUpdateForRename() {
 
 namespace upgrade_util {
 
-bool RelaunchChromeBrowser(const base::CommandLine& command_line) {
+bool RelaunchChromeBrowserImpl(const base::CommandLine& command_line) {
   base::FilePath chrome_exe;
   if (!base::PathService::Get(base::FILE_EXE, &chrome_exe)) {
     NOTREACHED();

chrome/test/BUILD.gn

@@ -5158,6 +5158,11 @@ if (!is_android) {
         "//chromeos/services/device_sync",
         "//chromeos/services/device_sync:test_support",
       ]
+    } else {
+      sources += [
+        "../browser/first_run/scoped_relaunch_chrome_browser_override.cc",
+        "../browser/first_run/scoped_relaunch_chrome_browser_override.h",
+      ]
     }
   }
 

chrome/test/base/chrome_test_launcher.cc

@@ -65,6 +65,12 @@
 #include "chrome/installer/util/firewall_manager_win.h"
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+    (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+#include "chrome/browser/first_run/scoped_relaunch_chrome_browser_override.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#endif
+
 ChromeTestSuiteRunner::ChromeTestSuiteRunner() {}
 ChromeTestSuiteRunner::~ChromeTestSuiteRunner() {}
 
@@ -242,5 +248,19 @@ int LaunchChromeTests(size_t parallel_jobs,
       base::Bind(&ash::mojo_test_interface_factory::RegisterInterfaces));
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX) || \
+    (defined(OS_LINUX) && !defined(OS_CHROMEOS))
+  // Cause a test failure for any test that triggers an unexpected relaunch.
+  // Tests that fail here should likely be restructured to put the "before
+  // relaunch" code into a PRE_ test with its own
+  // ScopedRelaunchChromeBrowserOverride and the "after relaunch" code into the
+  // normal non-PRE_ test.
+  upgrade_util::ScopedRelaunchChromeBrowserOverride fail_on_relaunch(
+      base::BindRepeating([](const base::CommandLine&) {
+        ADD_FAILURE() << "Unexpected call to RelaunchChromeBrowser";
+        return false;
+      }));
+#endif
+
   return content::LaunchTests(delegate, parallel_jobs, argc, argv);
 }