Migrate PreferredApps tests to AppServiceProxy tests.
When the flag kAppServicePreferredAppsWithoutMojom is enabled, AppServiceMojomImplTest won't do preferred apps operations, and all functions will be moved to AppServiceProxy, so the tests for preferred apps in AppServiceMojomImplTest will be moved to AppServiceProxyPreferredAppsTest to verify the functions done by AppServiceProxy.
This CL migrates below PreferredApps tests from AppServiceMojomImplTest
to AppServiceProxyPreferredAppsTest:
PreferredApps
PreferredAppsOverlap
PreferredAppsDuplicated
BUG=1253250
Change-Id: I8c56e82fbc07b7dbc9dc4d06ef37ebd56373d64e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3697709
Reviewed-by: Tim Sergeant <tsergeant@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1014358}
diff --git a/chrome/browser/apps/app_service/app_service_proxy_base.cc b/chrome/browser/apps/app_service/app_service_proxy_base.cc
index acb8c19..51a6f08c 100644
--- a/chrome/browser/apps/app_service/app_service_proxy_base.cc
+++ b/chrome/browser/apps/app_service/app_service_proxy_base.cc
@@ -750,7 +750,7 @@
void AppServiceProxyBase::OnApps(std::vector<AppPtr> deltas,
AppType app_type,
bool should_notify_initialized) {
- if (app_service_.is_connected()) {
+ if (preferred_apps_impl_ || app_service_.is_connected()) {
for (const auto& delta : deltas) {
if (delta->readiness != Readiness::kUnknown &&
!apps_util::IsInstalled(delta->readiness)) {
diff --git a/chrome/browser/apps/app_service/app_service_proxy_unittest.cc b/chrome/browser/apps/app_service/app_service_proxy_unittest.cc
index fd1044f..0efed9c 100644
--- a/chrome/browser/apps/app_service/app_service_proxy_unittest.cc
+++ b/chrome/browser/apps/app_service/app_service_proxy_unittest.cc
@@ -24,6 +24,7 @@
#include "components/services/app_service/public/cpp/intent_filter_util.h"
#include "components/services/app_service/public/cpp/intent_test_util.h"
#include "components/services/app_service/public/cpp/intent_util.h"
+#include "components/services/app_service/public/cpp/preferred_app.h"
#include "components/services/app_service/public/cpp/publisher_base.h"
#include "components/services/app_service/public/mojom/types.mojom-shared.h"
#include "components/services/app_service/public/mojom/types.mojom.h"
@@ -49,7 +50,7 @@
app_type_(app_type),
known_app_ids_(std::move(initial_app_ids)) {
RegisterPublisher(app_type_);
- CallOnApps();
+ CallOnApps(known_app_ids_, /*uninstall=*/false);
}
void LaunchAppWithParams(AppLaunchParams&& params,
@@ -62,15 +63,27 @@
bool allow_placeholder_icon,
LoadIconCallback callback) override {}
+ void UninstallApps(std::vector<std::string> app_ids) {
+ CallOnApps(app_ids, /*uninstall=*/true);
+
+ for (const auto& app_id : app_ids) {
+ known_app_ids_.push_back(app_id);
+ }
+ }
+
bool AppHasSupportedLinksPreference(const std::string& app_id) {
return supported_link_apps_.find(app_id) != supported_link_apps_.end();
}
private:
- void CallOnApps() {
+ void CallOnApps(std::vector<std::string>& app_ids, bool uninstall) {
std::vector<AppPtr> apps;
- for (const auto& app_id : known_app_ids_) {
- apps.push_back(std::make_unique<App>(app_type_, app_id));
+ for (const auto& app_id : app_ids) {
+ auto app = std::make_unique<App>(app_type_, app_id);
+ if (uninstall) {
+ app->readiness = Readiness::kUninstalledByUser;
+ }
+ apps.push_back(std::move(app));
}
AppPublisher::Publish(std::move(apps), app_type_,
/*should_notify_initialized=*/true);
@@ -106,6 +119,10 @@
preferred_apps_list_.ApplyBulkUpdate(std::move(changes));
}
+ void InitializePreferredApps(apps::PreferredApps preferred_apps) override {
+ preferred_apps_list_.Init(std::move(preferred_apps));
+ }
+
private:
apps::PreferredAppsList preferred_apps_list_;
};
@@ -562,6 +579,67 @@
proxy()->PreferredAppsList().FindPreferredAppForUrl(kTestUrl3));
}
+TEST_F(AppServiceProxyPreferredAppsTest, PreferredApps) {
+ // Test Initialize.
+ GetPreferredAppsList().Init();
+
+ const char kAppId1[] = "abcdefg";
+ const char kAppId2[] = "aaaaaaa";
+ GURL filter_url = GURL("https://www.google.com/abc");
+ auto intent_filter = apps_util::MakeIntentFilterForUrlScope(filter_url);
+
+ GetPreferredAppsList().AddPreferredApp(kAppId1, intent_filter);
+
+ FakePublisherForProxyTest pub(proxy(), AppType::kArc,
+ std::vector<std::string>{kAppId1, kAppId2});
+
+ // Test sync preferred app to all subscribers.
+ filter_url = GURL("https://www.abc.com/");
+ GURL another_filter_url = GURL("https://www.test.com/");
+ intent_filter = apps_util::MakeIntentFilterForUrlScope(filter_url);
+ auto another_intent_filter =
+ apps_util::MakeIntentFilterForUrlScope(another_filter_url);
+
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(another_filter_url));
+
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kUnknown, kAppId2, intent_filter->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView, filter_url),
+ /*from_publisher=*/true);
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kUnknown, kAppId2, another_intent_filter->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView,
+ another_filter_url),
+ /*from_publisher=*/true);
+ EXPECT_EQ(kAppId2, GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(kAppId2,
+ GetPreferredAppsList().FindPreferredAppForUrl(another_filter_url));
+
+ // Test that uninstall removes all the settings for the app.
+ pub.UninstallApps(std::vector<std::string>{kAppId2});
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(another_filter_url));
+
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kUnknown, kAppId2, intent_filter->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView, filter_url),
+ /*from_publisher=*/true);
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kUnknown, kAppId2, another_intent_filter->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView,
+ another_filter_url),
+ /*from_publisher=*/true);
+
+ EXPECT_EQ(kAppId2, GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(kAppId2,
+ GetPreferredAppsList().FindPreferredAppForUrl(another_filter_url));
+}
+
TEST_F(AppServiceProxyPreferredAppsTest,
PreferredAppsSetSupportedLinksPublisher) {
GetPreferredAppsList().Init();
@@ -632,6 +710,68 @@
EXPECT_FALSE(pub.AppHasSupportedLinksPreference(kAppId3));
}
+// Test that app with overlapped works properly.
+TEST_F(AppServiceProxyPreferredAppsTest, PreferredAppsOverlap) {
+ // Test Initialize.
+ GetPreferredAppsList().Init();
+
+ const char kAppId1[] = "abcdefg";
+ const char kAppId2[] = "hijklmn";
+
+ GURL filter_url_1 = GURL("https://www.google.com/abc");
+ GURL filter_url_2 = GURL("http://www.google.com.au/abc");
+ GURL filter_url_3 = GURL("https://www.abc.com/abc");
+
+ auto intent_filter_1 = apps_util::MakeIntentFilterForUrlScope(filter_url_1);
+ apps_util::AddConditionValue(ConditionType::kScheme, filter_url_2.scheme(),
+ PatternMatchType::kNone, intent_filter_1);
+ apps_util::AddConditionValue(ConditionType::kHost, filter_url_2.host(),
+ PatternMatchType::kNone, intent_filter_1);
+
+ auto intent_filter_2 = apps_util::MakeIntentFilterForUrlScope(filter_url_3);
+ apps_util::AddConditionValue(ConditionType::kScheme, filter_url_2.scheme(),
+ PatternMatchType::kNone, intent_filter_2);
+ apps_util::AddConditionValue(ConditionType::kHost, filter_url_2.host(),
+ PatternMatchType::kNone, intent_filter_2);
+
+ auto intent_filter_3 = apps_util::MakeIntentFilterForUrlScope(filter_url_1);
+
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_1));
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_2));
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_3));
+ EXPECT_EQ(0U, GetPreferredAppsList().GetEntrySize());
+ EXPECT_EQ(0U, GetPreferredAppsList().GetEntrySize());
+
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kArc, kAppId1, intent_filter_1->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView, filter_url_1),
+ /*from_publisher=*/true);
+ EXPECT_EQ(kAppId1,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_1));
+ EXPECT_EQ(kAppId1,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_2));
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_3));
+ EXPECT_EQ(1U, GetPreferredAppsList().GetEntrySize());
+
+ // Add preferred app with intent filter overlap with existing entry for
+ // another app will reset the preferred app setting for the other app.
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kArc, kAppId2, intent_filter_2->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView, filter_url_1),
+ /*from_publisher=*/true);
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_1));
+ EXPECT_EQ(kAppId2,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_2));
+ EXPECT_EQ(kAppId2,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url_3));
+ EXPECT_EQ(1U, GetPreferredAppsList().GetEntrySize());
+}
+
// Test that app with overlapped supported links works properly.
TEST_F(AppServiceProxyPreferredAppsTest, PreferredAppsOverlapSupportedLink) {
// Test Initialize.
@@ -720,6 +860,37 @@
EXPECT_EQ(2U, GetPreferredAppsList().GetEntrySize());
}
+// Test that duplicated entry will not be added.
+TEST_F(AppServiceProxyPreferredAppsTest, PreferredAppsDuplicated) {
+ // Test Initialize.
+ GetPreferredAppsList().Init();
+
+ const char kAppId1[] = "abcdefg";
+
+ GURL filter_url = GURL("https://www.google.com/abc");
+
+ auto intent_filter = apps_util::MakeIntentFilterForUrlScope(filter_url);
+
+ EXPECT_EQ(absl::nullopt,
+ GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(0U, GetPreferredAppsList().GetEntrySize());
+
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kArc, kAppId1, intent_filter->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView, filter_url),
+ /*from_publisher=*/true);
+ EXPECT_EQ(kAppId1, GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(1U, GetPreferredAppsList().GetEntrySize());
+ EXPECT_EQ(1U, GetPreferredAppsList().GetEntrySize());
+
+ proxy()->PreferredAppsImpl()->AddPreferredApp(
+ AppType::kArc, kAppId1, intent_filter->Clone(),
+ std::make_unique<Intent>(apps_util::kIntentActionView, filter_url),
+ /*from_publisher=*/true);
+ EXPECT_EQ(kAppId1, GetPreferredAppsList().FindPreferredAppForUrl(filter_url));
+ EXPECT_EQ(1U, GetPreferredAppsList().GetEntrySize());
+}
+
// Test that duplicated entry will not be added for supported links.
TEST_F(AppServiceProxyPreferredAppsTest, PreferredAppsDuplicatedSupportedLink) {
// Test Initialize.
diff --git a/chrome/browser/apps/app_service/subscriber_crosapi.cc b/chrome/browser/apps/app_service/subscriber_crosapi.cc
index d90dd72..d1d2962 100644
--- a/chrome/browser/apps/app_service/subscriber_crosapi.cc
+++ b/chrome/browser/apps/app_service/subscriber_crosapi.cc
@@ -18,6 +18,7 @@
#include "chrome/browser/ui/webui/settings/ash/app_management/app_management_uma.h"
#include "components/services/app_service/public/cpp/app_registry_cache.h"
#include "components/services/app_service/public/cpp/app_types.h"
+#include "components/services/app_service/public/cpp/features.h"
#include "components/services/app_service/public/mojom/types.mojom.h"
namespace {
@@ -191,8 +192,13 @@
void SubscriberCrosapi::AddPreferredApp(const std::string& app_id,
crosapi::mojom::IntentPtr intent) {
- proxy_->AddPreferredApp(
- app_id, apps_util::ConvertCrosapiToAppServiceIntent(intent, profile_));
+ if (base::FeatureList::IsEnabled(kAppServicePreferredAppsWithoutMojom)) {
+ proxy_->AddPreferredApp(
+ app_id, apps_util::CreateAppServiceIntentFromCrosapi(intent, profile_));
+ } else {
+ proxy_->AddPreferredApp(
+ app_id, apps_util::ConvertCrosapiToAppServiceIntent(intent, profile_));
+ }
}
void SubscriberCrosapi::ShowAppManagementPage(const std::string& app_id) {
diff --git a/chrome/browser/apps/app_service/subscriber_crosapi.h b/chrome/browser/apps/app_service/subscriber_crosapi.h
index c8dfb9f..ffbea681 100644
--- a/chrome/browser/apps/app_service/subscriber_crosapi.h
+++ b/chrome/browser/apps/app_service/subscriber_crosapi.h
@@ -46,7 +46,7 @@
void OnApps(const std::vector<AppPtr>& deltas);
- void InitializePreferredApps(PreferredApps preferred_apps);
+ virtual void InitializePreferredApps(PreferredApps preferred_apps);
virtual void OnPreferredAppsChanged(PreferredAppChangesPtr changes);
protected:
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 0442ad7..a18e842 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -6858,6 +6858,8 @@
"//components/send_tab_to_self:test_support",
"//components/services/app_service:unit_tests",
"//components/services/app_service/public/cpp:icon_loader_test_support",
+ "//components/services/app_service/public/cpp:preferred_app",
+ "//components/services/app_service/public/cpp:preferred_apps",
"//components/services/app_service/public/cpp:publisher",
"//components/services/app_service/public/cpp:test_support",
"//components/services/app_service/public/cpp:unit_tests",
