Only show paused dialog for active apps that reached daily limit

We were showing paused dialog for every app that was stopped when
time limit was reached. This was resulting in bad user experience when
Chrome was reaching the limit, because all installed web apps are paused
then and user was seeing multiple dialogs (one per every web app).
Now we show the dialog only for active app. When web app is active
there will be one dialog displayed. When Chrome is active there will be
no dialog, but the interstitial page. This should not change anything
for ARC++ apps.

Bug: 1052040
Change-Id: If4b4827ffb6dd7d022170c6503bf8e109c4bfb94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063219
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Yilkal Abe <yilkal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742745}

chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.cc

@@ -477,16 +477,25 @@ void AppActivityRegistry::Add(const AppId& app_id) {
 
 void AppActivityRegistry::SetAppState(const AppId& app_id, AppState app_state) {
   DCHECK(base::Contains(activity_registry_, app_id));
-  AppActivity& app_activity = activity_registry_.at(app_id).activity;
+  AppDetails& app_details = activity_registry_.at(app_id);
+  AppActivity& app_activity = app_details.activity;
   AppState previous_state = app_activity.app_state();
   app_activity.SetAppState(app_state);
 
   if (app_activity.app_state() == AppState::kLimitReached) {
+    bool was_active = false;
+    if (app_activity.is_active()) {
+      was_active = true;
+      app_details.active_windows.clear();
+      app_activity.SetAppInactive(base::Time::Now());
+    }
+
     for (auto& observer : app_state_observers_) {
       const base::Optional<AppLimit>& limit =
           activity_registry_.at(app_id).limit;
       DCHECK(limit->daily_limit());
-      observer.OnAppLimitReached(app_id, limit->daily_limit().value());
+      observer.OnAppLimitReached(app_id, limit->daily_limit().value(),
+                                 was_active);
     }
     return;
   }
@@ -654,9 +663,6 @@ void AppActivityRegistry::CheckTimeLimitForApp(const AppId& app_id) {
     if (ContributesToWebTimeLimit(app_id, GetAppState(app_id))) {
       WebTimeLimitReached(base::Time::Now());
     } else {
-      // Set app activity state as time limit reached.
-      details.active_windows.clear();
-      details.activity.SetAppInactive(base::Time::Now());
       SetAppState(app_id, AppState::kLimitReached);
     }
 
@@ -696,10 +702,6 @@ void AppActivityRegistry::WebTimeLimitReached(base::Time timestamp) {
     if (details.activity.app_state() == AppState::kLimitReached)
       return;
 
-    if (details.activity.is_active()) {
-      details.active_windows.clear();
-      details.activity.SetAppInactive(timestamp);
-    }
     SetAppState(app_id, AppState::kLimitReached);
   }
 }

chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.h

@@ -62,8 +62,11 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
     ~AppStateObserver() override = default;
 
     // Called when state of the app with |app_id| changed to |kLimitReached|.
+    // |was_active| indicates whether the app was active before reaching the
+    // limit.
     virtual void OnAppLimitReached(const AppId& app_id,
-                                   base::TimeDelta time_limit) = 0;
+                                   base::TimeDelta time_limit,
+                                   bool was_active) = 0;
 
     // Called when state of the app with |app_id| is no longer |kLimitReached|.
     virtual void OnAppLimitRemoved(const AppId& app_id) = 0;

chrome/browser/chromeos/child_accounts/time_limits/app_service_wrapper.cc

@@ -71,9 +71,10 @@ AppServiceWrapper::AppServiceWrapper(Profile* profile) : profile_(profile) {
 AppServiceWrapper::~AppServiceWrapper() = default;
 
 void AppServiceWrapper::PauseApp(const AppId& app_id,
-                                 base::TimeDelta daily_limit) {
+                                 base::TimeDelta daily_limit,
+                                 bool show_dialog) {
   apps::PauseData details;
-  details.should_show_pause_dialog = true;
+  details.should_show_pause_dialog = show_dialog;
   details.hours = daily_limit.InHours();
   details.minutes =
       (daily_limit - base::TimeDelta::FromHours(details.hours)).InMinutes();

chrome/browser/chromeos/child_accounts/time_limits/app_service_wrapper.h

@@ -85,8 +85,11 @@ class AppServiceWrapper : public apps::AppRegistryCache::Observer,
   // Pauses the app identified by |app_id|.
   // Uses |daily_limit| to communicate applied time restriction to the user by
   // showing the dialog. After this is called user will not be able to launch
-  // the app and the visual effect will be applied to the icon.
-  void PauseApp(const AppId& app_id, base::TimeDelta daily_limit);
+  // the app and the visual effect will be applied to the icon. |show_dialog|
+  // indicates whether the user should be notified with a dialog.
+  void PauseApp(const AppId& app_id,
+                base::TimeDelta daily_limit,
+                bool show_dialog);
 
   // Resets time restriction from the app identified with |app_id|. After this
   // is called user will be able to use the app again and the visual effect

chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.cc

@@ -365,8 +365,9 @@ void AppTimeController::ShowAppTimeLimitNotification(
 }
 
 void AppTimeController::OnAppLimitReached(const AppId& app_id,
-                                          base::TimeDelta time_limit) {
-  app_service_wrapper_->PauseApp(app_id, time_limit);
+                                          base::TimeDelta time_limit,
+                                          bool was_active) {
+  app_service_wrapper_->PauseApp(app_id, time_limit, was_active);
 }
 
 void AppTimeController::OnAppLimitRemoved(const AppId& app_id) {

chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.h

@@ -81,7 +81,8 @@ class AppTimeController : public SystemClockClient::Observer,
 
   // AppActivityRegistry::AppStateObserver:
   void OnAppLimitReached(const AppId& app_id,
-                         base::TimeDelta time_limit) override;
+                         base::TimeDelta time_limit,
+                         bool was_active) override;
   void OnAppLimitRemoved(const AppId& app_id) override;
   void OnAppInstalled(const AppId& app_id) override;
 

chrome/browser/chromeos/child_accounts/time_limits/app_types.cc

@@ -173,9 +173,12 @@ void AppActivity::SetAppInactive(base::Time timestamp) {
   base::TimeTicks now = base::TimeTicks::Now();
   base::TimeDelta active_time = now - last_updated_time_ticks_;
   base::Time start_time = timestamp - active_time;
-
   is_active_ = false;
-  active_times_.push_back(ActiveTime(start_time, timestamp));
+  // Timestamps can be equal if SetAppInactive() is called directly after
+  // SetAppState(). Happens in tests.
+  DCHECK_GE(timestamp, start_time);
+  if (timestamp > start_time)
+    active_times_.push_back(ActiveTime(start_time, timestamp));
 
   running_active_time_ += active_time;
   last_updated_time_ticks_ = now;