Skip to content

Commit

Permalink
Mojo C++ Bindings: Remove redundant AssociatedInterfaceRequest APIs
Browse files Browse the repository at this point in the history
Adds an explicit constructor over a ScopedInterfaceEndpointHandle and
removes the Bind method. Also removes the now-useless
MakeAssocaiatedRequest template helper.

BUG=721507
R=yzshen@chromium.org
TBR=jam@chromium.org

Change-Id: I80f90460bcbb2b275e9609205d5d1c112edb0325
Reviewed-on: https://chromium-review.googlesource.com/505330
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471886}
  • Loading branch information
krockot authored and Commit Bot committed May 15, 2017
1 parent 1ffabc5 commit 96d1b7b
Show file tree
Hide file tree
Showing 20 changed files with 44 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,7 @@ class PasswordAutofillAgentTest : public ChromeRenderViewTest {

void BindPasswordManagerClient(mojo::ScopedInterfaceEndpointHandle handle) {
fake_pw_client_.BindRequest(
mojo::MakeAssociatedRequest<mojom::PasswordManagerClient>(
std::move(handle)));
mojom::PasswordManagerClientAssociatedRequest(std::move(handle)));
}

FakeContentPasswordManagerDriver fake_driver_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ class PasswordGenerationAgentTest : public ChromeRenderViewTest {

void BindPasswordManagerClient(mojo::ScopedInterfaceEndpointHandle handle) {
fake_pw_client_.BindRequest(
mojo::MakeAssociatedRequest<mojom::PasswordManagerClient>(
std::move(handle)));
mojom::PasswordManagerClientAssociatedRequest(std::move(handle)));
}

FakeContentPasswordManagerDriver fake_driver_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ bool ServiceWorkerDispatcherHost::OnMessageReceived(
void ServiceWorkerDispatcherHost::AddMojoBinding(
mojo::ScopedInterfaceEndpointHandle handle) {
bindings_.AddBinding(
this, mojo::MakeAssociatedRequest<mojom::ServiceWorkerDispatcherHost>(
std::move(handle)));
this,
mojom::ServiceWorkerDispatcherHostAssociatedRequest(std::move(handle)));
}

bool ServiceWorkerDispatcherHost::Send(IPC::Message* message) {
Expand Down
5 changes: 2 additions & 3 deletions content/child/child_thread_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -737,9 +737,8 @@ void ChildThreadImpl::OnAssociatedInterfaceRequest(
mojo::ScopedInterfaceEndpointHandle handle) {
if (interface_name == mojom::RouteProvider::Name_) {
DCHECK(!route_provider_binding_.is_bound());
mojom::RouteProviderAssociatedRequest request;
request.Bind(std::move(handle));
route_provider_binding_.Bind(std::move(request));
route_provider_binding_.Bind(
mojom::RouteProviderAssociatedRequest(std::move(handle)));
} else {
LOG(ERROR) << "Request for unknown Channel-associated interface: "
<< interface_name;
Expand Down
5 changes: 2 additions & 3 deletions content/common/associated_interface_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,8 @@ AssociatedInterfaceProviderImpl::~AssociatedInterfaceProviderImpl() {}
void AssociatedInterfaceProviderImpl::GetInterface(
const std::string& name,
mojo::ScopedInterfaceEndpointHandle handle) {
mojom::AssociatedInterfaceAssociatedRequest request;
request.Bind(std::move(handle));
proxy_->GetAssociatedInterface(name, std::move(request));
proxy_->GetAssociatedInterface(
name, mojom::AssociatedInterfaceAssociatedRequest(std::move(handle)));
}

void AssociatedInterfaceProviderImpl::OverrideBinderForTesting(
Expand Down
4 changes: 2 additions & 2 deletions content/public/browser/browser_associated_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ class BrowserAssociatedInterface {
// If this interface has already been shut down we drop the request.
if (!bindings_)
return;
bindings_->AddBinding(
impl_, mojo::MakeAssociatedRequest<Interface>(std::move(handle)));
bindings_->AddBinding(impl_, mojo::AssociatedInterfaceRequest<Interface>(
std::move(handle)));
}

private:
Expand Down
6 changes: 3 additions & 3 deletions content/public/browser/web_contents_binding_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ class WebContentsFrameBindingSet : public WebContentsBindingSet {
void OnRequestForFrame(
RenderFrameHost* render_frame_host,
mojo::ScopedInterfaceEndpointHandle handle) override {
mojo::AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle));
bindings_.AddBinding(impl_, std::move(request), render_frame_host);
bindings_.AddBinding(
impl_, mojo::AssociatedInterfaceRequest<Interface>(std::move(handle)),
render_frame_host);
}

Interface* const impl_;
Expand Down
4 changes: 1 addition & 3 deletions content/public/common/associated_interface_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ class AssociatedInterfaceRegistry {
template <typename Interface>
static void BindInterface(const InterfaceBinder<Interface>& binder,
mojo::ScopedInterfaceEndpointHandle handle) {
mojo::AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle));
binder.Run(std::move(request));
binder.Run(mojo::AssociatedInterfaceRequest<Interface>(std::move(handle)));
}
};

Expand Down
5 changes: 2 additions & 3 deletions content/public/test/web_contents_binding_set_test_binder.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ class WebContentsBindingSetTestBinder : public WebContentsBindingSet::Binder {
// Binder:
void OnRequestForFrame(RenderFrameHost* render_frame_host,
mojo::ScopedInterfaceEndpointHandle handle) override {
mojo::AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle));
BindRequest(render_frame_host, std::move(request));
BindRequest(render_frame_host,
mojo::AssociatedInterfaceRequest<Interface>(std::move(handle)));
}
};

