Avoid using killed renderers when restarting after enrollment

When enrollment completes, WizardController may restart chrome. Part of
that restart does a fast shutdown on every RenderProcessHost, killing
all renderers.

However there may be tasks waiting to run that are IPC replies from the
webui renderer. Once such task is
EnrollmentScreenHandler::DoShowWithPartition().

The OobeConfigurationRollbackTest.TestEnterpriseRollbackRecover test
causes this restart to happen. Then as the test is ending, it runs
the message loop a number of times. If the renderer's reply happens to
be sent before being killed, and then arrives in the browser, the test
will run DoShowWithPartition(). This attempts to run JS in the
renderer which has been destroyed already. This becomes a crash when
we tear down the mojo connection with the renderer-process frame in
https://chromium-review.googlesource.com/c/chromium/src/+/2593750.

Rather than have CallJS() or some high-level object check if the
RenderFrame exists, which would hide logic errors, we inform the
EnrollmentScreen from WizardController when it is restarting chrome.
Then the EnrollmentScreenHandler can stop trying to talk to non-
existent renderer processes. We do this from WizardController because
EnrollmentScreen does not know this is happening on its own, as the
decision making about this is done up in WizardController.

R=achuith@chromium.org

Bug: 1158869
Change-Id: I83b6af31a463fa94869035f960b27bd8195a89fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629576Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Auto-Submit: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845173}

chrome/browser/chromeos/login/enrollment/enrollment_screen.cc

@@ -543,6 +543,12 @@ void EnrollmentScreen::JoinDomain(const std::string& dm_token,
       std::string() /* username */, authpolicy::ERROR_NONE);
 }
 
+void EnrollmentScreen::OnBrowserRestart() {
+  // When the browser is restarted, renderers are shutdown and the `view_`
+  // wants to know in order to stop trying to use the soon-invalid renderers.
+  view_->Shutdown();
+}
+
 void EnrollmentScreen::OnActiveDirectoryJoined(
     const std::string& machine_name,
     const std::string& username,

chrome/browser/chromeos/login/enrollment/enrollment_screen.h

@@ -87,6 +87,9 @@ class EnrollmentScreen
                   const std::string& domain_join_config,
                   OnDomainJoinedCallback on_joined_callback) override;
 
+  // Notification that the browser is being restarted.
+  void OnBrowserRestart();
+
   // Used for testing.
   EnrollmentScreenView* GetView() { return view_; }
 

chrome/browser/chromeos/login/enrollment/enrollment_screen_view.h

@@ -91,6 +91,8 @@ class EnrollmentScreenView {
 
   // Update the UI to report the `status` of the enrollment procedure.
   virtual void ShowEnrollmentStatus(policy::EnrollmentStatus status) = 0;
+
+  virtual void Shutdown() = 0;
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/login/enrollment/mock_enrollment_screen.h

@@ -57,6 +57,7 @@ class MockEnrollmentScreenView : public EnrollmentScreenView {
   MOCK_METHOD(void, ShowAuthError, (const GoogleServiceAuthError&));
   MOCK_METHOD(void, ShowOtherError, (EnterpriseEnrollmentHelper::OtherError));
   MOCK_METHOD(void, ShowEnrollmentStatus, (policy::EnrollmentStatus status));
+  MOCK_METHOD(void, Shutdown, ());
 };
 
 }  // namespace chromeos

chrome/browser/chromeos/login/wizard_controller.cc

@@ -1291,6 +1291,8 @@ void WizardController::OnEnrollmentDone() {
           policy::EnrollmentConfig::MODE_RECOVERY ||
       enrollment_mode_rollback) {
     LOG(WARNING) << "Restart Chrome to pick up the policy changes";
+    EnrollmentScreen* screen = EnrollmentScreen::Get(screen_manager());
+    screen->OnBrowserRestart();
     chrome::AttemptRestart();
     return;
   }

chrome/browser/ui/webui/chromeos/login/enrollment_screen_handler.cc

@@ -386,6 +386,10 @@ void EnrollmentScreenHandler::ShowOtherError(
   NOTREACHED();
 }
 
+void EnrollmentScreenHandler::Shutdown() {
+  shutdown_ = true;
+}
+
 void EnrollmentScreenHandler::ShowEnrollmentStatus(
     policy::EnrollmentStatus status) {
   switch (status.status()) {
@@ -846,7 +850,11 @@ void EnrollmentScreenHandler::OnGetCookiesForCompleteLogin(
 void EnrollmentScreenHandler::OnCookieWaitTimeout() {
   LOG(ERROR) << "Timeout waiting for OAuth cookie";
   oauth_code_waiter_.reset();
-  ShowError(IDS_LOGIN_FATAL_ERROR_NO_AUTH_TOKEN, true);
+
+  // If enrollment ends and the browser is being restarted, the renderers are
+  // killed so we can not talk to them anymore.
+  if (!shutdown_)
+    ShowError(IDS_LOGIN_FATAL_ERROR_NO_AUTH_TOKEN, true);
 }
 
 void EnrollmentScreenHandler::HandleAdCompleteLogin(
@@ -928,6 +936,11 @@ void EnrollmentScreenHandler::DoShow() {
 
 void EnrollmentScreenHandler::DoShowWithPartition(
     const std::string& partition_name) {
+  // If enrollment ends and the browser is being restarted, the renderers are
+  // killed so we can not talk to them anymore.
+  if (shutdown_)
+    return;
+
   base::DictionaryValue screen_data;
 
   screen_data.SetString("webviewPartitionName", partition_name);

chrome/browser/ui/webui/chromeos/login/enrollment_screen_handler.h

@@ -88,6 +88,7 @@ class EnrollmentScreenHandler
   void ShowEnrollmentStatus(policy::EnrollmentStatus status) override;
   void ShowOtherError(
       EnterpriseEnrollmentHelper::OtherError error_code) override;
+  void Shutdown() override;
 
   // Implements BaseScreenHandler:
   void Initialize() override;
@@ -180,6 +181,10 @@ class EnrollmentScreenHandler
   // True when signin screen step is shown.
   bool observe_network_failure_ = false;
 
+  // Set true when chrome is being restarted to pick up enrollment changes. The
+  // renderer processes will be destroyed and can no longer be talked to.
+  bool shutdown_ = false;
+
   // Network state informer used to keep signin screen up.
   scoped_refptr<NetworkStateInformer> network_state_informer_;