Commit 55b2140d authored by Maggie Cai's avatar Maggie Cai Committed by Commit Bot

[IntentHandling] Sync Preferred Apps in App Service.

This CL adds the preferred apps storage in the App Service Server,
and implement methods to sync the change between App Service Server
and the Subscribers.

BUG=853604

Change-Id: Iba78cd0d02d95464de038c7d97f267d75bdcec9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1866228
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarNancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708526}
parent 7a4406b3
...@@ -153,10 +153,6 @@ void AppServiceProxy::Initialize() { ...@@ -153,10 +153,6 @@ void AppServiceProxy::Initialize() {
weak_ptr_factory_.GetWeakPtr(), profile_)); weak_ptr_factory_.GetWeakPtr(), profile_));
#endif // OS_CHROMEOS #endif // OS_CHROMEOS
} }
// TODO(crbug.com/853604): Initialise from App Service stored preference.
preferred_apps_cache_.Init(
std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
} }
mojo::Remote<apps::mojom::AppService>& AppServiceProxy::AppService() { mojo::Remote<apps::mojom::AppService>& AppServiceProxy::AppService() {
...@@ -167,8 +163,8 @@ apps::AppRegistryCache& AppServiceProxy::AppRegistryCache() { ...@@ -167,8 +163,8 @@ apps::AppRegistryCache& AppServiceProxy::AppRegistryCache() {
return cache_; return cache_;
} }
apps::PreferredApps& AppServiceProxy::PreferredAppsCache() { apps::PreferredApps& AppServiceProxy::PreferredApps() {
return preferred_apps_cache_; return preferred_apps_;
} }
apps::mojom::IconKeyPtr AppServiceProxy::GetIconKey(const std::string& app_id) { apps::mojom::IconKeyPtr AppServiceProxy::GetIconKey(const std::string& app_id) {
...@@ -344,8 +340,16 @@ void AppServiceProxy::AddPreferredApp(const std::string& app_id, ...@@ -344,8 +340,16 @@ void AppServiceProxy::AddPreferredApp(const std::string& app_id,
void AppServiceProxy::AddPreferredApp(const std::string& app_id, void AppServiceProxy::AddPreferredApp(const std::string& app_id,
const apps::mojom::IntentPtr& intent) { const apps::mojom::IntentPtr& intent) {
auto intent_filter = FindBestMatchingFilter(intent); auto intent_filter = FindBestMatchingFilter(intent);
if (intent_filter) if (intent_filter) {
preferred_apps_cache_.AddPreferredApp(app_id, intent_filter); preferred_apps_.AddPreferredApp(app_id, intent_filter);
if (app_service_.is_connected()) {
cache_.ForOneApp(
app_id, [this, &intent_filter](const apps::AppUpdate& update) {
app_service_->AddPreferredApp(update.AppType(), update.AppId(),
std::move(intent_filter));
});
}
}
} }
void AppServiceProxy::AddAppIconSource(Profile* profile) { void AppServiceProxy::AddAppIconSource(Profile* profile) {
...@@ -374,4 +378,15 @@ void AppServiceProxy::Clone( ...@@ -374,4 +378,15 @@ void AppServiceProxy::Clone(
receivers_.Add(this, std::move(receiver)); receivers_.Add(this, std::move(receiver));
} }
void AppServiceProxy::OnPreferredAppSet(
const std::string& app_id,
apps::mojom::IntentFilterPtr intent_filter) {
preferred_apps_.AddPreferredApp(app_id, intent_filter);
}
void AppServiceProxy::InitializePreferredApps(base::Value preferred_apps) {
preferred_apps_.Init(
std::make_unique<base::Value>(std::move(preferred_apps)));
}
} // namespace apps } // namespace apps
...@@ -52,7 +52,7 @@ class AppServiceProxy : public KeyedService, ...@@ -52,7 +52,7 @@ class AppServiceProxy : public KeyedService,
mojo::Remote<apps::mojom::AppService>& AppService(); mojo::Remote<apps::mojom::AppService>& AppService();
apps::AppRegistryCache& AppRegistryCache(); apps::AppRegistryCache& AppRegistryCache();
apps::PreferredApps& PreferredAppsCache(); apps::PreferredApps& PreferredApps();
// apps::IconLoader overrides. // apps::IconLoader overrides.
apps::mojom::IconKeyPtr GetIconKey(const std::string& app_id) override; apps::mojom::IconKeyPtr GetIconKey(const std::string& app_id) override;
...@@ -175,6 +175,9 @@ class AppServiceProxy : public KeyedService, ...@@ -175,6 +175,9 @@ class AppServiceProxy : public KeyedService,
// apps::mojom::Subscriber overrides. // apps::mojom::Subscriber overrides.
void OnApps(std::vector<apps::mojom::AppPtr> deltas) override; void OnApps(std::vector<apps::mojom::AppPtr> deltas) override;
void Clone(mojo::PendingReceiver<apps::mojom::Subscriber> receiver) override; void Clone(mojo::PendingReceiver<apps::mojom::Subscriber> receiver) override;
void OnPreferredAppSet(const std::string& app_id,
apps::mojom::IntentFilterPtr intent_filter) override;
void InitializePreferredApps(base::Value preferred_apps) override;
// This proxy privately owns its instance of the App Service. This should not // This proxy privately owns its instance of the App Service. This should not
// be exposed except through the Mojo interface connected to |app_service_|. // be exposed except through the Mojo interface connected to |app_service_|.
...@@ -194,7 +197,7 @@ class AppServiceProxy : public KeyedService, ...@@ -194,7 +197,7 @@ class AppServiceProxy : public KeyedService,
IconCoalescer icon_coalescer_; IconCoalescer icon_coalescer_;
IconCache outer_icon_loader_; IconCache outer_icon_loader_;
PreferredApps preferred_apps_cache_; apps::PreferredApps preferred_apps_;
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
std::unique_ptr<BuiltInChromeOsApps> built_in_chrome_os_apps_; std::unique_ptr<BuiltInChromeOsApps> built_in_chrome_os_apps_;
......
...@@ -113,8 +113,7 @@ CommonAppsNavigationThrottle::FindAllAppsForUrl( ...@@ -113,8 +113,7 @@ CommonAppsNavigationThrottle::FindAllAppsForUrl(
std::vector<std::string> app_ids = proxy->GetAppIdsForUrl(url); std::vector<std::string> app_ids = proxy->GetAppIdsForUrl(url);
auto* menu_manager = auto* menu_manager =
extensions::MenuManager::Get(web_contents->GetBrowserContext()); extensions::MenuManager::Get(web_contents->GetBrowserContext());
auto preferred_app_id = auto preferred_app_id = proxy->PreferredApps().FindPreferredAppForUrl(url);
proxy->PreferredAppsCache().FindPreferredAppForUrl(url);
for (const std::string app_id : app_ids) { for (const std::string app_id : app_ids) {
proxy->AppRegistryCache().ForOneApp( proxy->AppRegistryCache().ForOneApp(
......
...@@ -14,6 +14,7 @@ source_set("lib") { ...@@ -14,6 +14,7 @@ source_set("lib") {
] ]
public_deps = [ public_deps = [
"//chrome/services//app_service/public/cpp:preferred_apps",
"//chrome/services/app_service/public/mojom", "//chrome/services/app_service/public/mojom",
] ]
} }
...@@ -27,6 +28,8 @@ source_set("unit_tests") { ...@@ -27,6 +28,8 @@ source_set("unit_tests") {
deps = [ deps = [
":lib", ":lib",
"//chrome/services//app_service/public/cpp:intents",
"//chrome/services//app_service/public/cpp:preferred_apps",
"//testing/gtest", "//testing/gtest",
] ]
} }
...@@ -23,7 +23,9 @@ void Connect(apps::mojom::Publisher* publisher, ...@@ -23,7 +23,9 @@ void Connect(apps::mojom::Publisher* publisher,
namespace apps { namespace apps {
AppServiceImpl::AppServiceImpl() = default; AppServiceImpl::AppServiceImpl() {
InitializePreferredApps();
}
AppServiceImpl::~AppServiceImpl() = default; AppServiceImpl::~AppServiceImpl() = default;
...@@ -69,6 +71,9 @@ void AppServiceImpl::RegisterSubscriber( ...@@ -69,6 +71,9 @@ void AppServiceImpl::RegisterSubscriber(
// TODO: store the opts somewhere. // TODO: store the opts somewhere.
// Initialise the Preferred Apps in the Subscribers on register.
subscriber->InitializePreferredApps(preferred_apps_.GetValue());
// Add the new subscriber to the set. // Add the new subscriber to the set.
subscribers_.Add(std::move(subscriber)); subscribers_.Add(std::move(subscriber));
} }
...@@ -155,8 +160,28 @@ void AppServiceImpl::OpenNativeSettings(apps::mojom::AppType app_type, ...@@ -155,8 +160,28 @@ void AppServiceImpl::OpenNativeSettings(apps::mojom::AppType app_type,
iter->second->OpenNativeSettings(app_id); iter->second->OpenNativeSettings(app_id);
} }
void AppServiceImpl::AddPreferredApp(
apps::mojom::AppType app_type,
const std::string& app_id,
apps::mojom::IntentFilterPtr intent_filter) {
preferred_apps_.AddPreferredApp(app_id, intent_filter);
for (auto& subscriber : subscribers_) {
subscriber->OnPreferredAppSet(app_id, intent_filter->Clone());
}
// TODO(crbug.com/853604): Update to the corresponding publisher.
}
void AppServiceImpl::OnPublisherDisconnected(apps::mojom::AppType app_type) { void AppServiceImpl::OnPublisherDisconnected(apps::mojom::AppType app_type) {
publishers_.erase(app_type); publishers_.erase(app_type);
} }
PreferredApps& AppServiceImpl::GetPreferredAppsForTesting() {
return preferred_apps_;
}
void AppServiceImpl::InitializePreferredApps() {
// TODO(crbug.com/853604): Initialise from disk.
preferred_apps_.Init(nullptr);
}
} // namespace apps } // namespace apps
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <map> #include <map>
#include "base/macros.h" #include "base/macros.h"
#include "chrome/services/app_service/public/cpp/preferred_apps.h"
#include "chrome/services/app_service/public/mojom/app_service.mojom.h" #include "chrome/services/app_service/public/mojom/app_service.mojom.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h"
...@@ -64,10 +65,19 @@ class AppServiceImpl : public apps::mojom::AppService { ...@@ -64,10 +65,19 @@ class AppServiceImpl : public apps::mojom::AppService {
bool report_abuse) override; bool report_abuse) override;
void OpenNativeSettings(apps::mojom::AppType app_type, void OpenNativeSettings(apps::mojom::AppType app_type,
const std::string& app_id) override; const std::string& app_id) override;
void AddPreferredApp(apps::mojom::AppType app_type,
const std::string& app_id,
apps::mojom::IntentFilterPtr intent_filter) override;
// Retern the preferred_apps_ for testing.
PreferredApps& GetPreferredAppsForTesting();
private: private:
void OnPublisherDisconnected(apps::mojom::AppType app_type); void OnPublisherDisconnected(apps::mojom::AppType app_type);
// Initialize the preferred apps from disk.
void InitializePreferredApps();
// publishers_ is a std::map, not a mojo::RemoteSet, since we want to // publishers_ is a std::map, not a mojo::RemoteSet, since we want to
// be able to find *the* publisher for a given apps::mojom::AppType. // be able to find *the* publisher for a given apps::mojom::AppType.
std::map<apps::mojom::AppType, mojo::Remote<apps::mojom::Publisher>> std::map<apps::mojom::AppType, mojo::Remote<apps::mojom::Publisher>>
...@@ -78,6 +88,8 @@ class AppServiceImpl : public apps::mojom::AppService { ...@@ -78,6 +88,8 @@ class AppServiceImpl : public apps::mojom::AppService {
// destroyed first, closing the connection to avoid dangling callbacks. // destroyed first, closing the connection to avoid dangling callbacks.
mojo::ReceiverSet<apps::mojom::AppService> receivers_; mojo::ReceiverSet<apps::mojom::AppService> receivers_;
PreferredApps preferred_apps_;
DISALLOW_COPY_AND_ASSIGN(AppServiceImpl); DISALLOW_COPY_AND_ASSIGN(AppServiceImpl);
}; };
......
...@@ -9,9 +9,12 @@ ...@@ -9,9 +9,12 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/callback.h" #include "base/callback.h"
#include "base/optional.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "chrome/services/app_service/app_service_impl.h" #include "chrome/services/app_service/app_service_impl.h"
#include "chrome/services/app_service/public/cpp/intent_filter_util.h"
#include "chrome/services/app_service/public/cpp/preferred_apps.h"
#include "chrome/services/app_service/public/mojom/types.mojom.h" #include "chrome/services/app_service/public/mojom/types.mojom.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h"
...@@ -117,6 +120,8 @@ class FakeSubscriber : public apps::mojom::Subscriber { ...@@ -117,6 +120,8 @@ class FakeSubscriber : public apps::mojom::Subscriber {
return ss.str(); return ss.str();
} }
PreferredApps& PreferredApps() { return preferred_apps_; }
private: private:
void OnApps(std::vector<apps::mojom::AppPtr> deltas) override { void OnApps(std::vector<apps::mojom::AppPtr> deltas) override {
for (const auto& delta : deltas) { for (const auto& delta : deltas) {
...@@ -128,8 +133,19 @@ class FakeSubscriber : public apps::mojom::Subscriber { ...@@ -128,8 +133,19 @@ class FakeSubscriber : public apps::mojom::Subscriber {
receivers_.Add(this, std::move(receiver)); receivers_.Add(this, std::move(receiver));
} }
void OnPreferredAppSet(const std::string& app_id,
apps::mojom::IntentFilterPtr intent_filter) override {
preferred_apps_.AddPreferredApp(app_id, intent_filter);
}
void InitializePreferredApps(base::Value preferred_apps) override {
preferred_apps_.Init(
std::make_unique<base::Value>(std::move(preferred_apps)));
}
mojo::ReceiverSet<apps::mojom::Subscriber> receivers_; mojo::ReceiverSet<apps::mojom::Subscriber> receivers_;
std::set<std::string> app_ids_seen_; std::set<std::string> app_ids_seen_;
apps::PreferredApps preferred_apps_;
}; };
class AppServiceImplTest : public testing::Test { class AppServiceImplTest : public testing::Test {
...@@ -229,4 +245,48 @@ TEST_F(AppServiceImplTest, PubSub) { ...@@ -229,4 +245,48 @@ TEST_F(AppServiceImplTest, PubSub) {
} }
} }
TEST_F(AppServiceImplTest, PreferredApps) {
// Test Initialize.
AppServiceImpl impl;
// TODO(crbug.com/853604): Update this test after reading from disk done.
EXPECT_TRUE(impl.GetPreferredAppsForTesting().GetValue().DictEmpty());
const char kAppId1[] = "abcdefg";
GURL filter_url = GURL("https://www.google.com/abc");
auto intent_filter = apps_util::CreateIntentFilterForUrlScope(filter_url);
impl.GetPreferredAppsForTesting().AddPreferredApp(kAppId1, intent_filter);
// Add one subscriber.
FakeSubscriber sub0(&impl);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(sub0.PreferredApps().GetValue(),
impl.GetPreferredAppsForTesting().GetValue());
// Add another subscriber.
FakeSubscriber sub1(&impl);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(sub1.PreferredApps().GetValue(),
impl.GetPreferredAppsForTesting().GetValue());
// Test sync preferred app to all subscribers.
const char kAppId2[] = "aaaaaaa";
filter_url = GURL("https://www.abc.com/");
intent_filter = apps_util::CreateIntentFilterForUrlScope(filter_url);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(base::nullopt,
sub0.PreferredApps().FindPreferredAppForUrl(filter_url));
EXPECT_EQ(base::nullopt,
sub1.PreferredApps().FindPreferredAppForUrl(filter_url));
impl.AddPreferredApp(apps::mojom::AppType::kUnknown, kAppId2,
std::move(intent_filter));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(kAppId2, sub0.PreferredApps().FindPreferredAppForUrl(filter_url));
EXPECT_EQ(kAppId2, sub1.PreferredApps().FindPreferredAppForUrl(filter_url));
}
} // namespace apps } // namespace apps
...@@ -335,4 +335,8 @@ base::Optional<std::string> PreferredApps::FindPreferredAppForUrl( ...@@ -335,4 +335,8 @@ base::Optional<std::string> PreferredApps::FindPreferredAppForUrl(
return FindPreferredAppForIntent(intent); return FindPreferredAppForIntent(intent);
} }
base::Value PreferredApps::GetValue() {
return preferred_apps_->Clone();
}
} // namespace apps } // namespace apps
...@@ -58,6 +58,9 @@ class PreferredApps { ...@@ -58,6 +58,9 @@ class PreferredApps {
// Find preferred app id for an |url|. // Find preferred app id for an |url|.
base::Optional<std::string> FindPreferredAppForUrl(const GURL& url); base::Optional<std::string> FindPreferredAppForUrl(const GURL& url);
// Get a copy of the preferred apps.
base::Value GetValue();
private: private:
std::unique_ptr<base::Value> preferred_apps_; std::unique_ptr<base::Value> preferred_apps_;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
module apps.mojom; module apps.mojom;
import "chrome/services/app_service/public/mojom/types.mojom"; import "chrome/services/app_service/public/mojom/types.mojom";
import "mojo/public/mojom/base/values.mojom";
// An intermediary between M app consumers (e.g. app launcher UI, intent // An intermediary between M app consumers (e.g. app launcher UI, intent
// pickers) and N app providers (also known as app platforms, e.g. Android // pickers) and N app providers (also known as app platforms, e.g. Android
...@@ -73,6 +74,12 @@ interface AppService { ...@@ -73,6 +74,12 @@ interface AppService {
OpenNativeSettings( OpenNativeSettings(
AppType app_type, AppType app_type,
string app_id); string app_id);
// Set app identified by |app_id| as preferred app for |intent_fitler|.
AddPreferredApp(
AppType app_type,
string app_id,
IntentFilter intent_filter);
}; };
interface Publisher { interface Publisher {
...@@ -133,6 +140,14 @@ interface Subscriber { ...@@ -133,6 +140,14 @@ interface Subscriber {
// //
// See https://groups.google.com/a/chromium.org/d/msg/chromium-mojo/nFhBzGsb5Pg/V7t_8kNRAgAJ // See https://groups.google.com/a/chromium.org/d/msg/chromium-mojo/nFhBzGsb5Pg/V7t_8kNRAgAJ
Clone(pending_receiver<Subscriber> receiver); Clone(pending_receiver<Subscriber> receiver);
// Indicates that the app identified by |app_id| has been set as a preferred
// app for |intent_fitler|. This method is used by the App Service to sync
// the change from one subscriber to the others.
OnPreferredAppSet(string app_id,
IntentFilter intent_filter);
InitializePreferredApps(mojo_base.mojom.Value preferred_apps);
}; };
struct ConnectOptions { struct ConnectOptions {
......
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