Increase the relaunch deadline if the user has potentially seen the

notification and the relaunch deadline is in less than the grace period.

Bug: 1044716
Change-Id: I299bb6a62029826b40a7fb739f332bd868bf75c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041600
Commit-Queue: Aya Elsayed <ayaelattar@google.com>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743502}

chrome/browser/ui/views/relaunch_notification/DEPS

+specific_include_rules = {
+  "relaunch_notification_controller_platform_impl_chromeos\.cc": [
+    "+ash/shell.h",
+  ],
+  "relaunch_notification_controller_unittest\.cc": [
+    "+ash/shell.h",
+    "+ash/test/ash_test_helper.h"
+  ],
+}

chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller.h"
 
 #include <algorithm>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -231,8 +232,8 @@ void RelaunchNotificationController::HandleRelaunchRequiredState(
   DCHECK_EQ(last_notification_style_, NotificationStyle::kRequired);
 
   // Make no changes if the new deadline is not in the future and the browser is
-  // within the grace period of the previous deadline. The user has already been
-  // given the fifteen-minutes countdown so just let it go.
+  // within the grace period of the previous deadline. The user has already seen
+  // the one-hour countdown so just let it go.
   const base::Time now = clock_->Now();
   if (timer_.IsRunning()) {
     const base::Time& desired_run_time = timer_.desired_run_time();
@@ -241,7 +242,7 @@ void RelaunchNotificationController::HandleRelaunchRequiredState(
       return;
   }
 
-  // Compute the new deadline (minimally fifteen minutes into the future).
+  // Compute the new deadline (minimally one hour into the future).
   const base::Time deadline =
       std::max(high_deadline, now) + kRelaunchGracePeriod;
 
@@ -250,7 +251,6 @@ void RelaunchNotificationController::HandleRelaunchRequiredState(
                &RelaunchNotificationController::OnRelaunchDeadlineExpired);
 
   if (platform_impl_.IsRequiredNotificationShown()) {
-    // Tell the notification to update its title if it is showing.
     platform_impl_.SetDeadline(deadline);
   } else {
     // Otherwise, show the dialog if there has been a level change or if the
@@ -259,6 +259,22 @@ void RelaunchNotificationController::HandleRelaunchRequiredState(
       NotifyRelaunchRequired();
   }
 }
+
+base::Time RelaunchNotificationController::IncreaseRelaunchDeadlineOnShow() {
+  DCHECK(timer_.IsRunning());
+  DCHECK(!timer_.desired_run_time().is_null());
+  base::Time relaunch_deadline = timer_.desired_run_time();
+
+  // Push the dealdine back if needed so that the user has at least the grace
+  // period to decide what to do.
+  relaunch_deadline =
+      std::max(clock_->Now() + kRelaunchGracePeriod, relaunch_deadline);
+
+  timer_.Start(FROM_HERE, relaunch_deadline, this,
+               &RelaunchNotificationController::OnRelaunchDeadlineExpired);
+  return relaunch_deadline;
+}
+
 void RelaunchNotificationController::StartReshowTimer() {
   DCHECK_EQ(last_notification_style_, NotificationStyle::kRecommended);
   DCHECK(!last_relaunch_notification_time_.is_null());
@@ -292,18 +308,28 @@ void RelaunchNotificationController::DoNotifyRelaunchRecommended(
 void RelaunchNotificationController::NotifyRelaunchRequired() {
   DCHECK(timer_.IsRunning());
   DCHECK(!timer_.desired_run_time().is_null());
-  DoNotifyRelaunchRequired(timer_.desired_run_time());
+  DoNotifyRelaunchRequired(
+      timer_.desired_run_time(),
+      base::BindOnce(
+          &RelaunchNotificationController::IncreaseRelaunchDeadlineOnShow,
+          base::Unretained(this)));
 }
 
 void RelaunchNotificationController::DoNotifyRelaunchRequired(
-    base::Time deadline) {
-  platform_impl_.NotifyRelaunchRequired(deadline);
+    base::Time relaunch_deadline,
+    base::OnceCallback<base::Time()> on_visible) {
+  platform_impl_.NotifyRelaunchRequired(relaunch_deadline,
+                                        std::move(on_visible));
 }
 
 void RelaunchNotificationController::Close() {
   platform_impl_.CloseRelaunchNotification();
 }
 
+void RelaunchNotificationController::SetDeadline(base::Time deadline) {
+  platform_impl_.SetDeadline(deadline);
+}
+
 void RelaunchNotificationController::OnRelaunchDeadlineExpired() {
   chrome::RelaunchIgnoreUnloadHandlers();
 }

chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_RELAUNCH_NOTIFICATION_RELAUNCH_NOTIFICATION_CONTROLLER_H_
 #define CHROME_BROWSER_UI_VIEWS_RELAUNCH_NOTIFICATION_RELAUNCH_NOTIFICATION_CONTROLLER_H_
 
+#include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "chrome/browser/ui/views/relaunch_notification/wall_clock_timer.h"
@@ -38,7 +39,7 @@ class TickClock;
 //
 // - Required (2): The controller displays the relaunch required dialog on each
 //   change to the UpgradeDetector's upgrade_notification_stage (described
-//   above). The browser is relaunched fifteen minutes after the third and final
+//   above). The browser is relaunched one hour after the third and final
 //   showing of the dialog (which takes place when the UpgradeDetector reaches
 //   the high annoyance level).
 //
@@ -57,12 +58,16 @@ class RelaunchNotificationController : public UpgradeObserver {
   // summarily relaunched on Chrome desktop, or the device is rebooted on
   // Chrome OS.
   static constexpr base::TimeDelta kRelaunchGracePeriod =
-      base::TimeDelta::FromMinutes(60);
+      base::TimeDelta::FromHours(1);
 
   RelaunchNotificationController(UpgradeDetector* upgrade_detector,
                                  const base::Clock* clock,
                                  const base::TickClock* tick_clock);
 
+  // The deadline may be extended to ensure that the user has at least the full
+  // duration of the grace period to take action.
+  base::Time IncreaseRelaunchDeadlineOnShow();
+
   // UpgradeObserver:
   void OnUpgradeRecommended() override;
 
@@ -122,8 +127,8 @@ class RelaunchNotificationController : public UpgradeObserver {
   // Recommended deadline was already passed or not.
   void NotifyRelaunchRecommended(bool past_deadline);
 
-  // Provide deadline to DoNotifyRelaunchRequired.
-  virtual void NotifyRelaunchRequired();
+  // Calls DoNotifyRelaunchRequired to show the notification.
+  void NotifyRelaunchRequired();
 
   // The following methods, which are invoked by the controller to show or close
   // notifications, are virtual for the sake of testing.
@@ -134,12 +139,20 @@ class RelaunchNotificationController : public UpgradeObserver {
   virtual void DoNotifyRelaunchRecommended(bool past_deadline);
 
   // Shows the relaunch required notification if it is not already open.
-  virtual void DoNotifyRelaunchRequired(base::Time deadline);
+  // |on_visible| is a callback to be run when the notification is potentially
+  // seen by the user to push back the relaunch deadline if the remaining time
+  // is less than the grace period.
+  virtual void DoNotifyRelaunchRequired(
+      base::Time relaunch_deadline,
+      base::OnceCallback<base::Time()> on_visible);
 
   // Closes bubble or dialog if either is still open on desktop, or sets the
   // default notification on Chrome OS.
   virtual void Close();
 
+  // Updates the required relaunch deadline in the UX.
+  virtual void SetDeadline(base::Time deadline);
+
   // Run to restart the browser/device once the relaunch deadline is reached
   // when relaunches are required by policy.
   virtual void OnRelaunchDeadlineExpired();

chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_chromeos.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_chromeos.h"
 
 #include "ash/public/cpp/update_types.h"
+#include "ash/shell.h"
 #include "base/bind.h"
 #include "chrome/browser/ui/ash/system_tray_client.h"
 #include "chrome/browser/ui/views/relaunch_notification/relaunch_notification_metrics.h"
@@ -35,7 +36,8 @@ void RelaunchNotificationControllerPlatformImpl::RecordRecommendedShowResult() {
 }
 
 void RelaunchNotificationControllerPlatformImpl::NotifyRelaunchRequired(
-    base::Time deadline) {
+    base::Time deadline,
+    base::OnceCallback<base::Time()> on_visible) {
   if (!relaunch_required_timer_) {
     relaunch_required_timer_ = std::make_unique<RelaunchRequiredTimer>(
         deadline,
@@ -48,6 +50,13 @@ void RelaunchNotificationControllerPlatformImpl::NotifyRelaunchRequired(
   }
 
   RefreshRelaunchRequiredTitle();
+
+  if (!CanScheduleReboot()) {
+    on_visible_ = std::move(on_visible);
+    StartObserving();
+  } else {
+    StopObserving();
+  }
 }
 
 void RelaunchNotificationControllerPlatformImpl::CloseRelaunchNotification() {
@@ -55,11 +64,14 @@ void RelaunchNotificationControllerPlatformImpl::CloseRelaunchNotification() {
       ash::NotificationStyle::kDefault, base::string16(), base::string16());
   recorded_shown_ = false;
   relaunch_required_timer_.reset();
+  on_visible_.Reset();
+  StopObserving();
 }
 
 void RelaunchNotificationControllerPlatformImpl::SetDeadline(
     base::Time deadline) {
-  relaunch_required_timer_->SetDeadline(deadline);
+  if (relaunch_required_timer_)
+    relaunch_required_timer_->SetDeadline(deadline);
 }
 
 void RelaunchNotificationControllerPlatformImpl::
@@ -77,15 +89,53 @@ void RelaunchNotificationControllerPlatformImpl::
   }
 }
 
+bool RelaunchNotificationControllerPlatformImpl::IsRequiredNotificationShown()
+    const {
+  return relaunch_required_timer_ != nullptr;
+}
+
 void RelaunchNotificationControllerPlatformImpl::
     RefreshRelaunchRequiredTitle() {
-  SystemTrayClient::Get()->SetUpdateNotificationState(
-      ash::NotificationStyle::kAdminRequired,
-      relaunch_required_timer_->GetWindowTitle(),
-      l10n_util::GetStringUTF16(IDS_RELAUNCH_REQUIRED_BODY));
+  // SystemTrayClient may not exist in unit tests.
+  if (SystemTrayClient::Get()) {
+    SystemTrayClient::Get()->SetUpdateNotificationState(
+        ash::NotificationStyle::kAdminRequired,
+        relaunch_required_timer_->GetWindowTitle(),
+        l10n_util::GetStringUTF16(IDS_RELAUNCH_REQUIRED_BODY));
+  }
 }
 
-bool RelaunchNotificationControllerPlatformImpl::IsRequiredNotificationShown()
-    const {
-  return relaunch_required_timer_ != nullptr;
+void RelaunchNotificationControllerPlatformImpl::OnPowerStateChanged(
+    chromeos::DisplayPowerState power_state) {
+  if (CanScheduleReboot() && on_visible_) {
+    base::Time new_deadline = std::move(on_visible_).Run();
+    SetDeadline(new_deadline);
+    StopObserving();
+  }
+}
+
+void RelaunchNotificationControllerPlatformImpl::OnSessionStateChanged() {
+  if (CanScheduleReboot() && on_visible_) {
+    base::Time new_deadline = std::move(on_visible_).Run();
+    SetDeadline(new_deadline);
+    StopObserving();
+  }
+}
+
+bool RelaunchNotificationControllerPlatformImpl::CanScheduleReboot() {
+  return ash::Shell::Get()->display_configurator()->IsDisplayOn() &&
+         session_manager::SessionManager::Get()->session_state() ==
+             session_manager::SessionState::ACTIVE;
+}
+
+void RelaunchNotificationControllerPlatformImpl::StartObserving() {
+  if (!display_observer_.IsObservingSources())
+    display_observer_.Add(ash::Shell::Get()->display_configurator());
+  if (!session_observer_.IsObservingSources())
+    session_observer_.Add(session_manager::SessionManager::Get());
+}
+
+void RelaunchNotificationControllerPlatformImpl::StopObserving() {
+  display_observer_.RemoveAll();
+  session_observer_.RemoveAll();
 }

chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_chromeos.h

@@ -7,21 +7,29 @@
 
 #include <memory>
 
+#include "base/callback.h"
+#include "base/scoped_observer.h"
 #include "base/time/time.h"
+#include "components/session_manager/core/session_manager.h"
+#include "components/session_manager/core/session_manager_observer.h"
+#include "ui/display/manager/display_configurator.h"
 
 class RelaunchRequiredTimer;
 
-class RelaunchNotificationControllerPlatformImpl {
+class RelaunchNotificationControllerPlatformImpl
+    : public display::DisplayConfigurator::Observer,
+      public session_manager::SessionManagerObserver {
  public:
   RelaunchNotificationControllerPlatformImpl();
 
-  ~RelaunchNotificationControllerPlatformImpl();
+  ~RelaunchNotificationControllerPlatformImpl() override;
 
   // Shows the relaunch recommended notification if it is not already open.
   void NotifyRelaunchRecommended(base::Time detection_time, bool past_deadline);
 
   // Shows the relaunch required notification if it is not already open.
-  void NotifyRelaunchRequired(base::Time deadline);
+  void NotifyRelaunchRequired(base::Time deadline,
+                              base::OnceCallback<base::Time()> on_visible);
 
   // Sets the notification title to the default one on Chrome OS.
   void CloseRelaunchNotification();
@@ -33,6 +41,12 @@ class RelaunchNotificationControllerPlatformImpl {
   // Returns true if relaunch required notification is shown.
   bool IsRequiredNotificationShown() const;
 
+  // display::DisplayConfigurator::Observer overrides.
+  void OnPowerStateChanged(chromeos::DisplayPowerState power_state) override;
+
+  // session_manager::SessionManagerObserver overrides.
+  void OnSessionStateChanged() override;
+
  private:
   // Callback triggered whenever the recommended notification's title has to
   // refresh.
@@ -45,6 +59,15 @@ class RelaunchNotificationControllerPlatformImpl {
   // refresh.
   void RefreshRelaunchRequiredTitle();
 
+  // Returns true if the display is on && the session is active
+  bool CanScheduleReboot();
+
+  // Registers itself to observe display & session state changes
+  void StartObserving();
+
+  // Removes itself from observe display & session state observers
+  void StopObserving();
+
   // Timer that takes care of the string refresh in the relaunch required
   // notification title.
   std::unique_ptr<RelaunchRequiredTimer> relaunch_required_timer_;
@@ -52,6 +75,15 @@ class RelaunchNotificationControllerPlatformImpl {
   // Indicate that show of the Recommended notification was already recorded.
   bool recorded_shown_ = false;
 
+  base::OnceCallback<base::Time()> on_visible_;
+
+  ScopedObserver<display::DisplayConfigurator,
+                 display::DisplayConfigurator::Observer>
+      display_observer_{this};
+  ScopedObserver<session_manager::SessionManager,
+                 session_manager::SessionManagerObserver>
+      session_observer_{this};
+
   DISALLOW_COPY_AND_ASSIGN(RelaunchNotificationControllerPlatformImpl);
 };
 

chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/views/relaunch_notification/relaunch_notification_metrics.h"
 #include "chrome/browser/ui/views/relaunch_notification/relaunch_recommended_bubble_view.h"
@@ -51,6 +52,13 @@ Browser* FindLastActiveTabbedBrowser() {
 RelaunchNotificationControllerPlatformImpl::
     RelaunchNotificationControllerPlatformImpl() = default;
 
+RelaunchNotificationControllerPlatformImpl::
+    ~RelaunchNotificationControllerPlatformImpl() {
+  DCHECK(!widget_);
+  if (on_visible_)
+    BrowserList::RemoveObserver(this);
+}
+
 void RelaunchNotificationControllerPlatformImpl::NotifyRelaunchRecommended(
     base::Time detection_time,
     bool /*past_deadline*/) {
@@ -74,35 +82,58 @@ void RelaunchNotificationControllerPlatformImpl::NotifyRelaunchRecommended(
 }
 
 void RelaunchNotificationControllerPlatformImpl::NotifyRelaunchRequired(
-    base::Time deadline) {
+    base::Time deadline,
+    base::OnceCallback<base::Time()> on_visible) {
   // Nothing to do if the dialog is visible.
   if (widget_)
     return;
 
-  // Show the dialog in the most recently active browser.
-  Browser* browser = FindLastActiveTabbedBrowser();
-  relaunch_notification::RecordRequiredShowResult(
-      browser ? relaunch_notification::ShowResult::kShown
-              : GetNotShownReason());
-  if (!browser)
+  // Show the dialog in the active tabbed browser window.
+  Browser* browser = chrome::FindBrowserWithActiveWindow();
+  if (browser && browser->is_type_normal()) {
+    DCHECK(!on_visible_);
+    ShowRequiredNotification(browser, deadline);
+    relaunch_notification::RecordRequiredShowResult(
+        relaunch_notification::ShowResult::kShown);
     return;
+  }
 
-  widget_ = RelaunchRequiredDialogView::Show(
-      browser, deadline, base::BindRepeating(&chrome::AttemptRelaunch));
+  relaunch_notification::RecordRequiredShowResult(GetNotShownReason());
 
-  // Monitor the widget so that |widget_| can be cleared on close.
-  widget_->AddObserver(this);
+  // If the instance is not already waiting for one to become active from a
+  // previous call, start observing now.
+  if (!on_visible_)
+    BrowserList::AddObserver(this);
+
+  // Hold on to the callback until an active tabbed browser is found.
+  on_visible_ = std::move(on_visible);
+
+  last_relaunch_deadline_ = deadline;
 }
 
 void RelaunchNotificationControllerPlatformImpl::CloseRelaunchNotification() {
   if (widget_)
     widget_->Close();
+  if (on_visible_) {
+    BrowserList::RemoveObserver(this);
+    on_visible_.Reset();
+    last_relaunch_deadline_ = base::Time();
+  }
+  has_shown_ = false;
 }
 
 void RelaunchNotificationControllerPlatformImpl::SetDeadline(
     base::Time deadline) {
-  DCHECK(widget_);
-  RelaunchRequiredDialogView::FromWidget(widget_)->SetDeadline(deadline);
+  // Nothing to do if the dialog hasn't been shown yet (because no tabbed
+  // browser has become active) or if the user has seen and dismissed the
+  // dialog.
+  if (widget_)
+    RelaunchRequiredDialogView::FromWidget(widget_)->SetDeadline(deadline);
+
+  // Hold on to the new deadline if the instance is waiting for a Browser to
+  // become active.
+  if (on_visible_)
+    last_relaunch_deadline_ = deadline;
 }
 
 bool RelaunchNotificationControllerPlatformImpl::IsRequiredNotificationShown()
@@ -116,3 +147,39 @@ void RelaunchNotificationControllerPlatformImpl::OnWidgetClosing(
   widget->RemoveObserver(this);
   widget_ = nullptr;
 }
+
+void RelaunchNotificationControllerPlatformImpl::OnWidgetDestroying(
+    views::Widget* widget) {
+  DCHECK_EQ(widget, widget_);
+  widget->RemoveObserver(this);
+  widget_ = nullptr;
+}
+
+void RelaunchNotificationControllerPlatformImpl::OnBrowserSetLastActive(
+    Browser* browser) {
+  // Ignore non-tabbed browsers.
+  if (!browser->is_type_normal())
+    return;
+
+  BrowserList::RemoveObserver(this);
+
+  base::Time new_deadline =
+      has_shown_ ? last_relaunch_deadline_ : std::move(on_visible_).Run();
+  DCHECK(!new_deadline.is_null());
+
+  on_visible_.Reset();
+  last_relaunch_deadline_ = base::Time();
+
+  ShowRequiredNotification(browser, new_deadline);
+}
+
+void RelaunchNotificationControllerPlatformImpl::ShowRequiredNotification(
+    Browser* browser,
+    base::Time deadline) {
+  widget_ = RelaunchRequiredDialogView::Show(
+      browser, deadline, base::BindRepeating(&chrome::AttemptRelaunch));
+  has_shown_ = true;
+
+  // Monitor the widget so that |widget_| can be cleared on close/destruction.
+  widget_->AddObserver(this);
+}

chrome/browser/ui/views/relaunch_notification/relaunch_notification_controller_platform_impl_desktop.h

@@ -5,24 +5,30 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_RELAUNCH_NOTIFICATION_RELAUNCH_NOTIFICATION_CONTROLLER_PLATFORM_IMPL_DESKTOP_H_
 #define CHROME_BROWSER_UI_VIEWS_RELAUNCH_NOTIFICATION_RELAUNCH_NOTIFICATION_CONTROLLER_PLATFORM_IMPL_DESKTOP_H_
 
+#include "base/callback.h"
 #include "base/time/time.h"
-
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "ui/views/widget/widget_observer.h"
 
 namespace views {
 class Widget;
 }
 
-class RelaunchNotificationControllerPlatformImpl
-    : public views::WidgetObserver {
+class RelaunchNotificationControllerPlatformImpl : public views::WidgetObserver,
+                                                   public BrowserListObserver {
  public:
   RelaunchNotificationControllerPlatformImpl();
+  ~RelaunchNotificationControllerPlatformImpl() override;
 
   // Shows the relaunch recommended notification if it is not already open.
   void NotifyRelaunchRecommended(base::Time detection_time, bool past_deadline);
 
-  // Shows the relaunch required notification if it is not already open.
-  void NotifyRelaunchRequired(base::Time deadline);
+  // Shows the relaunch required notification in the most recently active
+  // browser. window if it is not already open.  |on_visible| is run when the
+  // notification is potentially seen to push the deadline back if the remaining
+  // time is less than the grace period.
+  void NotifyRelaunchRequired(base::Time deadline,
+                              base::OnceCallback<base::Time()> on_visible);
 
   // Closes the bubble or dialog if either is still open.
   void CloseRelaunchNotification();
@@ -37,12 +43,31 @@ class RelaunchNotificationControllerPlatformImpl
  protected:
   // views::WidgetObserver:
   void OnWidgetClosing(views::Widget* widget) override;
+  void OnWidgetDestroying(views::Widget* widget) override;
+
+  // BrowserListObserver:
+  void OnBrowserSetLastActive(Browser* browser) override;
 
  private:
+  // Shows the notification in |browser| for a relaunch that will take place
+  // at |deadline|.
+  void ShowRequiredNotification(Browser* browser, base::Time deadline);
+
   // The widget hosting the bubble or dialog, or nullptr if neither is is
   // currently shown.
   views::Widget* widget_ = nullptr;
 
+  // A callback run when the relaunch required notification first becomes
+  // visible to the user. This callback is valid only while the instance is
+  // waiting for a tabbed browser to become active.
+  base::OnceCallback<base::Time()> on_visible_;
+
+  // A boolean to record if the relaunch notification has been shown or not.
+  bool has_shown_ = false;
+
+  // The last relaunch deadline if the relaunch notification has_shown_.
+  base::Time last_relaunch_deadline_;
+
   DISALLOW_COPY_AND_ASSIGN(RelaunchNotificationControllerPlatformImpl);
 };