Revert "Prevent additional Chrome restart on Chrome OS"

This reverts commit d4a5857.

Reason for revert: test failures -> https://b.corp.google.com/issues/216129138#comment35

Original change's description:
> Prevent additional Chrome restart on Chrome OS
>
> If AttemptRelanuch is called before this change both Chrome and
> Chrome OS are restarted. The Chrome OS shutdown will stop Chrome itself,
> so there's no reason to shutdown Chrome except for faked dbus clients.
>
> For those, move a faked shutdown of Chrome into fake power manager.
> As that changes call order, two tests that relied on the previous
> behavior need to be adapted.
>
> This change should also remove flakiness of
> EnableDebuggingDevTest.ShowAndRemoveProtection.
>
> Also remove RebootPolicy, it was only used for flash updates which don't
> exist anymore.
>
> Bug: b/216129138
> Change-Id: Ia1dddadc9d4e56495a96c841b5feccb54b6a2a35
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3506307
> Commit-Queue: Miriam Polzer <mpolzer@google.com>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1001417}

Bug: b/216129138
Change-Id: I02bdf84fe175df6ea23270fbbb969ebc034df514
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3637890
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Alexander Hendrich <hendrich@chromium.org>
Owners-Override: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001451}

chrome/browser/BUILD.gn

@@ -4729,8 +4729,6 @@ static_library("browser") {
       "google/google_brand_code_map_chromeos.cc",
       "google/google_brand_code_map_chromeos.h",
       "icon_loader_chromeos.cc",
-      "lifetime/application_lifetime_chromeos.cc",
-      "lifetime/application_lifetime_chromeos.h",
       "media/chromeos_login_media_access_handler.cc",
       "media/chromeos_login_media_access_handler.h",
       "media/public_session_media_access_handler.cc",

chrome/browser/apps/platform_apps/app_browsertest.cc

@@ -9,7 +9,6 @@
 #include "apps/launcher.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
-#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/containers/contains.h"
 #include "base/files/file_util.h"
@@ -73,7 +72,6 @@
 #include "url/gurl.h"
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-#include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/ash/login/users/mock_user_manager.h"
 #include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "components/user_manager/scoped_user_manager.h"
@@ -1341,48 +1339,53 @@ IN_PROC_BROWSER_TEST_F(PlatformAppIncognitoBrowserTest,
 
 class RestartDeviceTest : public PlatformAppBrowserTest {
  public:
+  RestartDeviceTest() = default;
+  RestartDeviceTest(const RestartDeviceTest&) = delete;
+  RestartDeviceTest& operator=(const RestartDeviceTest&) = delete;
+  ~RestartDeviceTest() override = default;
+
+  // PlatformAppBrowserTest overrides
+  void SetUpInProcessBrowserTestFixture() override {
+    PlatformAppBrowserTest::SetUpInProcessBrowserTestFixture();
+  }
+
   void SetUpOnMainThread() override {
     PlatformAppBrowserTest::SetUpOnMainThread();
 
-    // Disable "faked" shutdown of Chrome if the OS was supposed to restart.
-    // The fakes this test injects would cause it to crash.
-    chromeos::FakePowerManagerClient* fake_power_manager_client =
-        chromeos::FakePowerManagerClient::Get();
-    ASSERT_NE(nullptr, fake_power_manager_client);
-    fake_power_manager_client->set_restart_callback(base::DoNothing());
+    mock_user_manager_ = new ash::MockUserManager;
+    user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
+        base::WrapUnique(mock_user_manager_));
+
+    EXPECT_CALL(*mock_user_manager_, IsUserLoggedIn())
+        .WillRepeatedly(testing::Return(true));
+    EXPECT_CALL(*mock_user_manager_, IsLoggedInAsKioskApp())
+        .WillRepeatedly(testing::Return(true));
+    EXPECT_CALL(*mock_user_manager_, GetLoggedInUsers())
+        .WillRepeatedly(testing::Invoke(mock_user_manager_,
+                                        &ash::MockUserManager::GetUsers));
   }
 
   void TearDownOnMainThread() override {
-    PlatformAppBrowserTest::TearDownOnMainThread();
     user_manager_enabler_.reset();
-    fake_user_manager_ = nullptr;
+    PlatformAppBrowserTest::TearDownOnMainThread();
   }
 
- protected:
-  static int num_request_restart_calls() {
-    return chromeos::FakePowerManagerClient::Get()->num_request_restart_calls();
+  void TearDownInProcessBrowserTestFixture() override {
+    PlatformAppBrowserTest::TearDownInProcessBrowserTestFixture();
   }
 
-  void EnterKioskSession() {
-    fake_user_manager_ = new ash::FakeChromeUserManager();
-    user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
-        base::WrapUnique(fake_user_manager_));
-
-    const AccountId kiosk_account_id(
-        AccountId::FromUserEmail("kiosk@foobar.com"));
-    fake_user_manager_->AddKioskAppUser(kiosk_account_id);
-    fake_user_manager_->LoginUser(kiosk_account_id);
+  int num_request_restart_calls() const {
+    return chromeos::FakePowerManagerClient::Get()->num_request_restart_calls();
   }
 
  private:
-  ash::FakeChromeUserManager* fake_user_manager_ = nullptr;
+  ash::MockUserManager* mock_user_manager_ = nullptr;
   std::unique_ptr<user_manager::ScopedUserManager> user_manager_enabler_;
 };
 
 // Tests that chrome.runtime.restart would request device restart in
 // ChromeOS kiosk mode.
 IN_PROC_BROWSER_TEST_F(RestartDeviceTest, Restart) {
-  EnterKioskSession();
   ASSERT_EQ(0, num_request_restart_calls());
 
   ExtensionTestMessageListener launched_listener("Launched", true);

chrome/browser/ash/chrome_browser_main_parts_ash.cc

@@ -200,7 +200,6 @@
 #include "chromeos/components/sensors/ash/sensor_hal_dispatcher.h"
 #include "chromeos/dbus/constants/cryptohome_key_delegate_constants.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "chromeos/dbus/power/power_manager_client.h"
 #include "chromeos/dbus/power/power_policy_controller.h"
 #include "chromeos/dbus/session_manager/fake_session_manager_client.h"
@@ -293,35 +292,12 @@ void ApplySigninProfileModifications(Profile* profile) {
   prefs->SetBoolean(::prefs::kSafeBrowsingEnabled, false);
 }
 
-#if !defined(USE_REAL_DBUS_CLIENTS)
-chromeos::FakeSessionManagerClient* FakeSessionManagerClient() {
-  chromeos::FakeSessionManagerClient* fake_session_manager_client =
-      chromeos::FakeSessionManagerClient::Get();
-  DCHECK(fake_session_manager_client);
-  return fake_session_manager_client;
-}
-
-chromeos::FakePowerManagerClient* FakePowerManagerClient() {
-  chromeos::FakePowerManagerClient* fake_power_manager_client =
-      chromeos::FakePowerManagerClient::Get();
-  DCHECK(fake_power_manager_client);
-  return fake_power_manager_client;
-}
-
-void FakeShutdownSignal() {
-  // Receiving SIGTERM would result in `ExitIgnoreUnloadHandlers`.
+void FakeSessionStopped() {
+  // Session manager would ask Chrome to exit. Fake this behavior.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&chrome::ExitIgnoreUnloadHandlers));
 }
 
-void InstallFakeShutdownCalls() {
-  FakeSessionManagerClient()->set_stop_session_callback(
-      base::BindOnce(&FakeShutdownSignal));
-  FakePowerManagerClient()->set_restart_callback(
-      base::BindOnce(&FakeShutdownSignal));
-}
-#endif  // !defined(USE_REAL_DBUS_CLIENTS)
-
 }  // namespace
 
 namespace internal {
@@ -641,7 +617,7 @@ int ChromeBrowserMainPartsAsh::PreEarlyInitialization() {
   CHECK(DBusThreadManager::IsInitialized());
 
 #if !defined(USE_REAL_DBUS_CLIENTS)
-  // USE_REAL_DBUS clients may be undefined even if the device is using real
+  // USE_REAL_DBUS clients may be undefined even if the device is using reals
   // dbus clients.
   if (!base::SysInfo::IsRunningOnChromeOS()) {
     if (command_line->HasSwitch(switches::kFakeDriveFsLauncherChrootPath) &&
@@ -657,10 +633,11 @@ int ChromeBrowserMainPartsAsh::PreEarlyInitialization() {
     base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
     FakeUserDataAuthClient::Get()->set_user_data_dir(user_data_dir);
 
-    // If we're not running on a device, i.e. either in a test or in ash Chrome
-    // on linux, fake dbus calls that would result in a shutdown of Chrome by
-    // the system.
-    InstallFakeShutdownCalls();
+    chromeos::FakeSessionManagerClient* fake_session_manager_client =
+        chromeos::FakeSessionManagerClient::Get();
+    DCHECK(fake_session_manager_client);
+    fake_session_manager_client->set_stop_session_callback(
+        base::BindOnce(&FakeSessionStopped));
   }
 #endif  // !defined(USE_REAL_DBUS_CLIENTS)
 

chrome/browser/ash/login/enable_debugging_browsertest.cc

@@ -263,13 +263,6 @@ IN_PROC_BROWSER_TEST_F(EnableDebuggingDevTest, ShowAndCancelRemoveProtection) {
 // Show remove protection, click on [Remove protection] button and wait for
 // reboot.
 IN_PROC_BROWSER_TEST_F(EnableDebuggingDevTest, ShowAndRemoveProtection) {
-  // Disarm faked reboot, otherwise Chrome just stops and there's nothing to
-  // verify.
-  chromeos::FakePowerManagerClient* fake_power_manager_client =
-      chromeos::FakePowerManagerClient::Get();
-  ASSERT_NE(fake_power_manager_client, nullptr);
-  fake_power_manager_client->set_restart_callback(base::DoNothing());
-
   ShowRemoveProtectionScreen();
   debug_daemon_client_->ResetWait();
   test::OobeJS().TapOnPath(kRemoveProtectionButton);
@@ -280,7 +273,7 @@ IN_PROC_BROWSER_TEST_F(EnableDebuggingDevTest, ShowAndRemoveProtection) {
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(debug_daemon_client_->num_remove_protection(), 1);
   EXPECT_EQ(debug_daemon_client_->num_enable_debugging_features(), 0);
-  EXPECT_EQ(fake_power_manager_client->num_request_restart_calls(), 1);
+  EXPECT_EQ(FakePowerManagerClient::Get()->num_request_restart_calls(), 1);
 }
 
 // Show setup screen. Click on [Enable] button. Wait until done screen is shown.

chrome/browser/lifetime/application_lifetime.cc

@@ -54,7 +54,6 @@
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 #include "chrome/browser/ash/boot_times_recorder.h"
 #include "chrome/browser/ash/settings/cros_settings.h"
-#include "chrome/browser/lifetime/application_lifetime_chromeos.h"
 #include "chromeos/dbus/power/power_policy_controller.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/aura/env.h"
@@ -178,7 +177,7 @@ void AttemptRestartInternal(IgnoreUnloadHandlers ignore_unload_handlers) {
   // If an update is pending NotifyAndTerminate() will trigger a system reboot,
   // which in turn will send SIGTERM to Chrome, and that ends up processing
   // unload handlers.
-  if (UpdatePending()) {
+  if (browser_shutdown::UpdatePending()) {
     browser_shutdown::NotifyAndTerminate(true);
     return;
   }
@@ -338,18 +337,25 @@ void AttemptRestart() {
 }
 #endif  // !BUILDFLAG(IS_ANDROID)
 
-// The ChromeOS implementation is in application_lifetime_chromeos.cc
-#if !BUILDFLAG(IS_CHROMEOS_ASH)
 void AttemptRelaunch() {
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  chromeos::PowerManagerClient::Get()->RequestRestart(
+      power_manager::REQUEST_RESTART_OTHER, "Chrome relaunch");
+  // If running the Chrome OS build, but we're not on the device, fall through.
+#endif
   AttemptRestart();
 }
 
 #if !BUILDFLAG(IS_ANDROID)
 void RelaunchIgnoreUnloadHandlers() {
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  chromeos::PowerManagerClient::Get()->RequestRestart(
+      power_manager::REQUEST_RESTART_OTHER, "Chrome relaunch");
+  // If running the Chrome OS build, but we're not on the device, fall through.
+#endif
   AttemptRestartInternal(IgnoreUnloadHandlers(true));
 }
-#endif  // !BUILDFLAG(IS_ANDROID)
-#endif  // !BUILDFLAG(IS_CHROMEOS_ASH)
+#endif
 
 void AttemptExit() {
 #if BUILDFLAG(IS_CHROMEOS_ASH)

chrome/browser/lifetime/application_lifetime_chromeos_browsertest.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/lifetime/application_lifetime.h"
-#include "chrome/browser/lifetime/application_lifetime_chromeos.h"
 
 #include "base/run_loop.h"
 #include "chrome/browser/browser_process.h"
@@ -133,10 +132,10 @@ IN_PROC_BROWSER_TEST_F(ApplicationLifetimeTest, AttemptRelaunchRelaunchesOs) {
   auto* fake_power_manager_client = chromeos::FakePowerManagerClient::Get();
   EXPECT_GE(fake_power_manager_client->num_request_restart_calls(), 1);
 
-  // No restart flags set.
+  // Restart flags are set.
   PrefService* pref_service = g_browser_process->local_state();
-  EXPECT_FALSE(pref_service->GetBoolean(prefs::kWasRestarted));
-  EXPECT_FALSE(KeepAliveRegistry::GetInstance()->IsRestarting());
+  EXPECT_TRUE(pref_service->GetBoolean(prefs::kWasRestarted));
+  EXPECT_TRUE(KeepAliveRegistry::GetInstance()->IsRestarting());
 
   WaitForBrowserToClose();
 }
@@ -162,14 +161,4 @@ IN_PROC_BROWSER_TEST_F(ApplicationLifetimeTest,
   WaitForBrowserToClose();
 }
 
-IN_PROC_BROWSER_TEST_F(ApplicationLifetimeTest, RelaunchForUpdate) {
-  FakePendingUpdate();
-  RelaunchForUpdate();
-
-  // Reboot requested via update engine client.
-  EXPECT_TRUE(RequestedRebootAfterUpdate());
-
-  WaitForBrowserToClose();
-}
-
 }  // namespace chrome