Expand Down
3 changes: 1 addition & 2 deletions content/test/test_render_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class MockFrameHost : public mojom::FrameHost {
}

void Bind(mojo::ScopedInterfaceEndpointHandle handle) {
binding_.Bind(
mojo::MakeAssociatedRequest<mojom::FrameHost>(std::move(handle)));
binding_.Bind(mojom::FrameHostAssociatedRequest(std::move(handle)));
}

private:
Expand Down
5 changes: 2 additions & 3 deletions ipc/ipc_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,8 @@ class IPC_EXPORT Channel : public Sender {
static void BindAssociatedInterfaceRequest(
const AssociatedInterfaceFactory<Interface>& factory,
mojo::ScopedInterfaceEndpointHandle handle) {
mojo::AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle));
factory.Run(std::move(request));
factory.Run(
mojo::AssociatedInterfaceRequest<Interface>(std::move(handle)));
}
};

Expand Down
21 changes: 8 additions & 13 deletions ipc/ipc_channel_mojo_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -724,9 +724,8 @@ class ListenerWithSimpleProxyAssociatedInterface
const std::string& interface_name,
mojo::ScopedInterfaceEndpointHandle handle) override {
DCHECK_EQ(interface_name, IPC::mojom::SimpleTestDriver::Name_);
IPC::mojom::SimpleTestDriverAssociatedRequest request;
request.Bind(std::move(handle));
binding_.Bind(std::move(request));
binding_.Bind(
IPC::mojom::SimpleTestDriverAssociatedRequest(std::move(handle)));
}

bool received_all_messages() const {
Expand Down Expand Up @@ -856,9 +855,8 @@ class ListenerWithIndirectProxyAssociatedInterface
mojo::ScopedInterfaceEndpointHandle handle) override {
DCHECK(!driver_binding_.is_bound());
DCHECK_EQ(interface_name, IPC::mojom::IndirectTestDriver::Name_);
IPC::mojom::IndirectTestDriverAssociatedRequest request;
request.Bind(std::move(handle));
driver_binding_.Bind(std::move(request));
driver_binding_.Bind(
IPC::mojom::IndirectTestDriverAssociatedRequest(std::move(handle)));
}

