Skip to content

Commit

Permalink
Convert mojom::ServiceRequest to PendingReceiver<mojom::Service>
Browse files Browse the repository at this point in the history
There are many uses of mojom::ServiceRequest. This CL migrates
them to PendingReceiver<mojom::Service>.

Besides that, ServiceBinding is renamed to ServiceReceiver and
the variable name is also changed from |binding_| to |receiver_|
or |service_binding_| to |service_receiver_|.

Bug: 955171
Change-Id: I1ae084512b307b76956d10ffd21dcf441e7fed2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274485
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#784627}
  • Loading branch information
Gyuyoung authored and Commit Bot committed Jul 2, 2020
1 parent 74b965a commit bb551f1
Show file tree
Hide file tree
Showing 60 changed files with 389 additions and 340 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <utility>

#include "components/arc/session/arc_bridge_service.h"
#include "mojo/public/cpp/bindings/interface_ptr.h"

namespace arc {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "components/arc/session/arc_bridge_service.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents.h"
#include "mojo/public/cpp/bindings/interface_ptr.h"
#include "net/base/filename_util.h"
#include "ui/aura/window.h"
#include "url/gurl.h"
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/ash/chrome_new_window_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
#include "extensions/browser/extension_registry.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
#include "mojo/public/cpp/bindings/interface_ptr.h"
#include "ui/aura/window.h"
#include "ui/base/base_window.h"
#include "ui/base/page_transition_types.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ void CreateMediaDrmStorage(

#if BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)
void StartExternalMojoBrokerService(
service_manager::mojom::ServiceRequest request) {
mojo::PendingReceiver<service_manager::mojom::Service> receiver) {
service_manager::Service::RunAsyncUntilTermination(
std::make_unique<external_mojo::BrokerService>(std::move(request)));
std::make_unique<external_mojo::BrokerService>(std::move(receiver)));
}
#endif // BUILDFLAG(ENABLE_EXTERNAL_MOJO_SERVICES)

Expand Down
7 changes: 4 additions & 3 deletions chromecast/external_mojo/broker_service/broker_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ service_manager::Manifest MakePackagedServices(

} // namespace

