Commit 2724a28e authored by Julie Jeongeun Kim's avatar Julie Jeongeun Kim Committed by Commit Bot

Convert apps::mojom::Subscriber to new Mojo types

This CL converts SubscriberPtr and SubscriberRequest
to new Mojo types using PendingRemote or Remote,
RemoteSet, PendingReceiver, and ReceiverSet.

It also updates RegisterSubscriber, Connect, and
Clone from app_service.mojom.

Bug: 955171
Change-Id: I3de5fe264d54ba707966f5cecde4c9d0778c6911
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821498Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarOksana Zhuravlova <oksamyt@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#699696}
parent b3404ec3
......@@ -17,7 +17,6 @@
#include "chrome/services/app_service/public/mojom/types.mojom.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/url_data_source.h"
#include "mojo/public/cpp/bindings/interface_request.h"
namespace apps {
......@@ -106,8 +105,8 @@ void AppServiceProxy::Initialize(Profile* profile) {
if (app_service_.is_connected()) {
// The AppServiceProxy is a subscriber: something that wants to be able to
// list all known apps.
apps::mojom::SubscriberPtr subscriber;
bindings_.AddBinding(this, mojo::MakeRequest(&subscriber));
mojo::PendingRemote<apps::mojom::Subscriber> subscriber;
receivers_.Add(this, subscriber.InitWithNewPipeAndPassReceiver());
app_service_->RegisterSubscriber(std::move(subscriber), nullptr);
#if defined(OS_CHROMEOS)
......@@ -206,7 +205,7 @@ void AppServiceProxy::FlushMojoCallsForTesting() {
extension_apps_->FlushMojoCallsForTesting();
extension_web_apps_->FlushMojoCallsForTesting();
#endif
bindings_.FlushForTesting();
receivers_.FlushForTesting();
}
apps::IconLoader* AppServiceProxy::OverrideInnerIconLoaderForTesting(
......@@ -244,8 +243,9 @@ void AppServiceProxy::OnApps(std::vector<apps::mojom::AppPtr> deltas) {
cache_.OnApps(std::move(deltas));
}
void AppServiceProxy::Clone(apps::mojom::SubscriberRequest request) {
bindings_.AddBinding(this, std::move(request));
void AppServiceProxy::Clone(
mojo::PendingReceiver<apps::mojom::Subscriber> receiver) {
receivers_.Add(this, std::move(receiver));
}
} // namespace apps
......@@ -13,7 +13,8 @@
#include "chrome/services/app_service/public/cpp/icon_cache.h"
#include "chrome/services/app_service/public/cpp/icon_coalescer.h"
#include "components/keyed_service/core/keyed_service.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote.h"
#if defined(OS_CHROMEOS)
......@@ -144,7 +145,7 @@ class AppServiceProxy : public KeyedService,
// apps::mojom::Subscriber overrides.
void OnApps(std::vector<apps::mojom::AppPtr> deltas) override;
void Clone(apps::mojom::SubscriberRequest request) override;
void Clone(mojo::PendingReceiver<apps::mojom::Subscriber> receiver) override;
// This proxy privately owns its instance of the App Service. This should not
// be exposed except through the Mojo interface connected to |app_service_|.
......@@ -153,7 +154,7 @@ class AppServiceProxy : public KeyedService,
mojo::Remote<apps::mojom::AppService> app_service_;
apps::AppRegistryCache cache_;
mojo::BindingSet<apps::mojom::Subscriber> bindings_;
mojo::ReceiverSet<apps::mojom::Subscriber> receivers_;
// The LoadIconFromIconKey implementation sends a chained series of requests
// through each icon loader, starting from the outer and working back to the
......
......@@ -164,8 +164,9 @@ void ArcApps::Shutdown() {
arc_icon_once_loader_.StopObserving(prefs);
}
void ArcApps::Connect(apps::mojom::SubscriberPtr subscriber,
apps::mojom::ConnectOptionsPtr opts) {
void ArcApps::Connect(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) {
std::vector<apps::mojom::AppPtr> apps;
ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_);
if (prefs) {
......@@ -177,8 +178,10 @@ void ArcApps::Connect(apps::mojom::SubscriberPtr subscriber,
}
}
}
mojo::Remote<apps::mojom::Subscriber> subscriber(
std::move(subscriber_remote));
subscriber->OnApps(std::move(apps));
subscribers_.AddPtr(std::move(subscriber));
subscribers_.Add(std::move(subscriber));
}
void ArcApps::LoadIcon(const std::string& app_id,
......@@ -472,11 +475,11 @@ apps::mojom::AppPtr ArcApps::Convert(ArcAppListPrefs* prefs,
}
void ArcApps::Publish(apps::mojom::AppPtr app) {
subscribers_.ForAllPtrs([&app](apps::mojom::Subscriber* subscriber) {
for (auto& subscriber : subscribers_) {
std::vector<apps::mojom::AppPtr> apps;
apps.push_back(app.Clone());
subscriber->OnApps(std::move(apps));
});
}
}
void ArcApps::ConvertAndPublishPackageApps(
......
......@@ -17,8 +17,9 @@
#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
#include "chrome/services/app_service/public/mojom/app_service.mojom.h"
#include "components/keyed_service/core/keyed_service.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote_set.h"
class Profile;
......@@ -49,7 +50,7 @@ class ArcApps : public KeyedService,
void Shutdown() override;
// apps::mojom::Publisher overrides.
void Connect(apps::mojom::SubscriberPtr subscriber,
void Connect(mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override;
void LoadIcon(const std::string& app_id,
apps::mojom::IconKeyPtr icon_key,
......@@ -96,7 +97,7 @@ class ArcApps : public KeyedService,
const arc::mojom::ArcPackageInfo& package_info);
mojo::Receiver<apps::mojom::Publisher> receiver_{this};
mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
Profile* profile_;
ArcIconOnceLoader arc_icon_once_loader_;
......
......@@ -95,8 +95,9 @@ void BuiltInChromeOsApps::Initialize(
apps::mojom::AppType::kBuiltIn);
}
void BuiltInChromeOsApps::Connect(apps::mojom::SubscriberPtr subscriber,
apps::mojom::ConnectOptionsPtr opts) {
void BuiltInChromeOsApps::Connect(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) {
std::vector<apps::mojom::AppPtr> apps;
if (profile_) {
// TODO(crbug.com/826982): move source of truth for built-in apps from
......@@ -123,11 +124,13 @@ void BuiltInChromeOsApps::Connect(apps::mojom::SubscriberPtr subscriber,
}
}
}
mojo::Remote<apps::mojom::Subscriber> subscriber(
std::move(subscriber_remote));
subscriber->OnApps(std::move(apps));
// Unlike other apps::mojom::Publisher implementations, we don't need to
// retain the subscriber (e.g. add it to a
// mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_) after this
// mojo::RemoteSet<apps::mojom::Subscriber> subscribers_) after this
// function returns. The list of built-in Chrome OS apps is fixed for the
// lifetime of the Chrome OS session. There won't be any further updates.
}
......
......@@ -9,6 +9,7 @@
#include "base/macros.h"
#include "chrome/services/app_service/public/mojom/app_service.mojom.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
......@@ -33,7 +34,7 @@ class BuiltInChromeOsApps : public apps::mojom::Publisher {
void Initialize(const mojo::Remote<apps::mojom::AppService>& app_service);
// apps::mojom::Publisher overrides.
void Connect(apps::mojom::SubscriberPtr subscriber,
void Connect(mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override;
void LoadIcon(const std::string& app_id,
apps::mojom::IconKeyPtr icon_key,
......
......@@ -79,8 +79,9 @@ void CrostiniApps::Initialize(
apps::mojom::AppType::kCrostini);
}
void CrostiniApps::Connect(apps::mojom::SubscriberPtr subscriber,
apps::mojom::ConnectOptionsPtr opts) {
void CrostiniApps::Connect(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) {
std::vector<apps::mojom::AppPtr> apps;
for (const auto& pair : registry_->GetRegisteredApps()) {
const std::string& app_id = pair.first;
......@@ -88,8 +89,10 @@ void CrostiniApps::Connect(apps::mojom::SubscriberPtr subscriber,
pair.second;
apps.push_back(Convert(app_id, registration, true));
}
mojo::Remote<apps::mojom::Subscriber> subscriber(
std::move(subscriber_remote));
subscriber->OnApps(std::move(apps));
subscribers_.AddPtr(std::move(subscriber));
subscribers_.Add(std::move(subscriber));
}
void CrostiniApps::LoadIcon(const std::string& app_id,
......@@ -313,11 +316,11 @@ void CrostiniApps::PublishAppID(const std::string& app_id,
}
void CrostiniApps::Publish(apps::mojom::AppPtr app) {
subscribers_.ForAllPtrs([&app](apps::mojom::Subscriber* subscriber) {
for (auto& subscriber : subscribers_) {
std::vector<apps::mojom::AppPtr> apps;
apps.push_back(app.Clone());
subscriber->OnApps(std::move(apps));
});
}
}
} // namespace apps
......@@ -15,9 +15,10 @@
#include "chrome/browser/chromeos/crostini/crostini_registry_service.h"
#include "chrome/services/app_service/public/mojom/app_service.mojom.h"
#include "components/keyed_service/core/keyed_service.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/remote_set.h"
class PrefChangeRegistrar;
class Profile;
......@@ -51,7 +52,7 @@ class CrostiniApps : public KeyedService,
void Initialize(const mojo::Remote<apps::mojom::AppService>& app_service);
// apps::mojom::Publisher overrides.
void Connect(apps::mojom::SubscriberPtr subscriber,
void Connect(mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override;
void LoadIcon(const std::string& app_id,
apps::mojom::IconKeyPtr icon_key,
......@@ -96,7 +97,7 @@ class CrostiniApps : public KeyedService,
void Publish(apps::mojom::AppPtr app);
mojo::Receiver<apps::mojom::Publisher> receiver_{this};
mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
Profile* profile_;
......
......@@ -197,8 +197,9 @@ bool ExtensionApps::Accepts(const extensions::Extension* extension) {
}
}
void ExtensionApps::Connect(apps::mojom::SubscriberPtr subscriber,
apps::mojom::ConnectOptionsPtr opts) {
void ExtensionApps::Connect(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) {
std::vector<apps::mojom::AppPtr> apps;
if (profile_) {
extensions::ExtensionRegistry* registry =
......@@ -214,8 +215,10 @@ void ExtensionApps::Connect(apps::mojom::SubscriberPtr subscriber,
//
// If making changes to which sets are consulted, also change ShouldShow.
}
mojo::Remote<apps::mojom::Subscriber> subscriber(
std::move(subscriber_remote));
subscriber->OnApps(std::move(apps));
subscribers_.AddPtr(std::move(subscriber));
subscribers_.Add(std::move(subscriber));
}
void ExtensionApps::LoadIcon(const std::string& app_id,
......@@ -530,11 +533,11 @@ void ExtensionApps::OnExtensionUninstalled(
}
void ExtensionApps::Publish(apps::mojom::AppPtr app) {
subscribers_.ForAllPtrs([&app](apps::mojom::Subscriber* subscriber) {
for (auto& subscriber : subscribers_) {
std::vector<apps::mojom::AppPtr> apps;
apps.push_back(app.Clone());
subscriber->OnApps(std::move(apps));
});
}
}
// static
......
......@@ -16,9 +16,10 @@
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "extensions/browser/extension_prefs_observer.h"
#include "extensions/browser/extension_registry_observer.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/remote_set.h"
class Profile;
......@@ -58,7 +59,7 @@ class ExtensionApps : public apps::mojom::Publisher,
bool Accepts(const extensions::Extension* extension);
// apps::mojom::Publisher overrides.
void Connect(apps::mojom::SubscriberPtr subscriber,
void Connect(mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override;
void LoadIcon(const std::string& app_id,
apps::mojom::IconKeyPtr icon_key,
......@@ -130,7 +131,7 @@ class ExtensionApps : public apps::mojom::Publisher,
std::vector<apps::mojom::AppPtr>* apps_out);
mojo::Receiver<apps::mojom::Publisher> receiver_{this};
mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
Profile* profile_;
......
......@@ -8,14 +8,13 @@
#include "base/bind.h"
#include "chrome/services/app_service/public/mojom/types.mojom.h"
#include "mojo/public/cpp/bindings/interface_request.h"
namespace {
void Connect(apps::mojom::Publisher* publisher,
apps::mojom::Subscriber* subscriber) {
apps::mojom::SubscriberPtr clone;
subscriber->Clone(mojo::MakeRequest(&clone));
mojo::PendingRemote<apps::mojom::Subscriber> clone;
subscriber->Clone(clone.InitWithNewPipeAndPassReceiver());
// TODO: replace nullptr with a ConnectOptions.
publisher->Connect(std::move(clone), nullptr);
}
......@@ -43,9 +42,9 @@ void AppServiceImpl::RegisterPublisher(
apps::mojom::AppType app_type) {
mojo::Remote<apps::mojom::Publisher> publisher(std::move(publisher_remote));
// Connect the new publisher with every registered subscriber.
subscribers_.ForAllPtrs([&publisher](auto* subscriber) {
::Connect(publisher.get(), subscriber);
});
for (auto& subscriber : subscribers_) {
::Connect(publisher.get(), subscriber.get());
}
// Check that no previous publisher has registered for the same app_type.
CHECK(publishers_.find(app_type) == publishers_.end());
......@@ -58,9 +57,12 @@ void AppServiceImpl::RegisterPublisher(
CHECK(result.second);
}
void AppServiceImpl::RegisterSubscriber(apps::mojom::SubscriberPtr subscriber,
apps::mojom::ConnectOptionsPtr opts) {
void AppServiceImpl::RegisterSubscriber(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) {
// Connect the new subscriber with every registered publisher.
mojo::Remote<apps::mojom::Subscriber> subscriber(
std::move(subscriber_remote));
for (const auto& iter : publishers_) {
::Connect(iter.second.get(), subscriber.get());
}
......@@ -68,7 +70,7 @@ void AppServiceImpl::RegisterSubscriber(apps::mojom::SubscriberPtr subscriber,
// TODO: store the opts somewhere.
// Add the new subscriber to the set.
subscribers_.AddPtr(std::move(subscriber));
subscribers_.Add(std::move(subscriber));
}
void AppServiceImpl::LoadIcon(apps::mojom::AppType app_type,
......
......@@ -9,11 +9,11 @@
#include "base/macros.h"
#include "chrome/services/app_service/public/mojom/app_service.mojom.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/remote_set.h"
namespace apps {
......@@ -33,8 +33,9 @@ class AppServiceImpl : public apps::mojom::AppService {
void RegisterPublisher(
mojo::PendingRemote<apps::mojom::Publisher> publisher_remote,
apps::mojom::AppType app_type) override;
void RegisterSubscriber(apps::mojom::SubscriberPtr subscriber,
apps::mojom::ConnectOptionsPtr opts) override;
void RegisterSubscriber(
mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override;
void LoadIcon(apps::mojom::AppType app_type,
const std::string& app_id,
apps::mojom::IconKeyPtr icon_key,
......@@ -62,7 +63,7 @@ class AppServiceImpl : public apps::mojom::AppService {
// be able to find *the* publisher for a given apps::mojom::AppType.
std::map<apps::mojom::AppType, mojo::Remote<apps::mojom::Publisher>>
publishers_;
mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
// Must come after the publisher and subscriber maps to ensure it is
// destroyed first, closing the connection to avoid dangling callbacks.
......
......@@ -13,8 +13,10 @@
#include "base/test/task_environment.h"
#include "chrome/services/app_service/app_service_impl.h"
#include "chrome/services/app_service/public/mojom/types.mojom.h"
#include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/remote_set.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -32,9 +34,9 @@ class FakePublisher : public apps::mojom::Publisher {
}
void PublishMoreApps(std::vector<std::string> app_ids) {
subscribers_.ForAllPtrs([this, &app_ids](auto* subscriber) {
CallOnApps(subscriber, app_ids);
});
for (auto& subscriber : subscribers_) {
CallOnApps(subscriber.get(), app_ids);
}
for (const auto& app_id : app_ids) {
known_app_ids_.push_back(app_id);
}
......@@ -43,10 +45,12 @@ class FakePublisher : public apps::mojom::Publisher {
std::string load_icon_app_id;
private:
void Connect(apps::mojom::SubscriberPtr subscriber,
void Connect(mojo::PendingRemote<apps::mojom::Subscriber> subscriber_remote,
apps::mojom::ConnectOptionsPtr opts) override {
mojo::Remote<apps::mojom::Subscriber> subscriber(
std::move(subscriber_remote));
CallOnApps(subscriber.get(), known_app_ids_);
subscribers_.AddPtr(std::move(subscriber));
subscribers_.Add(std::move(subscriber));
}
void LoadIcon(const std::string& app_id,
......@@ -86,15 +90,15 @@ class FakePublisher : public apps::mojom::Publisher {
apps::mojom::AppType app_type_;
std::vector<std::string> known_app_ids_;
mojo::ReceiverSet<apps::mojom::Publisher> receivers_;
mojo::InterfacePtrSet<apps::mojom::Subscriber> subscribers_;
mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
};
class FakeSubscriber : public apps::mojom::Subscriber {
public:
explicit FakeSubscriber(AppServiceImpl* impl) {
apps::mojom::SubscriberPtr ptr;
bindings_.AddBinding(this, mojo::MakeRequest(&ptr));
impl->RegisterSubscriber(std::move(ptr), nullptr);
mojo::PendingRemote<apps::mojom::Subscriber> remote;
receivers_.Add(this, remote.InitWithNewPipeAndPassReceiver());
impl->RegisterSubscriber(std::move(remote), nullptr);
}
std::string AppIdsSeen() {
......@@ -112,11 +116,11 @@ class FakeSubscriber : public apps::mojom::Subscriber {
}
}
void Clone(apps::mojom::SubscriberRequest request) override {
bindings_.AddBinding(this, std::move(request));
void Clone(mojo::PendingReceiver<apps::mojom::Subscriber> receiver) override {
receivers_.Add(this, std::move(receiver));
}
mojo::BindingSet<apps::mojom::Subscriber> bindings_;
mojo::ReceiverSet<apps::mojom::Subscriber> receivers_;
std::set<std::string> app_ids_seen_;
};
......
......@@ -14,9 +14,13 @@ import "chrome/services/app_service/public/mojom/types.mojom";
//
// See chrome/services/app_service/README.md.
interface AppService {
// App Registry methods.
// Called by a publisher of apps to register itself and its apps with the App
// Service.
RegisterPublisher(pending_remote<Publisher> publisher, AppType app_type);
RegisterSubscriber(Subscriber subscriber, ConnectOptions? opts);
// Called by a consumer that wishes to know about available apps to register
// itself with the App Service.
RegisterSubscriber(pending_remote<Subscriber> subscriber, ConnectOptions? opts);
// App Icon Factory methods.
LoadIcon(
......@@ -51,7 +55,7 @@ interface AppService {
interface Publisher {
// App Registry methods.
Connect(Subscriber subscriber, ConnectOptions? opts);
Connect(pending_remote<Subscriber> subscriber, ConnectOptions? opts);
// App Icon Factory methods.
LoadIcon(
......@@ -82,11 +86,11 @@ interface Publisher {
interface Subscriber {
OnApps(array<App> deltas);
// Binds this to the given request (message pipe endpoint), being to Mojo
// Binds this to the given receiver (message pipe endpoint), being to Mojo
// interfaces what POSIX's dup is to file descriptors.
//
// See https://groups.google.com/a/chromium.org/d/msg/chromium-mojo/nFhBzGsb5Pg/V7t_8kNRAgAJ
Clone(Subscriber& request);
Clone(pending_receiver<Subscriber> receiver);
};
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