void set_ping_handler(const base::Closure& handler) {
Expand Down Expand Up @@ -986,10 +984,8 @@ class ListenerWithSyncAssociatedInterface
mojo::ScopedInterfaceEndpointHandle handle) override {
DCHECK(!binding_.is_bound());
DCHECK_EQ(interface_name, IPC::mojom::SimpleTestDriver::Name_);

IPC::mojom::SimpleTestDriverAssociatedRequest request;
request.Bind(std::move(handle));
binding_.Bind(std::move(request));
binding_.Bind(
IPC::mojom::SimpleTestDriverAssociatedRequest(std::move(handle)));
}

void BindRequest(IPC::mojom::SimpleTestDriverAssociatedRequest request) {
Expand Down Expand Up @@ -1122,9 +1118,8 @@ class SimpleTestClientImpl : public IPC::mojom::SimpleTestClient,
DCHECK(!binding_.is_bound());
DCHECK_EQ(interface_name, IPC::mojom::SimpleTestClient::Name_);

IPC::mojom::SimpleTestClientAssociatedRequest request;
request.Bind(std::move(handle));
binding_.Bind(std::move(request));
binding_.Bind(
IPC::mojom::SimpleTestClientAssociatedRequest(std::move(handle)));
}

bool use_sync_sender_ = false;
Expand Down
6 changes: 2 additions & 4 deletions ipc/ipc_channel_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -562,10 +562,8 @@ void ChannelProxy::GetGenericRemoteAssociatedInterface(
const std::string& name,
mojo::ScopedInterfaceEndpointHandle handle) {
DCHECK(did_init_);
mojom::GenericInterfaceAssociatedRequest request;
request.Bind(std::move(handle));
context()->thread_safe_channel().GetAssociatedInterface(name,
std::move(request));
context()->thread_safe_channel().GetAssociatedInterface(
name, mojom::GenericInterfaceAssociatedRequest(std::move(handle)));
}

void ChannelProxy::ClearIPCTaskRunner() {
Expand Down
4 changes: 1 addition & 3 deletions ipc/ipc_channel_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,7 @@ class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe {
static void BindAssociatedInterfaceRequest(
const AssociatedInterfaceFactory<Interface>& factory,
mojo::ScopedInterfaceEndpointHandle handle) {
mojo::AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle));
factory.Run(std::move(request));
factory.Run(mojo::AssociatedInterfaceRequest<Interface>(std::move(handle)));
}

// Always called once immediately after Init.
Expand Down
5 changes: 2 additions & 3 deletions ipc/ipc_message_pipe_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ void MessagePipeReader::GetRemoteInterface(
mojo::ScopedInterfaceEndpointHandle handle) {
if (!sender_.is_bound())
return;
mojom::GenericInterfaceAssociatedRequest request;
request.Bind(std::move(handle));
sender_->GetAssociatedInterface(name, std::move(request));
sender_->GetAssociatedInterface(
name, mojom::GenericInterfaceAssociatedRequest(std::move(handle)));
}

void MessagePipeReader::SetPeerPid(int32_t peer_pid) {
Expand Down
2 changes: 1 addition & 1 deletion ipc/ipc_mojo_bootstrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class ChannelAssociatedGroupController
CreateScopedInterfaceEndpointHandle(receiver_id);

sender->Bind(mojom::ChannelAssociatedPtrInfo(std::move(sender_handle), 0));
receiver->Bind(std::move(receiver_handle));
*receiver = mojom::ChannelAssociatedRequest(std::move(receiver_handle));
}