BrokerService::BrokerService(service_manager::mojom::ServiceRequest request)
: service_binding_(this, std::move(request)) {
BrokerService::BrokerService(
mojo::PendingReceiver<service_manager::mojom::Service> receiver)
: service_receiver_(this, std::move(receiver)) {
io_thread_ = std::make_unique<base::Thread>("external_mojo");
io_thread_->StartWithOptions(
base::Thread::Options(base::MessagePumpType::IO, 0));
Expand All @@ -72,7 +73,7 @@ BrokerService::BrokerService(service_manager::mojom::ServiceRequest request)
broker_ = base::SequenceBound<ExternalMojoBroker>(io_thread_->task_runner(),
GetBrokerPath());
broker_.Post(FROM_HERE, &ExternalMojoBroker::InitializeChromium,
service_binding_.GetConnector()->Clone(),
service_receiver_.GetConnector()->Clone(),
external_services_to_proxy);
}

Expand Down
8 changes: 5 additions & 3 deletions chromecast/external_mojo/broker_service/broker_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/threading/sequence_bound.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "services/service_manager/public/cpp/manifest.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/cpp/service_receiver.h"
#include "services/service_manager/public/mojom/service.mojom.h"

namespace base {
Expand Down Expand Up @@ -44,11 +45,12 @@ class BrokerService : public ::service_manager::Service {
// Returns the manifest for this service.
static const service_manager::Manifest& GetManifest();

explicit BrokerService(service_manager::mojom::ServiceRequest request);
explicit BrokerService(
mojo::PendingReceiver<service_manager::mojom::Service> receiver);
~BrokerService() override;

private:
service_manager::ServiceBinding service_binding_;
service_manager::ServiceReceiver service_receiver_;

std::unique_ptr<base::Thread> io_thread_;
scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/check.h"
#include "base/token.h"
#include "chromecast/external_mojo/external_service_support/external_connector.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
Expand Down
6 changes: 3 additions & 3 deletions chromecast/external_mojo/public/cpp/external_mojo_broker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
#include "services/service_manager/public/cpp/constants.h"
#include "services/service_manager/public/cpp/identity.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/cpp/service_filter.h"
#include "services/service_manager/public/cpp/service_receiver.h"
#include "services/service_manager/public/mojom/connector.mojom.h"
#include "services/service_manager/public/mojom/service.mojom.h"

Expand Down Expand Up @@ -92,7 +92,7 @@ class ExternalMojoBroker::ConnectorImpl : public mojom::ExternalConnector {
mojo::PendingReceiver<::service_manager::mojom::Service> receiver)
: connector_(connector),
service_name_(std::move(service_name)),
service_binding_(this, std::move(receiver)) {
service_receiver_(this, std::move(receiver)) {
DCHECK(connector_);
}

Expand All @@ -107,7 +107,7 @@ class ExternalMojoBroker::ConnectorImpl : public mojom::ExternalConnector {

ConnectorImpl* const connector_;
const std::string service_name_;
service_manager::ServiceBinding service_binding_;
service_manager::ServiceReceiver service_receiver_;

DISALLOW_COPY_AND_ASSIGN(ExternalServiceProxy);
};
Expand Down
43 changes: 22 additions & 21 deletions content/common/service_manager/service_manager_connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/cpp/service_receiver.h"
#include "services/service_manager/public/mojom/constants.mojom.h"

#if defined(OS_ANDROID)
Expand All @@ -50,11 +50,12 @@ class ServiceManagerConnectionImpl::IOThreadContext
: public base::RefCountedThreadSafe<IOThreadContext>,
public service_manager::Service {
public:
IOThreadContext(service_manager::mojom::ServiceRequest service_request,
scoped_refptr<base::SequencedTaskRunner> io_task_runner,
mojo::PendingReceiver<service_manager::mojom::Connector>
connector_receiver)
: pending_service_request_(std::move(service_request)),
IOThreadContext(
mojo::PendingReceiver<service_manager::mojom::Service> service_receiver,
scoped_refptr<base::SequencedTaskRunner> io_task_runner,
mojo::PendingReceiver<service_manager::mojom::Connector>
connector_receiver)
: pending_service_receiver_(std::move(service_receiver)),
io_task_runner_(io_task_runner),
pending_connector_receiver_(std::move(connector_receiver)) {
// This will be reattached by any of the IO thread functions on first call.
Expand Down Expand Up @@ -155,18 +156,18 @@ class ServiceManagerConnectionImpl::IOThreadContext

static void WrapServiceRequestHandlerNoCallback(
const ServiceRequestHandler& handler,
service_manager::mojom::ServiceRequest request,
mojo::PendingReceiver<service_manager::mojom::Service> receiver,
CreatePackagedServiceInstanceCallback callback) {
handler.Run(std::move(request));
handler.Run(std::move(receiver));
std::move(callback).Run(base::GetCurrentProcId());
}

void StartOnIOThread() {
// Should bind |io_thread_checker_| to the context's thread.
DCHECK(io_thread_checker_.CalledOnValidThread());
service_binding_ = std::make_unique<service_manager::ServiceBinding>(
this, std::move(pending_service_request_));
service_binding_->GetConnector()->BindConnectorReceiver(
service_receiver_ = std::make_unique<service_manager::ServiceReceiver>(
this, std::move(pending_service_receiver_));
service_receiver_->GetConnector()->BindConnectorReceiver(
std::move(pending_connector_receiver_));

// MessageLoopObserver owns itself.
Expand Down Expand Up @@ -196,7 +197,7 @@ class ServiceManagerConnectionImpl::IOThreadContext
// unwinds.
scoped_refptr<IOThreadContext> keepalive(this);

service_binding_.reset();
service_receiver_.reset();

StopOnIOThread();
}
Expand All @@ -222,20 +223,19 @@ class ServiceManagerConnectionImpl::IOThreadContext
mojo::PendingReceiver<service_manager::mojom::Service> receiver,
CreatePackagedServiceInstanceCallback callback) override {
DCHECK(io_thread_checker_.CalledOnValidThread());
service_manager::mojom::ServiceRequest request(std::move(receiver));
auto it = request_handlers_.find(service_name);
if (it == request_handlers_.end()) {
if (default_request_handler_) {
callback_task_runner_->PostTask(
FROM_HERE, base::BindOnce(default_request_handler_, service_name,
std::move(request)));
std::move(receiver)));
} else {
LOG(ERROR) << "Can't create service " << service_name
<< ". No handler found.";
}
std::move(callback).Run(base::nullopt);
} else {
it->second.Run(std::move(request), std::move(callback));
it->second.Run(std::move(receiver), std::move(callback));
}
}

Expand All @@ -251,7 +251,8 @@ class ServiceManagerConnectionImpl::IOThreadContext

// Temporary state established on construction and consumed on the IO thread
// once the connection is started.
service_manager::mojom::ServiceRequest pending_service_request_;
mojo::PendingReceiver<service_manager::mojom::Service>
pending_service_receiver_;
scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
mojo::PendingReceiver<service_manager::mojom::Connector>
pending_connector_receiver_;
Expand All @@ -263,7 +264,7 @@ class ServiceManagerConnectionImpl::IOThreadContext
// Callback to run if the service is stopped by the service manager.
base::OnceClosure stop_callback_;

std::unique_ptr<service_manager::ServiceBinding> service_binding_;
std::unique_ptr<service_manager::ServiceReceiver> service_receiver_;

// Not owned.
MessageLoopObserver* message_loop_observer_ = nullptr;
Expand Down Expand Up @@ -321,11 +322,11 @@ void ServiceManagerConnection::SetFactoryForTest(Factory* factory) {

// static
std::unique_ptr<ServiceManagerConnection> ServiceManagerConnection::Create(
service_manager::mojom::ServiceRequest request,
mojo::PendingReceiver<service_manager::mojom::Service> receiver,
scoped_refptr<base::SequencedTaskRunner> io_task_runner) {
if (service_manager_connection_factory)
return service_manager_connection_factory->Run();
return std::make_unique<ServiceManagerConnectionImpl>(std::move(request),
return std::make_unique<ServiceManagerConnectionImpl>(std::move(receiver),
io_task_runner);
}

Expand All @@ -335,11 +336,11 @@ ServiceManagerConnection::~ServiceManagerConnection() {}
// ServiceManagerConnectionImpl, public:

ServiceManagerConnectionImpl::ServiceManagerConnectionImpl(
service_manager::mojom::ServiceRequest request,
mojo::PendingReceiver<service_manager::mojom::Service> receiver,
scoped_refptr<base::SequencedTaskRunner> io_task_runner) {
mojo::PendingReceiver<service_manager::mojom::Connector> connector_receiver;
connector_ = service_manager::Connector::Create(&connector_receiver);
context_ = new IOThreadContext(std::move(request), io_task_runner,
context_ = new IOThreadContext(std::move(receiver), io_task_runner,
std::move(connector_receiver));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/sequenced_task_runner.h"
#include "content/common/content_export.h"
#include "content/public/common/service_manager_connection.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/system/message_pipe.h"
#include "services/service_manager/public/cpp/identity.h"
#include "services/service_manager/public/mojom/service.mojom.h"
Expand All @@ -27,7 +28,7 @@ class CONTENT_EXPORT ServiceManagerConnectionImpl
: public ServiceManagerConnection {
public:
explicit ServiceManagerConnectionImpl(
service_manager::mojom::ServiceRequest request,
mojo::PendingReceiver<service_manager::mojom::Service> receiver,
scoped_refptr<base::SequencedTaskRunner> io_task_runner);
~ServiceManagerConnectionImpl() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ TEST(ServiceManagerConnectionImplTest, ServiceLaunchThreading) {
base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED);
connection.AddServiceRequestHandler(
kTestServiceName, base::BindLambdaForTesting(
[&event](service_manager::mojom::ServiceRequest) {
event.Signal();
}));
kTestServiceName,
base::BindLambdaForTesting(
[&event](mojo::PendingReceiver<service_manager::mojom::Service>) {
event.Signal();
}));
connection.Start();

mojo::PendingRemote<service_manager::mojom::Service> packaged_service;
Expand Down
15 changes: 8 additions & 7 deletions content/public/common/service_manager_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/callback_forward.h"
#include "base/sequenced_task_runner.h"
#include "content/common/content_export.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "services/service_manager/public/cpp/identity.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/mojom/connector.mojom-forward.h"
Expand All @@ -33,10 +34,10 @@ namespace content {
// ContentBrowserClient::RegisterInProcessServices().
class CONTENT_EXPORT ServiceManagerConnection {
public:
using ServiceRequestHandler =
base::RepeatingCallback<void(service_manager::mojom::ServiceRequest)>;
using ServiceRequestHandler = base::RepeatingCallback<void(
mojo::PendingReceiver<service_manager::mojom::Service>)>;
using ServiceRequestHandlerWithCallback = base::RepeatingCallback<void(
service_manager::mojom::ServiceRequest,
mojo::PendingReceiver<service_manager::mojom::Service>,
service_manager::Service::CreatePackagedServiceInstanceCallback)>;
using Factory =
base::RepeatingCallback<std::unique_ptr<ServiceManagerConnection>(void)>;
Expand Down Expand Up @@ -65,7 +66,7 @@ class CONTENT_EXPORT ServiceManagerConnection {
// its interfaces and accept new connections on |io_task_runner| only. Note
// that no incoming connections are accepted until Start() is called.
static std::unique_ptr<ServiceManagerConnection> Create(
service_manager::mojom::ServiceRequest request,
mojo::PendingReceiver<service_manager::mojom::Service> receiver,
scoped_refptr<base::SequencedTaskRunner> io_task_runner);

// Begins accepting incoming connections.
Expand Down Expand Up @@ -100,9 +101,9 @@ class CONTENT_EXPORT ServiceManagerConnection {

// Sets a request handler to use if no registered handlers were interested in
// an incoming service request. Must be called before |Start()|.
using DefaultServiceRequestHandler =
base::RepeatingCallback<void(const std::string& service_name,
service_manager::mojom::ServiceRequest)>;
using DefaultServiceRequestHandler = base::RepeatingCallback<void(
const std::string& service_name,
mojo::PendingReceiver<service_manager::mojom::Service>)>;
virtual void SetDefaultServiceRequestHandler(
const DefaultServiceRequestHandler& handler) = 0;
};
Expand Down
5 changes: 3 additions & 2 deletions content/public/test/test_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ namespace content {

const char kTestServiceUrl[] = "system:content_test_service";

TestService::TestService(service_manager::mojom::ServiceRequest request)
: service_binding_(this, std::move(request)) {
TestService::TestService(
mojo::PendingReceiver<service_manager::mojom::Service> receiver)
: service_receiver_(this, std::move(receiver)) {
registry_.AddInterface<mojom::TestService>(
base::BindRepeating(&TestService::Create, base::Unretained(this)));
}
Expand Down
7 changes: 4 additions & 3 deletions content/public/test/test_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "mojo/public/cpp/bindings/receiver.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "services/service_manager/public/cpp/service.h"
#include "services/service_manager/public/cpp/service_binding.h"
#include "services/service_manager/public/cpp/service_receiver.h"
#include "services/service_manager/public/mojom/service.mojom.h"

namespace content {
Expand All @@ -24,7 +24,8 @@ extern const char kTestServiceUrl[];
// terminates itself after its TestService fulfills a single DoSomething call.
class TestService : public service_manager::Service, public mojom::TestService {
public:
explicit TestService(service_manager::mojom::ServiceRequest request);
explicit TestService(
mojo::PendingReceiver<service_manager::mojom::Service> receiver);
~TestService() override;

private:
Expand Down Expand Up @@ -52,7 +53,7 @@ class TestService : public service_manager::Service, public mojom::TestService {
CreateUnsafeSharedMemoryRegionCallback callback) override;
void IsProcessSandboxed(IsProcessSandboxedCallback callback) override;

service_manager::ServiceBinding service_binding_;
service_manager::ServiceReceiver service_receiver_;
service_manager::BinderRegistry registry_;
mojo::Receiver<mojom::TestService> receiver_{this};

Expand Down
2 changes: 1 addition & 1 deletion content/public/utility/content_utility_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ bool ContentUtilityClient::OnMessageReceived(const IPC::Message& message) {

bool ContentUtilityClient::HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) {
mojo::PendingReceiver<service_manager::mojom::Service> receiver) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion content/public/utility/content_utility_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class CONTENT_EXPORT ContentUtilityClient {
// If the embedder returns |false| this process is terminated immediately.
virtual bool HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request);
mojo::PendingReceiver<service_manager::mojom::Service> receiver);

// Allows the embedder to handle an incoming service interface request to run
// a service on the IO thread. Should return a ServiceFactory instance which
Expand Down
4 changes: 2 additions & 2 deletions content/shell/utility/shell_content_utility_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ void ShellContentUtilityClient::ExposeInterfacesToBrowser(

bool ShellContentUtilityClient::HandleServiceRequest(
const std::string& service_name,
service_manager::mojom::ServiceRequest request) {
mojo::PendingReceiver<service_manager::mojom::Service> receiver) {
std::unique_ptr<service_manager::Service> service;
if (service_name == kTestServiceUrl) {
service = std::make_unique<TestService>(std::move(request));
service = std::make_unique<TestService>(std::move(receiver));
}

if (service) {
Expand Down
Loading

0 comments on commit bb551f1

Please sign in to comment.