oobe: Add reboot progress dialog to UpdateScreen

Bug: 1108862
Change-Id: If36f057d4724f10f41558affe3900aee0d65a89d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2322827
Commit-Queue: Danila Kuzmin <dkuzmin@google.com>
Reviewed-by: Roman Aleksandrov <raleksandrov@google.com>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797710}

chrome/app/chromeos_strings.grdp

@@ -365,6 +365,9 @@
   <message name="IDS_UPDATE_AVAILABLE" desc="Notification for available update">
     System update available. Preparing to download&#x2026;
   </message>
+  <message name="IDS_UPDATE_COMPLETED_REBOOTING" desc="Notification for update completed and reboot is in progress">
+    Restarting to apply updates
+  </message>
   <message name="IDS_UPDATE_COMPLETED" desc="Notification for update completed">
     System update complete. Please restart the system.
   </message>

chrome/app/chromeos_strings_grdp/IDS_UPDATE_COMPLETED_REBOOTING.png.sha1

+5b1d9d9d154116b2fbcd85c2fad7dba0f521f1fa
\ No newline at end of file

chrome/browser/chromeos/login/screens/mock_update_screen.h

@@ -41,6 +41,7 @@ class MockUpdateView : public UpdateView {
   MOCK_METHOD(void, SetShowEstimatedTimeLeft, (bool value));
   MOCK_METHOD(void, SetUpdateCompleted, (bool value));
   MOCK_METHOD(void, SetShowCurtain, (bool value));
+  MOCK_METHOD(void, SetManualRebootNeeded, (bool value));
   MOCK_METHOD(void, SetProgressMessage, (const base::string16& value));
   MOCK_METHOD(void, SetProgress, (int value));
   MOCK_METHOD(void, SetRequiresPermissionForCellular, (bool value));

chrome/browser/chromeos/login/screens/update_screen.cc

@@ -36,6 +36,10 @@ constexpr const char kUserActionCancelUpdateShortcut[] = "cancel-update";
 
 const char kUpdateDeadlineFile[] = "/tmp/update-check-response-deadline";
 
+// Time in seconds after which we initiate reboot.
+constexpr const base::TimeDelta kWaitBeforeRebootTime =
+    base::TimeDelta::FromSeconds(2);
+
 // Delay before showing error message if captive portal is detected.
 // We wait for this delay to let captive portal to perform redirect and show
 // its login page before error message appears.
@@ -108,6 +112,7 @@ UpdateScreen::UpdateScreen(UpdateView* view,
       histogram_helper_(
           std::make_unique<ErrorScreensHistogramHelper>("Update")),
       version_updater_(std::make_unique<VersionUpdater>(this)),
+      wait_before_reboot_time_(kWaitBeforeRebootTime),
       tick_clock_(base::DefaultTickClock::GetInstance()) {
   if (chromeos::features::IsBetterUpdateEnabled())
     PowerManagerClient::Get()->AddObserver(this);
@@ -216,8 +221,13 @@ void UpdateScreen::ExitUpdate(Result result) {
 void UpdateScreen::OnWaitForRebootTimeElapsed() {
   LOG(ERROR) << "Unable to reboot - asking user for a manual reboot.";
   MakeSureScreenIsShown();
-  if (view_)
+  if (!view_)
+    return;
+  if (chromeos::features::IsBetterUpdateEnabled()) {
+    view_->SetManualRebootNeeded(true);
+  } else {
     view_->SetUpdateCompleted(true);
+  }
 }
 
 void UpdateScreen::PrepareForUpdateCheck() {
@@ -344,7 +354,14 @@ void UpdateScreen::UpdateInfoChanged(
                            finalize_time_);
         RecordDownloadingTime(tick_clock_->NowTicks() -
                               start_update_downloading_);
-        version_updater_->RebootAfterUpdate();
+        if (chromeos::features::IsBetterUpdateEnabled()) {
+          ShowRebootInProgress();
+          wait_reboot_timer_.Start(FROM_HERE, wait_before_reboot_time_,
+                                   version_updater_.get(),
+                                   &VersionUpdater::RebootAfterUpdate);
+        } else {
+          version_updater_->RebootAfterUpdate();
+        }
       } else {
         hide_progress_on_exit_ = true;
         ExitUpdate(Result::UPDATE_NOT_REQUIRED);
@@ -387,6 +404,12 @@ void UpdateScreen::PowerChanged(
   UpdateBatteryWarningVisibility();
 }
 
+void UpdateScreen::ShowRebootInProgress() {
+  MakeSureScreenIsShown();
+  if (view_)
+    view_->SetUpdateCompleted(true);
+}
+
 void UpdateScreen::UpdateBatteryWarningVisibility() {
   if (!view_)
     return;

chrome/browser/chromeos/login/screens/update_screen.h

@@ -107,6 +107,15 @@ class UpdateScreen : public BaseScreen,
     tick_clock_ = tick_clock;
   }
 
+  void set_wait_before_reboot_time_for_testing(
+      base::TimeDelta wait_before_reboot_time) {
+    wait_before_reboot_time_ = wait_before_reboot_time;
+  }
+
+  base::OneShotTimer* GetWaitRebootTimerForTesting() {
+    return &wait_reboot_timer_;
+  }
+
  protected:
   // BaseScreen:
   bool MaybeSkip(WizardContext* context) override;
@@ -144,6 +153,9 @@ class UpdateScreen : public BaseScreen,
   // stages. Called when power or update status changes.
   void UpdateBatteryWarningVisibility();
 
+  // Show reboot waiting screen.
+  void ShowRebootInProgress();
+
   UpdateView* view_;
   ErrorScreen* error_screen_;
   ScreenExitCallback exit_callback_;
@@ -187,6 +199,13 @@ class UpdateScreen : public BaseScreen,
   // instead.
   base::OneShotTimer error_message_timer_;
 
+  // Timer for the interval to wait for the reboot progress screen to be shown
+  // for at least wait_before_reboot_time_ before reboot call.
+  base::OneShotTimer wait_reboot_timer_;
+
+  // Time in seconds after which we initiate reboot.
+  base::TimeDelta wait_before_reboot_time_;
+
   const base::TickClock* tick_clock_;
 
   base::TimeTicks start_update_downloading_;

chrome/browser/chromeos/login/screens/update_screen_browsertest.cc

@@ -12,6 +12,7 @@
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "base/time/time.h"
 #include "chrome/browser/chromeos/login/login_wizard.h"
@@ -66,6 +67,11 @@ const test::UIPath kLowBatteryWarningMessage = {"oobe-update",
                                                 "battery-warning"};
 const test::UIPath kErrorMessage = {"error-message"};
 
+// Paths for better update screen https://crbug.com/1101317
+const test::UIPath kRestartingDialog = {"oobe-update", "restarting-dialog"};
+const test::UIPath kBetterUpdateCompletedDialog = {
+    "oobe-update", "better-update-complete-dialog"};
+
 // UMA names for better test reading.
 const char kTimeCheck[] = "OOBE.UpdateScreen.StageTime.Check";
 const char kTimeDownload[] = "OOBE.UpdateScreen.StageTime.Download";
@@ -86,6 +92,8 @@ constexpr base::TimeDelta kTimeAdvanceSeconds10 =
     base::TimeDelta::FromSeconds(10);
 constexpr base::TimeDelta kTimeAdvanceSeconds60 =
     base::TimeDelta::FromSeconds(60);
+constexpr base::TimeDelta kTimeDefaultWaiting =
+    base::TimeDelta::FromSeconds(10);
 
 std::string GetDownloadingString(int status_resource_id) {
   return l10n_util::GetStringFUTF8(
@@ -181,6 +189,17 @@ class BetterUpdateScreenTest : public UpdateScreenTest {
   }
   ~BetterUpdateScreenTest() override = default;
 
+  void SetTickClockAndDefaultDelaysForTesting(
+      const base::TickClock* tick_clock) {
+    version_updater_->set_tick_clock_for_testing(tick_clock);
+    update_screen_->set_tick_clock_for_testing(tick_clock);
+    // Set time for waiting in the test to not update constants manually, if
+    // they change.
+    version_updater_->set_wait_for_reboot_time_for_testing(kTimeDefaultWaiting);
+    update_screen_->set_wait_before_reboot_time_for_testing(
+        kTimeDefaultWaiting);
+  }
+
  protected:
   chromeos::FakePowerManagerClient* power_manager_client() {
     return chromeos::FakePowerManagerClient::Get();
@@ -778,6 +797,9 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest, TestInitialLowBatteryStatus) {
 
 IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest,
                        TestBatteryWarningDuringUpdateStages) {
+  base::ScopedMockTimeMessageLoopTaskRunner mocked_task_runner;
+  SetTickClockAndDefaultDelaysForTesting(
+      mocked_task_runner->GetMockTickClock());
   update_screen_->set_ignore_update_deadlines_for_testing(true);
   ShowUpdateScreen();
   EXPECT_TRUE(power_manager_client()->HasObserver(update_screen_));
@@ -832,6 +854,10 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest,
   update_engine_client()->set_default_status(status);
   update_engine_client()->NotifyObserversThatStatusChanged(status);
 
+  // Show waiting for reboot screen for several seconds.
+  ASSERT_TRUE(update_screen_->GetWaitRebootTimerForTesting()->IsRunning());
+  mocked_task_runner->FastForwardBy(kTimeDefaultWaiting);
+
   // UpdateStatusChanged(status) calls RebootAfterUpdate().
   EXPECT_EQ(update_engine_client()->reboot_after_update_call_count(), 1);
   test::OobeJS().ExpectVisiblePath(kLowBatteryWarningMessage);
@@ -884,4 +910,42 @@ IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest,
   test::OobeJS().ExpectHiddenPath(kLowBatteryWarningMessage);
 }
 
+IN_PROC_BROWSER_TEST_F(BetterUpdateScreenTest,
+                       TestUpdateCompletedRebootNeeded) {
+  base::ScopedMockTimeMessageLoopTaskRunner mocked_task_runner;
+  SetTickClockAndDefaultDelaysForTesting(
+      mocked_task_runner->GetMockTickClock());
+  update_screen_->set_ignore_update_deadlines_for_testing(true);
+  ShowUpdateScreen();
+
+  update_engine::StatusResult status;
+  status.set_current_operation(update_engine::Operation::UPDATED_NEED_REBOOT);
+  status.set_new_version("latest and greatest");
+  status.set_new_size(1'000'000'000);
+  update_engine_client()->set_default_status(status);
+  update_engine_client()->NotifyObserversThatStatusChanged(status);
+
+  test::OobeJS().CreateVisibilityWaiter(true, kRestartingDialog)->Wait();
+  test::OobeJS().ExpectHiddenPath(kCheckingForUpdatesDialog);
+  test::OobeJS().ExpectHiddenPath(kCellularPermissionDialog);
+  test::OobeJS().ExpectHiddenPath(kUpdatingDialog);
+  test::OobeJS().ExpectHiddenPath(kBetterUpdateCompletedDialog);
+
+  // Make sure that after the screen is shown waiting timer starts.
+  mocked_task_runner->RunUntilIdle();
+  // Show waiting for reboot screen for several seconds.
+  ASSERT_TRUE(update_screen_->GetWaitRebootTimerForTesting()->IsRunning());
+  mocked_task_runner->FastForwardBy(kTimeDefaultWaiting);
+
+  // UpdateStatusChanged(status) calls RebootAfterUpdate().
+  ASSERT_EQ(update_engine_client()->reboot_after_update_call_count(), 1);
+
+  // Simulate the situation where reboot does not happen in time.
+  ASSERT_TRUE(version_updater_->GetRebootTimerForTesting()->IsRunning());
+  mocked_task_runner->FastForwardBy(kTimeDefaultWaiting);
+
+  test::OobeJS().ExpectHiddenPath(kRestartingDialog);
+  test::OobeJS().ExpectVisiblePath(kBetterUpdateCompletedDialog);
+}
+
 }  // namespace chromeos

chrome/browser/resources/chromeos/login/oobe_update.html

@@ -50,7 +50,26 @@
       </oobe-dialog>
     </div>
     <div hidden="[[!betterUpdateScreenFeatureEnabled_]]">
-      <oobe-dialog>
+      <oobe-dialog footer-shrinkable id="restarting-dialog"
+          hidden="[[!isRebootProgressBarShown_(manualRebootNeeded,
+                                               updateCompleted)]]"
+          title-key="updateCompeletedRebootingMsg"
+          aria-live="polite">
+        <hd-iron-icon slot="oobe-icon"
+            icon1x="oobe-32:googleg" icon2x="oobe-64:googleg">
+        </hd-iron-icon>
+        <paper-progress slot="progress" id="restarting-progress" indeterminate>
+        </paper-progress>
+      </oobe-dialog>
+      <oobe-dialog footer-shrinkable id="better-update-complete-dialog"
+          hidden="[[!manualRebootNeeded]]"
+          title-key="updateCompeletedMsg"
+          aria-live="polite">
+        <hd-iron-icon slot="oobe-icon"
+            icon1x="oobe-32:googleg" icon2x="oobe-64:googleg">
+        </hd-iron-icon>
+      </oobe-dialog>
+      <oobe-dialog hidden="[[!showLowBatteryWarning]]">
         <div hidden="[[!showLowBatteryWarning]]" id="battery-warning"
             slot="footer">
         </div>

chrome/browser/resources/chromeos/login/oobe_update.js

@@ -23,6 +23,7 @@ Polymer({
     'setEstimatedTimeLeft',
     'showEstimatedTimeLeft',
     'setUpdateCompleted',
+    'setManualRebootNeeded',
     'showUpdateCurtain',
     'setProgressMessage',
     'setUpdateProgress',
@@ -100,6 +101,14 @@ Polymer({
       value: false,
     },
 
+    /**
+     * True if update is fully completed and manual action is required.
+     */
+    manualRebootNeeded: {
+      type: Boolean,
+      value: false,
+    },
+
     /**
      * If update cancellation is allowed.
      */
@@ -140,8 +149,10 @@ Polymer({
   },
 
   onBeforeShow() {
-    cr.ui.login.invokePolymerMethod(
-        this.$['checking-downloading-update'], 'onBeforeShow');
+    if (!this.betterUpdateScreenFeatureEnabled_) {
+      cr.ui.login.invokePolymerMethod(
+          this.$['checking-downloading-update'], 'onBeforeShow');
+    }
   },
 
   onBackClicked_() {
@@ -209,6 +220,13 @@ Polymer({
     this.updateCompleted = is_completed;
   },
 
+  /**
+   * @param {boolean} is_needed True if manual reboot after update is needed.
+   */
+  setManualRebootNeeded(is_needed) {
+    this.manualRebootNeeded = is_needed;
+  },
+
   /**
    * Shows or hides update curtain.
    * @param {boolean} visible Are curtains visible?
@@ -225,5 +243,15 @@ Polymer({
     this.showLowBatteryWarning = visible;
   },
 
+  /**
+   * Calculates visibility of the reboot progress dialog.
+   * @param {Boolean} manualRebootNeeded If the automatic reboot timer has
+   * elapsed and manual reboot is needed.
+   * @param {Boolean} updateCompleted If update is completed and all
+   * intermediate status elements are hidden.
+   */
+  isRebootProgressBarShown_(manualRebootNeeded, updateCompleted) {
+    return updateCompleted && !manualRebootNeeded;
+  },
 });
 })();

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

@@ -62,6 +62,10 @@ void UpdateScreenHandler::SetUpdateCompleted(bool value) {
   CallJS("login.UpdateScreen.setUpdateCompleted", value);
 }
 
+void UpdateScreenHandler::SetManualRebootNeeded(bool value) {
+  CallJS("login.UpdateScreen.setManualRebootNeeded", value);
+}
+
 void UpdateScreenHandler::SetShowCurtain(bool value) {
   CallJS("login.UpdateScreen.showUpdateCurtain", value);
 }
@@ -92,6 +96,7 @@ void UpdateScreenHandler::DeclareLocalizedValues(
   builder->AddF("installingUpdateDesc", IDS_UPDATE_MSG,
                 ui::GetChromeOSDeviceName());
   builder->Add("updateCompeletedMsg", IDS_UPDATE_COMPLETED);
+  builder->Add("updateCompeletedRebootingMsg", IDS_UPDATE_COMPLETED_REBOOTING);
   builder->Add("updateScreenAccessibleTitle",
                IDS_UPDATE_SCREEN_ACCESSIBLE_TITLE);
   builder->Add("checkingForUpdates", IDS_CHECKING_FOR_UPDATES);

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

@@ -39,6 +39,7 @@ class UpdateView {
   virtual void SetEstimatedTimeLeft(int value) = 0;
   virtual void SetShowEstimatedTimeLeft(bool value) = 0;
   virtual void SetUpdateCompleted(bool value) = 0;
+  virtual void SetManualRebootNeeded(bool value) = 0;
   virtual void SetShowCurtain(bool value) = 0;
   virtual void SetProgressMessage(const base::string16& value) = 0;
   virtual void SetProgress(int value) = 0;
@@ -63,6 +64,7 @@ class UpdateScreenHandler : public UpdateView, public BaseScreenHandler {
   void SetEstimatedTimeLeft(int value) override;
   void SetShowEstimatedTimeLeft(bool value) override;
   void SetUpdateCompleted(bool value) override;
+  void SetManualRebootNeeded(bool value) override;
   void SetShowCurtain(bool value) override;
   void SetProgressMessage(const base::string16& value) override;
   void SetProgress(int value) override;