Make AppActivityRegistry listen to app AppInstance update

AppInstance updates will be used to determine app usage time.

Bug: 1015658
Test: AppServiceWrapperTest
Change-Id: Idcb0232ecf80f02ac3fc3f1a3c2b7cb468db5152
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1958894
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723424}

chrome/browser/chromeos/BUILD.gn

@@ -85,6 +85,7 @@ source_set("chromeos") {
     "//chrome/common/string_matching",
     "//chrome/services/app_service:lib",
     "//chrome/services/app_service/public/cpp:app_update",
+    "//chrome/services/app_service/public/cpp:instance_update",
     "//chrome/services/file_util/public/cpp",
     "//chrome/services/printing/public/mojom",
     "//chrome/services/wilco_dtc_supportd/public/mojom",

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

@@ -56,6 +56,16 @@ void AppActivityRegistry::OnAppBlocked(const AppId& app_id) {
     SetAppState(app_id, AppState::kBlocked);
 }
 
+void AppActivityRegistry::OnAppActive(const AppId& app_id,
+                                      base::Time timestamp) {
+  NOTIMPLEMENTED_LOG_ONCE();
+}
+
+void AppActivityRegistry::OnAppInactive(const AppId& app_id,
+                                        base::Time timestamp) {
+  NOTIMPLEMENTED_LOG_ONCE();
+}
+
 void AppActivityRegistry::Add(const AppId& app_id) {
   auto result = activity_registry_.emplace(
       app_id, AppDetails(AppActivity(AppState::kAvailable)));

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

@@ -29,6 +29,8 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
   void OnAppUninstalled(const AppId& app_id) override;
   void OnAppAvailable(const AppId& app_id) override;
   void OnAppBlocked(const AppId& app_id) override;
+  void OnAppActive(const AppId& app_id, base::Time timestamp) override;
+  void OnAppInactive(const AppId& app_id, base::Time timestamp) override;
 
  private:
   // Bundles detailed data stored for a specific app.

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

@@ -6,12 +6,14 @@
 
 #include <string>
 
+#include "base/optional.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
 #include "chrome/services/app_service/public/cpp/app_update.h"
+#include "chrome/services/app_service/public/cpp/instance_update.h"
 #include "chrome/services/app_service/public/mojom/types.mojom.h"
 
 namespace chromeos {
@@ -19,10 +21,11 @@ namespace app_time {
 
 namespace {
 
-// Return whether |app| should be included for per-app time limits.
+// Return whether app with |app_id| should be included for per-app time
+// limits.
 // TODO(agawronska): Add support for PWA and Chrome.
-bool ShouldIncludeApp(const apps::AppUpdate& app) {
-  return app.AppType() == apps::mojom::AppType::kArc;
+bool ShouldIncludeApp(const AppId& app_id) {
+  return app_id.app_type() == apps::mojom::AppType::kArc;
 }
 
 // Gets AppId from |update|.
@@ -32,10 +35,29 @@ AppId AppIdFromAppUpdate(const apps::AppUpdate& update) {
                is_arc ? update.PublisherId() : update.AppId());
 }
 
+// Gets AppId from |update|.
+AppId AppIdFromInstanceUpdate(const apps::InstanceUpdate& update,
+                              apps::AppRegistryCache* app_cache) {
+  base::Optional<AppId> app_id;
+  app_cache->ForOneApp(update.AppId(),
+                       [&app_id](const apps::AppUpdate& update) {
+                         app_id = AppIdFromAppUpdate(update);
+                       });
+  return app_id.value();
+}
+
+// Gets app service id from |app_id|.
+std::string AppServiceIdFromAppId(const AppId& app_id, Profile* profile) {
+  return app_id.app_type() == apps::mojom::AppType::kArc
+             ? arc::ArcPackageNameToAppId(app_id.app_id(), profile)
+             : app_id.app_id();
+}
+
 }  // namespace
 
 AppServiceWrapper::AppServiceWrapper(Profile* profile) : profile_(profile) {
-  Observe(&GetAppCache());
+  apps::AppRegistryCache::Observer::Observe(&GetAppCache());
+  apps::InstanceRegistry::Observer::Observe(&GetInstanceRegistry());
 }
 
 AppServiceWrapper::~AppServiceWrapper() = default;
@@ -46,14 +68,27 @@ std::vector<AppId> AppServiceWrapper::GetInstalledApps() const {
     if (update.Readiness() == apps::mojom::Readiness::kUninstalledByUser)
       return;
 
-    if (!ShouldIncludeApp(update))
+    const AppId app_id = AppIdFromAppUpdate(update);
+    if (!ShouldIncludeApp(app_id))
       return;
 
-    installed_apps.push_back(AppIdFromAppUpdate(update));
+    installed_apps.push_back(app_id);
   });
   return installed_apps;
 }
 
+std::string AppServiceWrapper::GetAppName(const AppId& app_id) const {
+  const std::string app_service_id = AppServiceIdFromAppId(app_id, profile_);
+  DCHECK(!app_service_id.empty());
+
+  std::string app_name;
+  GetAppCache().ForOneApp(app_service_id,
+                          [&app_name](const apps::AppUpdate& update) {
+                            app_name = update.ShortName();
+                          });
+  return app_name;
+}
+
 void AppServiceWrapper::AddObserver(EventListener* listener) {
   DCHECK(listener);
   listeners_.AddObserver(listener);
@@ -68,10 +103,10 @@ void AppServiceWrapper::OnAppUpdate(const apps::AppUpdate& update) {
   if (!update.ReadinessChanged())
     return;
 
-  if (!ShouldIncludeApp(update))
+  const AppId app_id = AppIdFromAppUpdate(update);
+  if (!ShouldIncludeApp(app_id))
     return;
 
-  const AppId app_id = AppIdFromAppUpdate(update);
   switch (update.Readiness()) {
     case apps::mojom::Readiness::kReady:
       for (auto& listener : listeners_)
@@ -101,7 +136,30 @@ void AppServiceWrapper::OnAppUpdate(const apps::AppUpdate& update) {
 
 void AppServiceWrapper::OnAppRegistryCacheWillBeDestroyed(
     apps::AppRegistryCache* cache) {
-  Observe(nullptr);
+  apps::AppRegistryCache::Observer::Observe(nullptr);
+}
+
+void AppServiceWrapper::OnInstanceUpdate(const apps::InstanceUpdate& update) {
+  if (!update.StateChanged())
+    return;
+
+  const AppId app_id = AppIdFromInstanceUpdate(update, &GetAppCache());
+  if (!ShouldIncludeApp(app_id))
+    return;
+
+  bool is_active = update.State() & apps::InstanceState::kActive;
+  for (auto& listener : listeners_) {
+    if (is_active) {
+      listener.OnAppActive(app_id, update.LastUpdatedTime());
+    } else {
+      listener.OnAppInactive(app_id, update.LastUpdatedTime());
+    }
+  }
+}
+
+void AppServiceWrapper::OnInstanceRegistryWillBeDestroyed(
+    apps::InstanceRegistry* cache) {
+  apps::InstanceRegistry::Observer::Observe(nullptr);
 }
 
 apps::AppRegistryCache& AppServiceWrapper::GetAppCache() const {
@@ -111,5 +169,12 @@ apps::AppRegistryCache& AppServiceWrapper::GetAppCache() const {
   return proxy->AppRegistryCache();
 }
 
+apps::InstanceRegistry& AppServiceWrapper::GetInstanceRegistry() const {
+  apps::AppServiceProxy* proxy =
+      apps::AppServiceProxyFactory::GetForProfile(profile_);
+  DCHECK(proxy);
+  return proxy->InstanceRegistry();
+}
+
 }  // namespace app_time
 }  // namespace chromeos

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

@@ -9,12 +9,15 @@
 
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
+#include "base/time/time.h"
 #include "chrome/services/app_service/public/cpp/app_registry_cache.h"
+#include "chrome/services/app_service/public/cpp/instance_registry.h"
 
 class Profile;
 
 namespace apps {
 class AppUpdate;
+class InstanceUpdate;
 }  // namespace apps
 
 namespace chromeos {
@@ -26,16 +29,39 @@ class AppId;
 // Provides abstraction layer for Per-App Time Limits (PATL). Takes care of
 // types conversions and data filetering, so those operations are not spread
 // around the PATL code.
-class AppServiceWrapper : public apps::AppRegistryCache::Observer {
+class AppServiceWrapper : public apps::AppRegistryCache::Observer,
+                          public apps::InstanceRegistry::Observer {
  public:
   // Notifies listeners about app state changes.
   // Listener only get updates about apps that are relevant for PATL feature.
   class EventListener : public base::CheckedObserver {
    public:
+    // Called when app with |app_id| is installed and at the beginning of each
+    // user session (because AppService does not store apps information between
+    // sessions).
     virtual void OnAppInstalled(const AppId& app_id) {}
+
+    // Called when app with |app_id| is uninstalled.
     virtual void OnAppUninstalled(const AppId& app_id) {}
+
+    // Called when app with |app_id| become available for usage. Usually when
+    // app is unblocked.
     virtual void OnAppAvailable(const AppId& app_id) {}
+
+    // Called when app with |app_id| become disabled and cannot be used.
     virtual void OnAppBlocked(const AppId& app_id) {}
+
+    // Called when app with |app_id| becomes active.
+    // Active means that the app is in usage (visible in foreground).
+    // |timestamp| indicates the time when the app became active.
+    virtual void OnAppActive(const AppId& app_id, base::Time timestamp) {}
+
+    // Called when app with |app_id| becomes inactive.
+    // Inactive means that the app is not in the foreground. It still can run
+    // and be partially visible. |timestamp| indicates the time when the app
+    // became inactive. Note: This can be called for the app that is already
+    // inactive.
+    virtual void OnAppInactive(const AppId& app_id, base::Time timestamp) {}
   };
 
   explicit AppServiceWrapper(Profile* profile);
@@ -47,6 +73,10 @@ class AppServiceWrapper : public apps::AppRegistryCache::Observer {
   // Installed apps of unsupported types will not be included.
   std::vector<AppId> GetInstalledApps() const;
 
+  // Returns short name of the app identified by |app_id|.
+  // Might return empty string.
+  std::string GetAppName(const AppId& app_id) const;
+
   void AddObserver(EventListener* observer);
   void RemoveObserver(EventListener* observer);
 
@@ -55,8 +85,14 @@ class AppServiceWrapper : public apps::AppRegistryCache::Observer {
   void OnAppRegistryCacheWillBeDestroyed(
       apps::AppRegistryCache* cache) override;
 
+  // apps::InstanceRegistry::Observer:
+  void OnInstanceUpdate(const apps::InstanceUpdate& update) override;
+  void OnInstanceRegistryWillBeDestroyed(
+      apps::InstanceRegistry* cache) override;
+
  private:
   apps::AppRegistryCache& GetAppCache() const;
+  apps::InstanceRegistry& GetInstanceRegistry() const;
 
   base::ObserverList<EventListener> listeners_;
 

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

@@ -163,6 +163,19 @@ TEST_F(AppServiceWrapperTest, GetInstalledApps) {
   }
 }
 
+TEST_F(AppServiceWrapperTest, GetAppName) {
+  const AppId app1(apps::mojom::AppType::kArc, kArcPackage1);
+  EXPECT_CALL(test_listener(), OnAppInstalled(app1)).Times(1);
+  SimulateAppInstalled(app1, kArcApp1);
+
+  const AppId app2(apps::mojom::AppType::kArc, kArcPackage2);
+  EXPECT_CALL(test_listener(), OnAppInstalled(app2)).Times(1);
+  SimulateAppInstalled(app2, kArcApp2);
+
+  EXPECT_EQ(kArcApp1, tested_wrapper().GetAppName(app1));
+  EXPECT_EQ(kArcApp2, tested_wrapper().GetAppName(app2));
+}
+
 // Tests installs and uninstalls of Arc apps.
 TEST_F(AppServiceWrapperTest, ArcAppInstallation) {
   // No app installed.
@@ -209,5 +222,8 @@ TEST_F(AppServiceWrapperTest, ArcAppDisabled) {
   SimulateAppDisabled(app, kArcApp1, false);
 }
 
+// TODO(agawronska): Add tests for ARC apps activity once crrev.com/c/1906614 is
+// landed.
+
 }  // namespace app_time
 }  // namespace chromeos