void ShutDown() {
Expand Down
17 changes: 2 additions & 15 deletions mojo/public/cpp/bindings/associated_binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,6 @@ class AssociatedBinding : public AssociatedBindingBase {
stub_.set_sink(std::move(impl));
}

// Constructs a completed associated binding of |impl|. The output |ptr_info|
// should be sent by another interface. |impl| must outlive this object.
AssociatedBinding(ImplPointerType impl,
AssociatedInterfacePtrInfo<Interface>* ptr_info,
scoped_refptr<base::SingleThreadTaskRunner> runner =
base::ThreadTaskRunnerHandle::Get())
: AssociatedBinding(std::move(impl)) {
Bind(ptr_info, std::move(runner));
}

// Constructs a completed associated binding of |impl|. |impl| must outlive
// the binding.
AssociatedBinding(ImplPointerType impl,
Expand Down Expand Up @@ -138,12 +128,9 @@ class AssociatedBinding : public AssociatedBindingBase {
// implementation. Puts this object into a state where it can be rebound.
AssociatedInterfaceRequest<Interface> Unbind() {
DCHECK(endpoint_client_);

AssociatedInterfaceRequest<Interface> request;
request.Bind(endpoint_client_->PassHandle());

AssociatedInterfaceRequest<Interface> request(
endpoint_client_->PassHandle());
endpoint_client_.reset();

return request;
}

Expand Down
9 changes: 2 additions & 7 deletions mojo/public/cpp/bindings/associated_interface_ptr.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,7 @@ AssociatedInterfaceRequest<Interface> MakeRequest(
ptr_info->set_handle(std::move(handle0));
ptr_info->set_version(0);

AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle1));
return request;
return AssociatedInterfaceRequest<Interface>(std::move(handle1));
}

// Like MakeRequest() above, but it creates a dedicated message pipe. The
Expand Down Expand Up @@ -265,10 +263,7 @@ AssociatedInterfaceRequest<Interface> MakeIsolatedRequest(

ptr->Bind(AssociatedInterfacePtrInfo<Interface>(std::move(endpoint0),
Interface::Version_));

AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(endpoint1));
return request;
return AssociatedInterfaceRequest<Interface>(std::move(endpoint1));
}

// |handle| is supposed to be the request of an associated interface. This
Expand Down
22 changes: 5 additions & 17 deletions mojo/public/cpp/bindings/associated_interface_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,15 @@ class AssociatedInterfaceRequest {
AssociatedInterfaceRequest() {}
AssociatedInterfaceRequest(decltype(nullptr)) {}

explicit AssociatedInterfaceRequest(ScopedInterfaceEndpointHandle handle)
: handle_(std::move(handle)) {}

// Takes the interface endpoint handle from another
// AssociatedInterfaceRequest.
AssociatedInterfaceRequest(AssociatedInterfaceRequest&& other) {
handle_ = std::move(other.handle_);
}

AssociatedInterfaceRequest& operator=(AssociatedInterfaceRequest&& other) {
if (this != &other)
handle_ = std::move(other.handle_);
Expand All @@ -46,13 +50,7 @@ class AssociatedInterfaceRequest {
// handle.
bool is_pending() const { return handle_.is_valid(); }

void Bind(ScopedInterfaceEndpointHandle handle) {
handle_ = std::move(handle);
}

ScopedInterfaceEndpointHandle PassHandle() {
return std::move(handle_);
}
ScopedInterfaceEndpointHandle PassHandle() { return std::move(handle_); }

const ScopedInterfaceEndpointHandle& handle() const { return handle_; }

Expand All @@ -75,16 +73,6 @@ class AssociatedInterfaceRequest {
DISALLOW_COPY_AND_ASSIGN(AssociatedInterfaceRequest);
};

// Makes an AssociatedInterfaceRequest bound to the specified associated
// endpoint.
template <typename Interface>
AssociatedInterfaceRequest<Interface> MakeAssociatedRequest(
ScopedInterfaceEndpointHandle handle) {
AssociatedInterfaceRequest<Interface> request;
request.Bind(std::move(handle));
return request;
}

} // namespace mojo

#endif // MOJO_PUBLIC_CPP_BINDINGS_ASSOCIATED_INTERFACE_REQUEST_H_
4 changes: 2 additions & 2 deletions mojo/public/cpp/bindings/lib/handle_interface_serialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ struct Serializer<AssociatedInterfaceRequestDataView<Base>,
SerializationContext* context) {
if (input->is_valid()) {
DCHECK_LT(input->value, context->associated_endpoint_handles.size());
output->Bind(
*output = AssociatedInterfaceRequest<T>(
std::move(context->associated_endpoint_handles[input->value]));
} else {
output->Bind(ScopedInterfaceEndpointHandle());
*output = AssociatedInterfaceRequest<T>();
}
return true;
}
Expand Down

0 comments on commit 96d1b7b

Please sign in to comment.