arc: Don't update install time for apps, installed by PAI or policy.

This to let App Launcher correctly rank app and prevent coming these
apps to the recent list.

TEST=Manually + unit_test
BUG=757639

Change-Id: Ifff93cc7089531d2450d2584baedd8a6002ec353
Reviewed-on: https://chromium-review.googlesource.com/1138748
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Yury Khmel <khmel@google.com>
Cr-Commit-Position: refs/heads/master@{#575730}
diff --git a/chrome/browser/chromeos/arc/policy/arc_policy_util.cc b/chrome/browser/chromeos/arc/policy/arc_policy_util.cc
index 057f056..3dc378f 100644
--- a/chrome/browser/chromeos/arc/policy/arc_policy_util.cc
+++ b/chrome/browser/chromeos/arc/policy/arc_policy_util.cc
@@ -5,6 +5,8 @@
 #include "chrome/browser/chromeos/arc/policy/arc_policy_util.h"
 
 #include "base/command_line.h"
+#include "base/json/json_reader.h"
+#include "base/values.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -13,6 +15,17 @@
 namespace arc {
 namespace policy_util {
 
+namespace {
+
+// Constants used to parse ARC++ JSON policy.
+constexpr char kApplicationsKey[] = "applications";
+constexpr char kInstallTypeKey[] = "installType";
+constexpr char kPackageNameKey[] = "packageName";
+constexpr char kInstallTypeRequired[] = "REQUIRED";
+constexpr char kInstallTypeForceInstalled[] = "FORCE_INSTALLED";
+
+}  // namespace
+
 bool IsAccountManaged(const Profile* profile) {
   return policy::ProfilePolicyConnectorFactory::IsProfileManaged(profile);
 }
@@ -31,5 +44,37 @@
                                : EcryptfsMigrationAction::kDisallowMigration;
 }
 
+std::set<std::string> GetRequestedPackagesFromArcPolicy(
+    const std::string& arc_policy) {
+  std::unique_ptr<base::Value> dict = base::JSONReader::Read(arc_policy);
+  if (!dict || !dict->is_dict())
+    return {};
+
+  const base::Value* const packages =
+      dict->FindKeyOfType(kApplicationsKey, base::Value::Type::LIST);
+  if (!packages)
+    return {};
+
+  std::set<std::string> requested_packages;
+  for (const auto& package : packages->GetList()) {
+    if (!package.is_dict())
+      continue;
+    const base::Value* const install_type =
+        package.FindKeyOfType(kInstallTypeKey, base::Value::Type::STRING);
+    if (!install_type)
+      continue;
+    if (install_type->GetString() != kInstallTypeRequired &&
+        install_type->GetString() != kInstallTypeForceInstalled) {
+      continue;
+    }
+    const base::Value* const package_name =
+        package.FindKeyOfType(kPackageNameKey, base::Value::Type::STRING);
+    if (!package_name || package_name->GetString().empty())
+      continue;
+    requested_packages.insert(package_name->GetString());
+  }
+  return requested_packages;
+}
+
 }  // namespace policy_util
 }  // namespace arc
diff --git a/chrome/browser/chromeos/arc/policy/arc_policy_util.h b/chrome/browser/chromeos/arc/policy/arc_policy_util.h
index 8b57ebb..fc243c9 100644
--- a/chrome/browser/chromeos/arc/policy/arc_policy_util.h
+++ b/chrome/browser/chromeos/arc/policy/arc_policy_util.h
@@ -8,6 +8,9 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <set>
+#include <string>
+
 class Profile;
 
 namespace arc {
@@ -49,6 +52,12 @@
 EcryptfsMigrationAction GetDefaultEcryptfsMigrationActionForManagedUser(
     bool active_directory_user);
 
+// Returns set of packages requested to install from |arc_policy|. |arc_policy|
+// has JSON blob format, see
+// https://www.chromium.org/administrators/policy-list-3#ArcPolicy
+std::set<std::string> GetRequestedPackagesFromArcPolicy(
+    const std::string& arc_policy);
+
 }  // namespace policy_util
 }  // namespace arc
 
diff --git a/chrome/browser/chromeos/policy/app_install_event_logger.cc b/chrome/browser/chromeos/policy/app_install_event_logger.cc
index 0278f9d..14744c5 100644
--- a/chrome/browser/chromeos/policy/app_install_event_logger.cc
+++ b/chrome/browser/chromeos/policy/app_install_event_logger.cc
@@ -11,7 +11,6 @@
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
-#include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/sys_info.h"
 #include "base/task_scheduler/post_task.h"
@@ -19,6 +18,7 @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/chromeos/arc/policy/arc_policy_util.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -39,43 +39,6 @@
 
 constexpr int kNonComplianceReasonAppNotInstalled = 5;
 
