Commit c402ede1 authored by Kyle Williams's avatar Kyle Williams Committed by Commit Bot

hotline: Remove CfmServiceRegistry interface

Removing the CfmServiceRegistry allows for more succinct APIs and a more
clear design.

* Replace |RequestBindService| with |BindRegistry|
* Fix Tests and function definitions

BUG=b:170679101
TEST='./out/Default/chromeos_unittests --dbus-stub
--single-process-tests --gtest_filter=Cfm* && xvfb-run
./out/Default/unit_tests --single-process-tests
--gtest_filter=Cfm*'

Change-Id: I9e2a0abc0a1ad388db28d096d9e4080d80945b67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2467279Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarJorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Kyle Williams <kdgwill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817300}
parent a03c70a7
...@@ -37,8 +37,8 @@ void CfmBrowserService::OnAdaptorDisconnect() { ...@@ -37,8 +37,8 @@ void CfmBrowserService::OnAdaptorDisconnect() {
receivers_.Clear(); receivers_.Clear();
} }
void CfmBrowserService::BindService( void CfmBrowserService::OnBindService(
::mojo::ScopedMessagePipeHandle receiver_pipe) { mojo::ScopedMessagePipeHandle receiver_pipe) {
receivers_.Add( receivers_.Add(
this, mojo::PendingReceiver<mojom::CfmBrowser>(std::move(receiver_pipe))); this, mojo::PendingReceiver<mojom::CfmBrowser>(std::move(receiver_pipe)));
} }
......
...@@ -39,7 +39,7 @@ class CfmBrowserService : public CfmObserver, ...@@ -39,7 +39,7 @@ class CfmBrowserService : public CfmObserver,
// Forward |ServiceAdaptorDelegate| implementation // Forward |ServiceAdaptorDelegate| implementation
void OnAdaptorConnect(bool success) override; void OnAdaptorConnect(bool success) override;
void OnAdaptorDisconnect() override; void OnAdaptorDisconnect() override;
void BindService(::mojo::ScopedMessagePipeHandle receiver_pipe) override; void OnBindService(mojo::ScopedMessagePipeHandle receiver_pipe) override;
virtual void OnServiceDisconnect(); virtual void OnServiceDisconnect();
......
...@@ -19,12 +19,12 @@ void FakeCfmServiceContext::ProvideAdaptor( ...@@ -19,12 +19,12 @@ void FakeCfmServiceContext::ProvideAdaptor(
std::move(callback)); std::move(callback));
} }
void FakeCfmServiceContext::BindRegistry( void FakeCfmServiceContext::RequestBindService(
const std::string& interface_name, const std::string& interface_name,
mojo::PendingReceiver<mojom::CfmServiceRegistry> broker_receiver, mojo::ScopedMessagePipeHandle receiver_pipe,
BindRegistryCallback callback) { RequestBindServiceCallback callback) {
std::move(bind_registry_callback_) std::move(request_bind_service_callback_)
.Run(std::move(interface_name), std::move(broker_receiver), .Run(std::move(interface_name), std::move(receiver_pipe),
std::move(callback)); std::move(callback));
} }
...@@ -33,9 +33,9 @@ void FakeCfmServiceContext::SetFakeProvideAdaptorCallback( ...@@ -33,9 +33,9 @@ void FakeCfmServiceContext::SetFakeProvideAdaptorCallback(
provide_adaptor_callback_ = std::move(callback); provide_adaptor_callback_ = std::move(callback);
} }
void FakeCfmServiceContext::SetFakeBindRegistryCallback( void FakeCfmServiceContext::SetFakeRequestBindServiceCallback(
FakeBindRegistryCallback callback) { FakeRequestBindServiceCallback callback) {
bind_registry_callback_ = std::move(callback); request_bind_service_callback_ = std::move(callback);
} }
} // namespace cfm } // namespace cfm
......
...@@ -21,10 +21,10 @@ class FakeCfmServiceContext : public mojom::CfmServiceContext { ...@@ -21,10 +21,10 @@ class FakeCfmServiceContext : public mojom::CfmServiceContext {
mojo::PendingRemote<mojom::CfmServiceAdaptor> adaptor_remote, mojo::PendingRemote<mojom::CfmServiceAdaptor> adaptor_remote,
ProvideAdaptorCallback callback)>; ProvideAdaptorCallback callback)>;
using FakeBindRegistryCallback = base::OnceCallback<void( using FakeRequestBindServiceCallback =
const std::string& interface_name, base::OnceCallback<void(const std::string& interface_name,
mojo::PendingReceiver<mojom::CfmServiceRegistry> broker_receiver, mojo::ScopedMessagePipeHandle receiver_pipe,
BindRegistryCallback callback)>; RequestBindServiceCallback callback)>;
FakeCfmServiceContext(); FakeCfmServiceContext();
FakeCfmServiceContext(const FakeCfmServiceContext&) = delete; FakeCfmServiceContext(const FakeCfmServiceContext&) = delete;
...@@ -36,18 +36,18 @@ class FakeCfmServiceContext : public mojom::CfmServiceContext { ...@@ -36,18 +36,18 @@ class FakeCfmServiceContext : public mojom::CfmServiceContext {
mojo::PendingRemote<mojom::CfmServiceAdaptor> adaptor_remote, mojo::PendingRemote<mojom::CfmServiceAdaptor> adaptor_remote,
ProvideAdaptorCallback callback) override; ProvideAdaptorCallback callback) override;
void BindRegistry( void RequestBindService(const std::string& interface_name,
const std::string& interface_name, mojo::ScopedMessagePipeHandle receiver_pipe,
mojo::PendingReceiver<mojom::CfmServiceRegistry> broker_receiver, RequestBindServiceCallback callback) override;
BindRegistryCallback callback) override;
void SetFakeProvideAdaptorCallback(FakeProvideAdaptorCallback callback); void SetFakeProvideAdaptorCallback(FakeProvideAdaptorCallback callback);
void SetFakeBindRegistryCallback(FakeBindRegistryCallback callback); void SetFakeRequestBindServiceCallback(
FakeRequestBindServiceCallback callback);
private: private:
FakeProvideAdaptorCallback provide_adaptor_callback_; FakeProvideAdaptorCallback provide_adaptor_callback_;
FakeBindRegistryCallback bind_registry_callback_; FakeRequestBindServiceCallback request_bind_service_callback_;
}; };
} // namespace cfm } // namespace cfm
......
...@@ -40,8 +40,9 @@ void ServiceAdaptor::BindServiceAdaptor() { ...@@ -40,8 +40,9 @@ void ServiceAdaptor::BindServiceAdaptor() {
&ServiceAdaptor::OnAdaptorDisconnect, base::Unretained(this))); &ServiceAdaptor::OnAdaptorDisconnect, base::Unretained(this)));
} }
void ServiceAdaptor::BindService(mojo::ScopedMessagePipeHandle receiver_pipe) { void ServiceAdaptor::OnBindService(
delegate_->BindService(std::move(receiver_pipe)); mojo::ScopedMessagePipeHandle receiver_pipe) {
delegate_->OnBindService(std::move(receiver_pipe));
} }
void ServiceAdaptor::OnAdaptorConnect(bool success) { void ServiceAdaptor::OnAdaptorConnect(bool success) {
......
...@@ -32,7 +32,7 @@ class ServiceAdaptor : public mojom::CfmServiceAdaptor { ...@@ -32,7 +32,7 @@ class ServiceAdaptor : public mojom::CfmServiceAdaptor {
// Called when attempting to Bind a mojom using using a message pipe of the // Called when attempting to Bind a mojom using using a message pipe of the
// given types PendingReceiver. // given types PendingReceiver.
virtual void BindService(mojo::ScopedMessagePipeHandle receiver_pipe) = 0; virtual void OnBindService(mojo::ScopedMessagePipeHandle receiver_pipe) = 0;
protected: protected:
Delegate() = default; Delegate() = default;
...@@ -51,7 +51,7 @@ class ServiceAdaptor : public mojom::CfmServiceAdaptor { ...@@ -51,7 +51,7 @@ class ServiceAdaptor : public mojom::CfmServiceAdaptor {
protected: protected:
// Forward |mojom::CfmServiceAdaptor| implementation // Forward |mojom::CfmServiceAdaptor| implementation
void BindService(mojo::ScopedMessagePipeHandle receiver_pipe) override; void OnBindService(mojo::ScopedMessagePipeHandle receiver_pipe) override;
// Called when the Service Adaptor has successfully connected to the // Called when the Service Adaptor has successfully connected to the
// |mojom::CfmServiceContext| // |mojom::CfmServiceContext|
......
...@@ -48,7 +48,7 @@ class FakeDelegate : public ServiceAdaptor::Delegate { ...@@ -48,7 +48,7 @@ class FakeDelegate : public ServiceAdaptor::Delegate {
disconnect_callback_.Run(); disconnect_callback_.Run();
} }
void BindService(mojo::ScopedMessagePipeHandle) override { void OnBindService(mojo::ScopedMessagePipeHandle) override {
bind_request_count++; bind_request_count++;
bind_service_callback_.Run(); bind_service_callback_.Run();
} }
......
...@@ -53,10 +53,9 @@ class ServiceConnectionImpl : public ServiceConnection, ...@@ -53,10 +53,9 @@ class ServiceConnectionImpl : public ServiceConnection,
const std::string& interface_name, const std::string& interface_name,
mojo::PendingRemote<mojom::CfmServiceAdaptor> adaptor_remote, mojo::PendingRemote<mojom::CfmServiceAdaptor> adaptor_remote,
ProvideAdaptorCallback callback) override; ProvideAdaptorCallback callback) override;
void BindRegistry( void RequestBindService(const std::string& interface_name,
const std::string& interface_name, mojo::ScopedMessagePipeHandle receiver_pipe,
mojo::PendingReceiver<mojom::CfmServiceRegistry> broker_receiver, RequestBindServiceCallback callback) override;
BindRegistryCallback callback) override;
void OnMojoConnectionError(); void OnMojoConnectionError();
...@@ -154,14 +153,14 @@ void ServiceConnectionImpl::ProvideAdaptor( ...@@ -154,14 +153,14 @@ void ServiceConnectionImpl::ProvideAdaptor(
std::move(callback)); std::move(callback));
} }
void ServiceConnectionImpl::BindRegistry( void ServiceConnectionImpl::RequestBindService(
const std::string& interface_name, const std::string& interface_name,
mojo::PendingReceiver<mojom::CfmServiceRegistry> broker_receiver, mojo::ScopedMessagePipeHandle receiver_pipe,
BindRegistryCallback callback) { RequestBindServiceCallback callback) {
BindPlatformServiceContextIfNeeded(); BindPlatformServiceContextIfNeeded();
remote_->BindRegistry(std::move(interface_name), std::move(broker_receiver), remote_->RequestBindService(std::move(interface_name),
std::move(callback)); std::move(receiver_pipe), std::move(callback));
} }
void ServiceConnectionImpl::OnMojoConnectionError() { void ServiceConnectionImpl::OnMojoConnectionError() {
......
These mojoms are for a service that exists outside of chromium. Implementation These mojoms are for a service that exists outside of chromium. Implementation
for CfmServiceContext and CfmServiceRegistry are found in the private CrOS repo. for CfmServiceContext are found in the private CrOS repo.
Note: //chromeos/services/chromebox_for_meetings/public/mojom/cfm_service_manager.mojom Note: //chromeos/services/chromebox_for_meetings/public/mojom/cfm_service_manager.mojom
is copied as is from its chromeos version as such the original file should be is copied as is from its chromeos version as such the original file should be
......
...@@ -17,20 +17,10 @@ interface CfmServiceContext { ...@@ -17,20 +17,10 @@ interface CfmServiceContext {
string interface_name, string interface_name,
pending_remote<CfmServiceAdaptor> adaptor_remote) => (bool success); pending_remote<CfmServiceAdaptor> adaptor_remote) => (bool success);
// Request to bind |CfmServiceRegistry| requests assuming a valid
// |interface_name| is given.
// Note: |interface_name| MUST be the interface name, i.e. Interface::Name_
BindRegistry@1(
string interface_name,
pending_receiver<CfmServiceRegistry> registry_receiver) => (bool success);
};
// Interface used to bind to registered interfaces by name
interface CfmServiceRegistry {
// Attempt to bind the |receiver_pipe| of an intended mojo::Receiver // Attempt to bind the |receiver_pipe| of an intended mojo::Receiver
// to a remote service that is internally identified as |interface_name| // to a remote service that is internally identified as |interface_name|
// Note: |interface_name| MUST be the interface name, i.e. Interface::Name_ // Note: |interface_name| MUST be the interface name, i.e. Interface::Name_
RequestBindService@0( RequestBindService@1(
string interface_name, string interface_name,
handle<message_pipe> receiver_pipe) => (bool success); handle<message_pipe> receiver_pipe) => (bool success);
}; };
...@@ -47,5 +37,5 @@ interface CfmServiceAdaptor { ...@@ -47,5 +37,5 @@ interface CfmServiceAdaptor {
// FooImpl assuming FooImpl also implements the CfmServiceAdaptor. // FooImpl assuming FooImpl also implements the CfmServiceAdaptor.
// Note: Ideally services should handle multiple clients (i.e BindingSet) // Note: Ideally services should handle multiple clients (i.e BindingSet)
// unless explicitly stated. // unless explicitly stated.
BindService@0(handle<message_pipe> receiver_pipe); OnBindService@0(handle<message_pipe> receiver_pipe);
}; };
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