Ignore OnAppAvailable calls if AppState is limit reached.

This CL ensures that restored AppStates are not overwritten
when OnAppAvailable calls are called.

Bug: 1056475, 1019837
Change-Id: I8f8d14300a1e9d63a7e1cfcb2de21beda6ea7123
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2085844
Commit-Queue: Yilkal Abe <yilkal@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747409}

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

@@ -190,8 +190,13 @@ void AppActivityRegistry::OnAppUninstalled(const AppId& app_id) {
 }
 
 void AppActivityRegistry::OnAppAvailable(const AppId& app_id) {
-  if (base::Contains(activity_registry_, app_id))
-    SetAppState(app_id, AppState::kAvailable);
+  if (!base::Contains(activity_registry_, app_id))
+    return;
+
+  if (GetAppState(app_id) == AppState::kLimitReached)
+    return;
+
+  SetAppState(app_id, AppState::kAvailable);
 }
 
 void AppActivityRegistry::OnAppBlocked(const AppId& app_id) {
@@ -205,6 +210,18 @@ void AppActivityRegistry::OnAppActive(const AppId& app_id,
   if (!base::Contains(activity_registry_, app_id))
     return;
 
+  // We are notified that a paused app is active. Notify observers to pause it.
+  if (GetAppState(app_id) == AppState::kLimitReached) {
+    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(),
+                                 /* was_active */ true);
+    }
+    return;
+  }
+
   if (app_id == GetChromeAppId())
     return;
 