-std::set<std::string> GetRequestedPackagesFromArcPolicy(
-    const std::string& arc_policy) {
-  std::unique_ptr<base::Value> dict = base::JSONReader::Read(arc_policy);
-  if (!dict || !dict->is_dict()) {
-    return {};
-  }
-
-  const base::Value* const packages =
-      dict->FindKeyOfType("applications", base::Value::Type::LIST);
-  if (!packages) {
-    return {};
-  }
-
-  std::set<std::string> requested_packages;
-  for (const auto& package : packages->GetList()) {
-    if (!package.is_dict()) {
-      continue;
-    }
-    const base::Value* const install_type =
-        package.FindKeyOfType("installType", base::Value::Type::STRING);
-    if (!install_type) {
-      continue;
-    }
-    if (install_type->GetString() != "REQUIRED" &&
-        install_type->GetString() != "FORCE_INSTALLED") {
-      continue;
-    }
-    const base::Value* const package_name =
-        package.FindKeyOfType("packageName", base::Value::Type::STRING);
-    if (!package_name || package_name->GetString().empty()) {
-      continue;
-    }
-    requested_packages.insert(package_name->GetString());
-  }
-  return requested_packages;
-}
-
 std::set<std::string> GetRequestedPackagesFromPolicy(
     const policy::PolicyMap& policy) {
   const base::Value* const arc_enabled = policy.GetValue(key::kArcEnabled);
@@ -88,7 +51,8 @@
     return {};
   }
 
-  return GetRequestedPackagesFromArcPolicy(arc_policy->GetString());
+  return arc::policy_util::GetRequestedPackagesFromArcPolicy(
+      arc_policy->GetString());
 }
 
 // Return all elements that are members of |first| but not |second|.
@@ -218,7 +182,8 @@
 }
 
 void AppInstallEventLogger::OnPolicySent(const std::string& policy) {
-  requested_in_arc_ = GetRequestedPackagesFromArcPolicy(policy);
+  requested_in_arc_ =
+      arc::policy_util::GetRequestedPackagesFromArcPolicy(policy);
 }
 
 void AppInstallEventLogger::OnComplianceReportReceived(
diff --git a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
index e208f3c..cf5ca73 100644
--- a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
+++ b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
@@ -299,6 +299,12 @@
   const std::vector<std::string> existing_app_ids = GetAppIds();
   tracked_apps_.insert(existing_app_ids.begin(), existing_app_ids.end());
   // Once default apps are ready OnDefaultAppsReady is called.
+
+  // Not always set in unit_tests
+  arc::ArcPolicyBridge* policy_bridge =
+      arc::ArcPolicyBridge::GetForBrowserContext(profile_);
+  if (policy_bridge)
+    policy_bridge->AddObserver(this);
 }
 
 ArcAppListPrefs::~ArcAppListPrefs() {
@@ -776,6 +782,19 @@
   StartPrefs();
 }
 
