Commit 99842054 authored by khmel's avatar khmel Committed by Commit bot

arc: Fix removing shortcuts on package update.

TEST=unit tests extented. Manually, updated test app via adb and
     its shorcuts were left in app launcher
BUG=718625
BUG=b/34749664

Review-Url: https://codereview.chromium.org/2860243002
Cr-Commit-Position: refs/heads/master@{#469529}
parent bf2f59c9
...@@ -1118,6 +1118,13 @@ void ArcAppListPrefs::OnUninstallShortcut(const std::string& package_name, ...@@ -1118,6 +1118,13 @@ void ArcAppListPrefs::OnUninstallShortcut(const std::string& package_name,
std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage( std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage(
const std::string& package_name) const { const std::string& package_name) const {
return GetAppsAndShortcutsForPackage(package_name,
false /* include_shortcuts */);
}
std::unordered_set<std::string> ArcAppListPrefs::GetAppsAndShortcutsForPackage(
const std::string& package_name,
bool include_shortcuts) const {
std::unordered_set<std::string> app_set; std::unordered_set<std::string> app_set;
const base::DictionaryValue* apps = prefs_->GetDictionary(prefs::kArcApps); const base::DictionaryValue* apps = prefs_->GetDictionary(prefs::kArcApps);
for (base::DictionaryValue::Iterator app_it(*apps); !app_it.IsAtEnd(); for (base::DictionaryValue::Iterator app_it(*apps); !app_it.IsAtEnd();
...@@ -1138,6 +1145,12 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage( ...@@ -1138,6 +1145,12 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage(
if (package_name != app_package) if (package_name != app_package)
continue; continue;
if (!include_shortcuts) {
bool shortcut = false;
if (app->GetBoolean(kShortcut, &shortcut) && shortcut)
continue;
}
app_set.insert(app_it.key()); app_set.insert(app_it.key());
} }
...@@ -1146,7 +1159,7 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage( ...@@ -1146,7 +1159,7 @@ std::unordered_set<std::string> ArcAppListPrefs::GetAppsForPackage(
void ArcAppListPrefs::HandlePackageRemoved(const std::string& package_name) { void ArcAppListPrefs::HandlePackageRemoved(const std::string& package_name) {
const std::unordered_set<std::string> apps_to_remove = const std::unordered_set<std::string> apps_to_remove =
GetAppsForPackage(package_name); GetAppsAndShortcutsForPackage(package_name, true /* include_shortcuts */);
for (const auto& app_id : apps_to_remove) for (const auto& app_id : apps_to_remove)
RemoveApp(app_id); RemoveApp(app_id);
......
...@@ -239,6 +239,8 @@ class ArcAppListPrefs ...@@ -239,6 +239,8 @@ class ArcAppListPrefs
return package_list_initial_refreshed_; return package_list_initial_refreshed_;
} }
// Returns set of ARC apps for provided package name, not including shortcuts,
// associated with this package.
std::unordered_set<std::string> GetAppsForPackage( std::unordered_set<std::string> GetAppsForPackage(
const std::string& package_name) const; const std::string& package_name) const;
...@@ -329,6 +331,10 @@ class ArcAppListPrefs ...@@ -329,6 +331,10 @@ class ArcAppListPrefs
void DisableAllApps(); void DisableAllApps();
void RemoveAllApps(); void RemoveAllApps();
std::vector<std::string> GetAppIdsNoArcEnabledCheck() const; std::vector<std::string> GetAppIdsNoArcEnabledCheck() const;
std::unordered_set<std::string> GetAppsAndShortcutsForPackage(
const std::string& package_name,
bool include_shortcuts) const;
// Enumerates apps from preferences and notifies listeners about available // Enumerates apps from preferences and notifies listeners about available
// apps while ARC is not started yet. All apps in this case have disabled // apps while ARC is not started yet. All apps in this case have disabled
// state. // state.
......
...@@ -383,10 +383,12 @@ class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase, ...@@ -383,10 +383,12 @@ class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase,
void AddPackage(const arc::mojom::ArcPackageInfo& package) { void AddPackage(const arc::mojom::ArcPackageInfo& package) {
arc_test_.AddPackage(package); arc_test_.AddPackage(package);
app_instance()->SendPackageAdded(package);
} }
void RemovePackage(const arc::mojom::ArcPackageInfo& package) { void RemovePackage(const arc::mojom::ArcPackageInfo& package) {
arc_test_.RemovePackage(package); arc_test_.RemovePackage(package);
app_instance()->SendPackageUninstalled(package.package_name);
} }
AppListControllerDelegate* controller() { return controller_.get(); } AppListControllerDelegate* controller() { return controller_.get(); }
...@@ -557,11 +559,9 @@ TEST_P(ArcAppModelBuilderTest, ArcPackagePref) { ...@@ -557,11 +559,9 @@ TEST_P(ArcAppModelBuilderTest, ArcPackagePref) {
package.sync = true; package.sync = true;
RemovePackage(package); RemovePackage(package);
app_instance()->SendPackageUninstalled(package.package_name);
ValidateHavePackages(fake_packages()); ValidateHavePackages(fake_packages());
AddPackage(package); AddPackage(package);
app_instance()->SendPackageAdded(package);
ValidateHavePackages(fake_packages()); ValidateHavePackages(fake_packages());
} }
...@@ -586,13 +586,12 @@ TEST_P(ArcAppModelBuilderTest, InstallUninstallShortcut) { ...@@ -586,13 +586,12 @@ TEST_P(ArcAppModelBuilderTest, InstallUninstallShortcut) {
std::vector<arc::mojom::ShortcutInfo> shortcuts = fake_shortcuts(); std::vector<arc::mojom::ShortcutInfo> shortcuts = fake_shortcuts();
ASSERT_GE(shortcuts.size(), 2U); ASSERT_GE(shortcuts.size(), 2U);
// Adding package is requred to safely call SendPackageUninstalled. // Adding package is required to safely call SendPackageUninstalled.
arc::mojom::ArcPackageInfo package; arc::mojom::ArcPackageInfo package;
package.package_name = shortcuts[1].package_name; package.package_name = shortcuts[1].package_name;
package.package_version = 1; package.package_version = 1;
package.sync = true; package.sync = true;
AddPackage(package); AddPackage(package);
app_instance()->SendPackageAdded(package);
app_instance()->SendInstallShortcuts(shortcuts); app_instance()->SendInstallShortcuts(shortcuts);
ValidateHaveShortcuts(shortcuts); ValidateHaveShortcuts(shortcuts);
...@@ -1364,7 +1363,7 @@ TEST_P(ArcAppModelBuilderTest, NonLaunchableApp) { ...@@ -1364,7 +1363,7 @@ TEST_P(ArcAppModelBuilderTest, NonLaunchableApp) {
EXPECT_TRUE(prefs->IsRegistered(app_id)); EXPECT_TRUE(prefs->IsRegistered(app_id));
} }
TEST_P(ArcAppModelBuilderTest, ArcAppsOnPackageUpdated) { TEST_P(ArcAppModelBuilderTest, ArcAppsAndShortcutsOnPackageChange) {
ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get()); ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_.get());
ASSERT_NE(nullptr, prefs); ASSERT_NE(nullptr, prefs);
...@@ -1372,13 +1371,27 @@ TEST_P(ArcAppModelBuilderTest, ArcAppsOnPackageUpdated) { ...@@ -1372,13 +1371,27 @@ TEST_P(ArcAppModelBuilderTest, ArcAppsOnPackageUpdated) {
ASSERT_GE(3u, apps.size()); ASSERT_GE(3u, apps.size());
apps[0].package_name = apps[2].package_name; apps[0].package_name = apps[2].package_name;
apps[1].package_name = apps[2].package_name; apps[1].package_name = apps[2].package_name;
std::vector<arc::mojom::ShortcutInfo> shortcuts = fake_shortcuts();
for (auto& shortcut : shortcuts)
shortcut.package_name = apps[0].package_name;
// Second app should be preserved after update. // Second app should be preserved after update.
std::vector<arc::mojom::AppInfo> apps1(apps.begin(), apps.begin() + 2); std::vector<arc::mojom::AppInfo> apps1(apps.begin(), apps.begin() + 2);
std::vector<arc::mojom::AppInfo> apps2(apps.begin() + 1, apps.begin() + 3); std::vector<arc::mojom::AppInfo> apps2(apps.begin() + 1, apps.begin() + 3);
// Adding package is required to safely call SendPackageUninstalled.
arc::mojom::ArcPackageInfo package;
package.package_name = apps[0].package_name;
package.package_version = 1;
package.sync = true;
AddPackage(package);
app_instance()->RefreshAppList(); app_instance()->RefreshAppList();
app_instance()->SendRefreshAppList(apps1); app_instance()->SendRefreshAppList(apps1);
ValidateHaveApps(apps1); app_instance()->SendInstallShortcuts(shortcuts);
ValidateHaveAppsAndShortcuts(apps1, shortcuts);
const std::string app_id = ArcAppTest::GetAppId(apps[1]); const std::string app_id = ArcAppTest::GetAppId(apps[1]);
const base::Time now_time = base::Time::Now(); const base::Time now_time = base::Time::Now();
...@@ -1389,12 +1402,16 @@ TEST_P(ArcAppModelBuilderTest, ArcAppsOnPackageUpdated) { ...@@ -1389,12 +1402,16 @@ TEST_P(ArcAppModelBuilderTest, ArcAppsOnPackageUpdated) {
EXPECT_EQ(now_time, app_info_before->last_launch_time); EXPECT_EQ(now_time, app_info_before->last_launch_time);
app_instance()->SendPackageAppListRefreshed(apps[0].package_name, apps2); app_instance()->SendPackageAppListRefreshed(apps[0].package_name, apps2);
ValidateHaveApps(apps2); ValidateHaveAppsAndShortcuts(apps2, shortcuts);
std::unique_ptr<ArcAppListPrefs::AppInfo> app_info_after = std::unique_ptr<ArcAppListPrefs::AppInfo> app_info_after =
prefs->GetApp(app_id); prefs->GetApp(app_id);
ASSERT_TRUE(app_info_after); ASSERT_TRUE(app_info_after);
EXPECT_EQ(now_time, app_info_after->last_launch_time); EXPECT_EQ(now_time, app_info_after->last_launch_time);
RemovePackage(package);
ValidateHaveAppsAndShortcuts(std::vector<arc::mojom::AppInfo>(),
std::vector<arc::mojom::ShortcutInfo>());
} }
TEST_P(ArcDefaulAppTest, DefaultApps) { TEST_P(ArcDefaulAppTest, DefaultApps) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment