arc: Don't close runtime apps on package change.

This prevents removing runtime apps on package change event.
This fixes the issue when running Andorod Instant apps are closed
on package update. They appear in ARC as runtime app.

TEST=Manually, unit_test
BUG=b/119671931

Change-Id: Ife214bafb11a7b86c2b8fa86eea9824f1bc6e6d6
Reviewed-on: https://chromium-review.googlesource.com/c/1354686
Commit-Queue: Yury Khmel <khmel@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612268}

chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc

@@ -1325,7 +1325,9 @@ void ArcAppListPrefs::OnPackageAppListRefreshed(
   }
 
   std::unordered_set<std::string> apps_to_remove =
-      GetAppsForPackage(package_name);
+      GetAppsAndShortcutsForPackage(package_name,
+                                    true, /* include_only_launchable_apps */
+                                    false /* include_shortcuts */);
 
   for (const auto& app : apps) {
     const std::string app_id = GetAppId(app->package_name, app->activity);
@@ -1386,11 +1388,13 @@ void ArcAppListPrefs::OnUninstallShortcut(const std::string& package_name,
 std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage(
     const std::string& package_name) const {
   return GetAppsAndShortcutsForPackage(package_name,
+                                       false, /* include_only_launchable_apps */
                                        false /* include_shortcuts */);
 }
 
 std::unordered_set<std::string> ArcAppListPrefs::GetAppsAndShortcutsForPackage(
     const std::string& package_name,
+    bool include_only_launchable_apps,
     bool include_shortcuts) const {
   std::unordered_set<std::string> app_set;
   const base::DictionaryValue* apps =
@@ -1422,6 +1426,13 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsAndShortcutsForPackage(
         continue;
     }
 
+    if (include_only_launchable_apps) {
+      // Filter out non-lauchable apps.
+      bool launchable = false;
+      if (!app->GetBoolean(kLaunchable, &launchable) || !launchable)
+        continue;
+    }
+
     app_set.insert(app_it.key());
   }
 
@@ -1431,7 +1442,9 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsAndShortcutsForPackage(
 void ArcAppListPrefs::HandlePackageRemoved(const std::string& package_name) {
   DCHECK(IsArcAndroidEnabledForProfile(profile_));
   const std::unordered_set<std::string> apps_to_remove =
-      GetAppsAndShortcutsForPackage(package_name, true /* include_shortcuts */);
+      GetAppsAndShortcutsForPackage(package_name,
+                                    false /* include_only_launchable_apps */,
+                                    true /* include_shortcuts */);
   for (const auto& app_id : apps_to_remove)
     RemoveApp(app_id);
 

chrome/browser/ui/app_list/arc/arc_app_list_prefs.h

@@ -394,8 +394,13 @@ class ArcAppListPrefs : public KeyedService,
   void DisableAllApps();
   void RemoveAllAppsAndPackages();
   std::vector<std::string> GetAppIdsNoArcEnabledCheck() const;
+  // Retrieves registered apps and shortcuts for specific package |package_name|
+  // If |include_only_launchable_apps| is set to true then only launchable apps
+  // are included and runtime apps are ignored. Otherwise all apps are returned.
+  // |include_shortcuts| specifies if shorcuts needs to be included.
   std::unordered_set<std::string> GetAppsAndShortcutsForPackage(
       const std::string& package_name,
+      bool include_only_launchable_apps,
       bool include_shortcuts) const;
 
   // Enumerates apps from preferences and notifies listeners about available

chrome/browser/ui/app_list/arc/arc_app_unittest.cc

@@ -186,6 +186,20 @@ int GetAppListIconDimensionForScaleFactor(ui::ScaleFactor scale_factor) {
   }
 }
 
+ArcAppListPrefs::AppInfo GetAppInfoExpectation(const arc::mojom::AppInfo& app,
+                                               bool launchable) {
+  return ArcAppListPrefs::AppInfo(
+      app.name, app.package_name, app.activity, std::string() /* intent_uri */,
+      std::string() /* icon_resource_id */, base::Time() /* last_launch_time */,
+      base::Time() /* install_time */, app.sticky, app.notifications_enabled,
+      true /* ready */, false /* suspended */, launchable /* show_in_launcher*/,
+      false /* shortcut */, launchable);
+}
+
+MATCHER_P(ArcPackageInfoIs, package, "") {
+  return arg.Equals(package);
+}
+
 }  // namespace
 
 class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase,
@@ -1235,12 +1249,8 @@ TEST_P(ArcAppModelBuilderTest, AppLifeCycleEventsOnOptOut) {
   const std::string app_id = ArcAppTest::GetAppId(app);
 
   ArcAppListPrefs::AppInfo::SetIgnoreCompareInstallTimeForTesting(true);
-  const ArcAppListPrefs::AppInfo expected_app_info_registered(
-      app.name, app.package_name, app.activity, std::string() /* intent_uri */,
-      std::string() /* icon_resource_id */, base::Time() /* last_launch_time */,
-      base::Time() /* install_time */, app.sticky, app.notifications_enabled,
-      true /* ready */, false /* suspended */, true /* show_in_launcher*/,
-      false /* shortcut */, true /* launchable */);
+  const ArcAppListPrefs::AppInfo expected_app_info_registered =
+      GetAppInfoExpectation(app, true /* launchable */);
 
   ArcAppListPrefs::AppInfo expected_app_info_disabled(
       expected_app_info_registered);
@@ -2146,6 +2156,76 @@ TEST_P(ArcAppModelBuilderTest, ArcAppsAndShortcutsOnPackageChange) {
                                std::vector<arc::mojom::ShortcutInfo>());
 }
 
+// This validates that runtime apps are not removed on package change event.
+TEST_P(ArcAppModelBuilderTest, DontRemoveRuntimeAppOnPackageChange) {
+  ArcAppListPrefs::AppInfo::SetIgnoreCompareInstallTimeForTesting(true);
+  ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
+  ASSERT_TRUE(prefs);
+
+  arc::MockArcAppListPrefsObserver observer;
+
+  ASSERT_GE(fake_apps().size(), 2U);
+
+  // Second app should be preserved after the package update.
+  std::vector<arc::mojom::AppInfo> apps(fake_apps().begin(),
+                                        fake_apps().begin() + 2);
+  apps[0].package_name = apps[1].package_name;
+
+  const std::string app_id1 = ArcAppTest::GetAppId(apps[0]);
+  const std::string app_id2 = ArcAppTest::GetAppId(apps[1]);
+
+  arc::mojom::ArcPackageInfoPtr package = CreatePackage(apps[0].package_name);
+
+  prefs->AddObserver(&observer);
+
+  EXPECT_CALL(observer, OnPackageInstalled(ArcPackageInfoIs(*package)))
+      .Times(1);
+  EXPECT_CALL(observer,
+              OnAppRegistered(app_id1, GetAppInfoExpectation(
+                                           apps[0], true /* launchable */)))
+      .Times(1);
+  EXPECT_CALL(observer,
+              OnAppRegistered(app_id2, GetAppInfoExpectation(
+                                           apps[1], true /* launchable */)))
+      .Times(1);
+
+  AddPackage(package);
+
+  app_instance()->RefreshAppList();
+  app_instance()->SendRefreshAppList(apps);
+
+  // Send a task for non-existing lauchable app. That would register new runtime
+  // app.
+  arc::mojom::AppInfo app_runtime = apps[0];
+  app_runtime.activity += "_runtime";
+  // Runtime apps have notifications_enabled and sticky false.
+  app_runtime.notifications_enabled = false;
+  app_runtime.sticky = false;
+  const std::string app_id3 = ArcAppTest::GetAppId(app_runtime);
+
+  EXPECT_CALL(observer, OnAppRegistered(
+                            app_id3, GetAppInfoExpectation(
+                                         app_runtime, false /* launchable */)))
+      .Times(1);
+  EXPECT_CALL(observer,
+              OnTaskCreated(1 /* task_id */, app_runtime.package_name,
+                            app_runtime.activity, std::string() /* name */))
+      .Times(1);
+
+  app_instance()->SendTaskCreated(1, app_runtime, std::string());
+
+  // Simulate package update when first launchable app is removed. This should
+  // trigger app removing for it but not for the runtime app.
+  EXPECT_CALL(observer, OnAppRemoved(app_id1)).Times(1);
+  EXPECT_CALL(observer, OnAppRemoved(app_id2)).Times(0);
+  EXPECT_CALL(observer, OnAppRemoved(app_id3)).Times(0);
+
+  apps.erase(apps.begin());
+  app_instance()->SendPackageAppListRefreshed(apps[0].package_name, apps);
+
+  prefs->RemoveObserver(&observer);
+}
+
 TEST_P(ArcDefaulAppTest, DefaultApps) {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
   ASSERT_NE(nullptr, prefs);