+void ArcAppListPrefs::OnPolicySent(const std::string& policy) {
+  // Update set of packages installed by policy.
+  packages_by_policy_ =
+      arc::policy_util::GetRequestedPackagesFromArcPolicy(policy);
+}
+
+void ArcAppListPrefs::Shutdown() {
+  arc::ArcPolicyBridge* policy_bridge =
+      arc::ArcPolicyBridge::GetForBrowserContext(profile_);
+  if (policy_bridge)
+    policy_bridge->RemoveObserver(this);
+}
+
 void ArcAppListPrefs::RegisterDefaultApps() {
   // Report default apps first, note, app_map includes uninstalled and filtered
   // out apps as well.
@@ -935,7 +954,7 @@
 
   // Note the install time is the first time the Chrome OS sees the app, not the
   // actual install time in Android side.
-  if (GetInstallTime(app_id).is_null()) {
+  if (GetInstallTime(app_id).is_null() && NeedSetInstallTime(package_name)) {
     std::string install_time_str =
         base::Int64ToString(base::Time::Now().ToInternalValue());
     app_dict->SetString(kInstallTime, install_time_str);
@@ -1185,6 +1204,24 @@
     InvalidateAppIcons(app_id);
 }
 
+bool ArcAppListPrefs::NeedSetInstallTime(
+    const std::string& package_name) const {
+  // If checked package is in active default list that means it is installed by
+  // PAI and install time should not be recorded. Once package is not in active
+  // default list then this package was removed from default and user installs
+  // it manually. In last case we have to record install time.
+  if (default_apps_.GetActivePackages().count(package_name))
+    return false;
+
+  // Check if package is installed by policy. In this case don't set install
+  // time.
+  if (packages_by_policy_.count(package_name))
+    return false;
+
+  // TODO(b/34248841) - Handle apps, installed by sync.
+  return true;
+}
+
 void ArcAppListPrefs::OnPackageAppListRefreshed(
     const std::string& package_name,
     std::vector<arc::mojom::AppInfoPtr> apps) {
diff --git a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
index 199bafe..d478927 100644
--- a/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
+++ b/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
@@ -22,6 +22,7 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
+#include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
 #include "chrome/browser/ui/app_list/arc/arc_default_app_list.h"
 #include "components/arc/common/app.mojom.h"
 #include "components/arc/connection_observer.h"
@@ -54,7 +55,8 @@
                         public arc::mojom::AppHost,
                         public arc::ConnectionObserver<arc::mojom::AppInstance>,
                         public arc::ArcSessionManager::Observer,
-                        public ArcDefaultAppList::Delegate {
+                        public ArcDefaultAppList::Delegate,
+                        public arc::ArcPolicyBridge::Observer {
  public:
   struct AppInfo {
     AppInfo(const std::string& name,
@@ -254,6 +256,12 @@
   // ArcDefaultAppList::Delegate:
   void OnDefaultAppsReady() override;
 
+  // arc::ArcPolicyBridge::Observer:
+  void OnPolicySent(const std::string& policy) override;
+
+  // KeyedService:
+  void Shutdown() override;
+
   // Removes app with the given app_id.
   void RemoveApp(const std::string& app_id);
 
@@ -432,6 +440,12 @@
   // Marks package icons as invalidated and request icons updated.
   void InvalidatePackageIcons(const std::string& package_name);
 
+  // Returns true if install time has to be set to current time for the newly
+  // installed app from the |package_name|. App launcher uses install time to
+  // rank apps. Do not set install time for apps, installed by default or by
+  // policy.
+  bool NeedSetInstallTime(const std::string& package_name) const;
+
   Profile* const profile_;
 
   // Owned by the BrowserContext.
@@ -483,6 +497,8 @@
   bool default_apps_ready_ = false;
   ArcDefaultAppList default_apps_;
   base::Closure default_apps_ready_callback_;
+  // Set of packages installed by policy in case of managed user.
+  std::set<std::string> packages_by_policy_;
 
   // TODO (b/70566216): Remove this once fixed.
   base::OnceClosure app_list_refreshed_callback_;
diff --git a/chrome/browser/ui/app_list/arc/arc_app_unittest.cc b/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
index b794658..85ef506 100644
--- a/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
+++ b/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
@@ -1209,6 +1209,49 @@
   EXPECT_GE(app_info->last_launch_time, time_before);
 }
 
+// Makes sure that install time is set.
+TEST_P(ArcAppModelBuilderTest, InstallTime) {
+  ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
+  ASSERT_TRUE(prefs);
+
+  ASSERT_TRUE(fake_apps().size());
+
+  const std::string app_id = ArcAppTest::GetAppId(fake_apps()[0]);
+  EXPECT_FALSE(prefs->GetApp(app_id));
+
+  app_instance()->RefreshAppList();
+  app_instance()->SendRefreshAppList(fake_apps());
+
+  std::unique_ptr<ArcAppListPrefs::AppInfo> app_info = prefs->GetApp(app_id);
+  ASSERT_TRUE(app_info);
+  EXPECT_NE(base::Time(), app_info->install_time);
+  EXPECT_LE(app_info->install_time, base::Time::Now());
+}
+
+// Makes sure that install time is not set when installed by policy.
+TEST_P(ArcAppModelBuilderTest, InstallTimeForPolicyApps) {
+  ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
+  ASSERT_TRUE(prefs);
+
+  ASSERT_TRUE(fake_apps().size());
+
+  const std::string app_id = ArcAppTest::GetAppId(fake_apps()[0]);
+  EXPECT_FALSE(prefs->GetApp(app_id));
+
+  const std::string policy = base::StringPrintf(
+      "{\"applications\":[{\"installType\":\"FORCE_INSTALLED\",\"packageName\":"
+      "\"%s\"}]}",
+      fake_apps()[0].package_name.c_str());
+  prefs->OnPolicySent(policy);
+
+  app_instance()->RefreshAppList();
+  app_instance()->SendRefreshAppList(fake_apps());
+
+  std::unique_ptr<ArcAppListPrefs::AppInfo> app_info = prefs->GetApp(app_id);
+  ASSERT_TRUE(app_info);
+  EXPECT_EQ(base::Time(), app_info->install_time);
+}
+
 // Validate that arc model contains expected elements on restart.
 TEST_P(ArcAppModelBuilderRecreate, AppModelRestart) {
   // No apps on initial start.
@@ -2002,6 +2045,12 @@
     package_apps.push_back(default_app);
     app_instance()->SendPackageAppListRefreshed(default_app.package_name,
                                                 package_apps);
+
+    // Install time is not set for installed default apps.
+    std::unique_ptr<ArcAppListPrefs::AppInfo> app_info =
+        prefs->GetApp(ArcAppTest::GetAppId(default_app));
+    ASSERT_TRUE(app_info);
+    EXPECT_EQ(base::Time(), app_info->install_time);
   }
 
   // And now default apps are ready.
diff --git a/chrome/browser/ui/app_list/arc/arc_default_app_list.h b/chrome/browser/ui/app_list/arc/arc_default_app_list.h
index 39f992e..fd34180 100644
--- a/chrome/browser/ui/app_list/arc/arc_default_app_list.h
+++ b/chrome/browser/ui/app_list/arc/arc_default_app_list.h
@@ -9,6 +9,7 @@
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <unordered_set>