@@ -473,6 +490,11 @@ void AppActivityRegistry::OnChromeAppActivityChanged(
 
   bool is_active = (state == ChromeAppActivityState::kActive);
 
+  // No need to notify observers that limit has reached. They will be notified
+  // in AppActivityRegistry::OnAppActive.
+  if (GetAppState(chrome_app_id) == AppState::kLimitReached && is_active)
+    return;
+
   // No change in state.
   if (was_active == is_active)
     return;
@@ -622,7 +644,7 @@ void AppActivityRegistry::SetAppState(const AppId& app_id, AppState app_state) {
     if (app_activity.is_active()) {
       was_active = true;
       app_details.active_windows.clear();
-      app_activity.SetAppInactive(base::Time::Now());
+      SetAppInactive(app_id, base::Time::Now());
     }
 
     for (auto& observer : app_state_observers_) {

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

@@ -52,6 +52,19 @@ class AppTimeNotificationDelegateMock : public AppTimeNotificationDelegate {
                     chromeos::app_time::AppNotification));
 };
 
+class AppStateObserverMock : public AppActivityRegistry::AppStateObserver {
+ public:
+  AppStateObserverMock() = default;
+  AppStateObserverMock(const AppStateObserverMock&) = delete;
+  AppStateObserverMock& operator=(const AppStateObserverMock&) = delete;
+
+  ~AppStateObserverMock() = default;
+
+  MOCK_METHOD3(OnAppLimitReached, void(const AppId&, base::TimeDelta, bool));
+  MOCK_METHOD1(OnAppLimitRemoved, void(const AppId&));
+  MOCK_METHOD1(OnAppInstalled, void(const AppId&));
+};
+
 }  // namespace
 
 class AppActivityRegistryTest : public ChromeViewsTestBase {
@@ -66,7 +79,10 @@ class AppActivityRegistryTest : public ChromeViewsTestBase {
   // ChromeViewsTestBase:
   void SetUp() override;
 
+  void InstallApps();
+
   aura::Window* CreateWindowForApp(const AppId& app_id);
+  aura::Window* GetWindowForApp(const AppId& app_id);
 
   void SetAppLimit(const AppId& app_id,
                    const base::Optional<AppLimit>& app_limit);
@@ -104,7 +120,10 @@ class AppActivityRegistryTest : public ChromeViewsTestBase {
 void AppActivityRegistryTest::SetUp() {
   ChromeViewsTestBase::SetUp();
   ReInitializeRegistry();
+  InstallApps();
+}
 
+void AppActivityRegistryTest::InstallApps() {
   registry().OnAppInstalled(GetChromeAppId());
   registry().OnAppInstalled(kApp1);
   registry().OnAppInstalled(kApp2);
@@ -117,12 +136,18 @@ aura::Window* AppActivityRegistryTest::CreateWindowForApp(const AppId& app_id) {
   std::unique_ptr<aura::Window> window =
       std::make_unique<aura::Window>(nullptr);
   window->Init(ui::LayerType::LAYER_NOT_DRAWN);
-
-  aura::Window* to_return = window.get();
+  auto* to_return = window.get();
   windows_[app_id].push_back(std::move(window));
   return to_return;
 }
 
+aura::Window* AppActivityRegistryTest::GetWindowForApp(const AppId& app_id) {
+  const std::vector<std::unique_ptr<aura::Window>>& app_windows =
+      windows_.at(app_id);
+  EXPECT_GE(app_windows.size(), 0u);
+  return app_windows[app_windows.size() - 1].get();
+}
+
 void AppActivityRegistryTest::SetAppLimit(
     const AppId& app_id,
     const base::Optional<AppLimit>& app_limit) {
@@ -764,5 +789,60 @@ TEST_F(AppActivityRegistryTest, ActiveWebAppBlocked) {
   EXPECT_EQ(registry().GetAppState(GetChromeAppId()), AppState::kLimitReached);
 }
 
+TEST_F(AppActivityRegistryTest, OverrideLimitReachedState) {
+  AppStateObserverMock state_observer_mock;
+  registry().AddAppStateObserver(&state_observer_mock);
+  const base::TimeDelta limit = base::TimeDelta::FromMinutes(30);
+
+  std::map<AppId, AppLimit> app_limits = {
+      {kApp1, AppLimit(AppRestriction::kTimeLimit, limit, base::Time::Now())},
+      {GetChromeAppId(),
+       AppLimit(AppRestriction::kTimeLimit, limit, base::Time::Now())}};
+
+  registry().UpdateAppLimits(app_limits);
+
+  // Save app activity and reinitialize.
+  EXPECT_CALL(state_observer_mock,
+              OnAppLimitReached(kApp1, base::TimeDelta::FromMinutes(30),
+                                /* was_active */ true))
+      .Times(1);
+  EXPECT_CALL(state_observer_mock,
+              OnAppLimitReached(kApp2, base::TimeDelta::FromMinutes(30),
+                                /* was_active */ true))
+      .Times(1);
+  EXPECT_CALL(
+      state_observer_mock,
+      OnAppLimitReached(GetChromeAppId(), base::TimeDelta::FromMinutes(30),
+                        /* was_active */ false))
+      .Times(1);
+
+  // App limits will be reached.
+  CreateAppActivityForApp(kApp1, 2 * limit);
+  CreateAppActivityForApp(kApp2, 2 * limit);
+
+  // Save app activity and reinitialize.
+  registry().SaveAppActivity();
+  ReInitializeRegistry();
+  registry().AddAppStateObserver(&state_observer_mock);
+  registry().UpdateAppLimits(app_limits);
+  InstallApps();
+
+  EXPECT_EQ(registry().GetAppState(kApp1), AppState::kLimitReached);
+  EXPECT_EQ(registry().GetAppState(kApp2), AppState::kLimitReached);
+  EXPECT_EQ(registry().GetAppState(GetChromeAppId()), AppState::kLimitReached);
+
+  EXPECT_CALL(state_observer_mock,
+              OnAppLimitReached(kApp1, base::TimeDelta::FromMinutes(30),
+                                /* was_active */ true))
+      .Times(1);
+  EXPECT_CALL(state_observer_mock,
+              OnAppLimitReached(kApp2, base::TimeDelta::FromMinutes(30),
+                                /* was_active */ true))
+      .Times(1);
+
+  registry().OnAppActive(kApp1, GetWindowForApp(kApp1), base::Time::Now());
+  registry().OnAppActive(kApp2, GetWindowForApp(kApp2), base::Time::Now());
+}
+
 }  // namespace app_time
 }  // namespace chromeos