Skip to content

Commit

Permalink
Convert HidManagerClient to new Mojo types
Browse files Browse the repository at this point in the history
This CL converts HidManagerClient{AssociatedPtr, AssociatedPtrInfo
and AssociatedRequest} in device, services and extensions to the
new Mojo type.

Bug: 955171
Change-Id: I86e47ad4d6bdb35a89cca8337bd066aa167b2c5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1803938
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697482}
  • Loading branch information
MyidShin authored and Commit Bot committed Sep 18, 2019
1 parent 35e8900 commit 657f4b1
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 79 deletions.
10 changes: 3 additions & 7 deletions device/fido/hid/fake_hid_impl_for_testing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ void FakeFidoHidManager::GetDevicesAndSetClient(
GetDevicesCallback callback) {
GetDevices(std::move(callback));

device::mojom::HidManagerClientAssociatedPtr client_ptr;
client_ptr.Bind(std::move(client));
clients_.AddPtr(std::move(client_ptr));
clients_.Add(std::move(client));
}

void FakeFidoHidManager::GetDevices(GetDevicesCallback callback) {
Expand All @@ -185,9 +183,8 @@ void FakeFidoHidManager::Connect(

void FakeFidoHidManager::AddDevice(device::mojom::HidDeviceInfoPtr device) {
device::mojom::HidDeviceInfo* device_info = device.get();
clients_.ForAllPtrs([device_info](device::mojom::HidManagerClient* client) {
for (auto& client : clients_)
client->DeviceAdded(device_info->Clone());
});

devices_[device->guid] = std::move(device);
}
Expand All @@ -205,9 +202,8 @@ void FakeFidoHidManager::RemoveDevice(const std::string device_guid) {
return;

device::mojom::HidDeviceInfo* device_info = it->second.get();
clients_.ForAllPtrs([device_info](device::mojom::HidManagerClient* client) {
for (auto& client : clients_)
client->DeviceRemoved(device_info->Clone());
});
devices_.erase(it);
}

Expand Down
4 changes: 2 additions & 2 deletions device/fido/hid/fake_hid_impl_for_testing.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "device/fido/fido_constants.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote_set.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "services/device/public/mojom/hid.mojom.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -122,7 +122,7 @@ class FakeFidoHidManager : public device::mojom::HidManager {
private:
std::map<std::string, device::mojom::HidDeviceInfoPtr> devices_;
std::map<std::string, device::mojom::HidConnectionPtr> connections_;
mojo::AssociatedInterfacePtrSet<device::mojom::HidManagerClient> clients_;
mojo::AssociatedRemoteSet<device::mojom::HidManagerClient> clients_;
mojo::ReceiverSet<device::mojom::HidManager> receivers_;

DISALLOW_COPY_AND_ASSIGN(FakeFidoHidManager);
Expand Down
11 changes: 4 additions & 7 deletions device/fido/hid/fido_hid_discovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@

#include "base/bind.h"
#include "device/fido/hid/fido_hid_device.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "services/device/public/mojom/constants.mojom.h"
#include "services/service_manager/public/cpp/connector.h"

namespace device {

FidoHidDiscovery::FidoHidDiscovery(::service_manager::Connector* connector)
: FidoDeviceDiscovery(FidoTransportProtocol::kUsbHumanInterfaceDevice),
connector_(connector),
binding_(this) {
connector_(connector) {
// TODO(piperc@): Give this constant a name.
filter_.SetUsagePage(0xf1d0);
}
Expand All @@ -28,12 +26,11 @@ void FidoHidDiscovery::StartInternal() {
DCHECK(connector_);
connector_->Connect(device::mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());
device::mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));

hid_manager_->GetDevicesAndSetClient(
std::move(client), base::BindOnce(&FidoHidDiscovery::OnGetDevices,
weak_factory_.GetWeakPtr()));
receiver_.BindNewEndpointAndPassRemote(),
base::BindOnce(&FidoHidDiscovery::OnGetDevices,
weak_factory_.GetWeakPtr()));
}

void FidoHidDiscovery::DeviceAdded(
Expand Down
4 changes: 2 additions & 2 deletions device/fido/hid/fido_hid_discovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "device/fido/fido_device_discovery.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/cpp/hid/hid_device_filter.h"
#include "services/device/public/mojom/hid.mojom.h"
Expand Down Expand Up @@ -45,7 +45,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidDiscovery

service_manager::Connector* connector_;
mojo::Remote<device::mojom::HidManager> hid_manager_;
mojo::AssociatedBinding<device::mojom::HidManagerClient> binding_;
mojo::AssociatedReceiver<device::mojom::HidManagerClient> receiver_{this};
HidDeviceFilter filter_;
base::WeakPtrFactory<FidoHidDiscovery> weak_factory_{this};

Expand Down
9 changes: 4 additions & 5 deletions device/gamepad/nintendo_data_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

namespace device {

NintendoDataFetcher::NintendoDataFetcher() : binding_(this) {}
NintendoDataFetcher::NintendoDataFetcher() = default;

NintendoDataFetcher::~NintendoDataFetcher() {
for (auto& entry : controllers_) {
Expand All @@ -32,11 +32,10 @@ void NintendoDataFetcher::OnAddedToProvider() {
// OnGetDevices will be called with a list of connected HID devices.
connector()->Connect(mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());
mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));
hid_manager_->GetDevicesAndSetClient(
std::move(client), base::BindOnce(&NintendoDataFetcher::OnGetDevices,
weak_factory_.GetWeakPtr()));
receiver_.BindNewEndpointAndPassRemote(),
base::BindOnce(&NintendoDataFetcher::OnGetDevices,
weak_factory_.GetWeakPtr()));
}

void NintendoDataFetcher::OnGetDevices(
Expand Down
4 changes: 2 additions & 2 deletions device/gamepad/nintendo_data_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "device/gamepad/gamepad_data_fetcher.h"
#include "device/gamepad/nintendo_controller.h"
#include "device/gamepad/public/cpp/gamepads.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/hid.mojom.h"

Expand Down Expand Up @@ -112,7 +112,7 @@ class DEVICE_GAMEPAD_EXPORT NintendoDataFetcher : public GamepadDataFetcher,
ControllerMap controllers_;

mojo::Remote<mojom::HidManager> hid_manager_;
mojo::AssociatedBinding<mojom::HidManagerClient> binding_;
mojo::AssociatedReceiver<mojom::HidManagerClient> receiver_{this};
base::WeakPtrFactory<NintendoDataFetcher> weak_factory_{this};

DISALLOW_COPY_AND_ASSIGN(NintendoDataFetcher);
Expand Down
13 changes: 4 additions & 9 deletions extensions/browser/api/device_permissions_prompt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
#include "extensions/browser/api/device_permissions_manager.h"
#include "extensions/browser/api/usb/usb_device_manager.h"
#include "extensions/common/extension.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/cpp/hid/hid_device_filter.h"
#include "services/device/public/cpp/hid/hid_usage_and_page.h"
Expand Down Expand Up @@ -195,8 +194,7 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,
: Prompt(extension, context, multiple),
initialized_(false),
filters_(filters),
callback_(callback),
binding_(this) {}
callback_(callback) {}

private:
~HidDevicePermissionsPrompt() override {}
Expand All @@ -221,11 +219,8 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,
connector->Connect(device::mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());

device::mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));

hid_manager_->GetDevicesAndSetClient(
std::move(client),
receiver_.BindNewEndpointAndPassRemote(),
base::BindOnce(&HidDevicePermissionsPrompt::OnDevicesEnumerated, this));

initialized_ = true;
Expand Down Expand Up @@ -300,7 +295,7 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,
std::vector<HidDeviceFilter> filters_;
mojo::Remote<device::mojom::HidManager> hid_manager_;
DevicePermissionsPrompt::HidDevicesCallback callback_;
mojo::AssociatedBinding<device::mojom::HidManagerClient> binding_;
mojo::AssociatedReceiver<device::mojom::HidManagerClient> receiver_{this};
};

} // namespace
Expand Down
7 changes: 2 additions & 5 deletions extensions/browser/api/hid/hid_device_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "extensions/common/permissions/permissions_data.h"
#include "extensions/common/permissions/usb_device_permission.h"
#include "mojo/public/cpp/bindings/callback_helpers.h"
#include "mojo/public/cpp/bindings/interface_request.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "services/device/public/cpp/hid/hid_device_filter.h"
#include "services/device/public/cpp/hid/hid_usage_and_page.h"
Expand Down Expand Up @@ -102,7 +101,7 @@ struct HidDeviceManager::GetApiDevicesParams {
};

HidDeviceManager::HidDeviceManager(content::BrowserContext* context)
: browser_context_(context), binding_(this) {
: browser_context_(context) {
event_router_ = EventRouter::Get(context);
if (event_router_) {
event_router_->RegisterObserver(this, hid::OnDeviceAdded::kEventName);
Expand Down Expand Up @@ -296,10 +295,8 @@ void HidDeviceManager::LazyInitialize() {
}
// Enumerate HID devices and set client.
std::vector<device::mojom::HidDeviceInfoPtr> empty_devices;
device::mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));
hid_manager_->GetDevicesAndSetClient(
std::move(client),
receiver_.BindNewEndpointAndPassRemote(),
mojo::WrapCallbackWithDefaultInvokeIfNotRun(
base::BindOnce(&HidDeviceManager::OnEnumerationComplete,
weak_factory_.GetWeakPtr()),
Expand Down
4 changes: 2 additions & 2 deletions extensions/browser/api/hid/hid_device_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_event_histogram_value.h"
#include "extensions/common/api/hid.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/hid.mojom.h"
Expand Down Expand Up @@ -125,7 +125,7 @@ class HidDeviceManager : public BrowserContextKeyedAPI,
EventRouter* event_router_ = nullptr;
bool initialized_ = false;
mojo::Remote<device::mojom::HidManager> hid_manager_;
mojo::AssociatedBinding<device::mojom::HidManagerClient> binding_;
mojo::AssociatedReceiver<device::mojom::HidManagerClient> receiver_{this};
bool enumeration_ready_ = false;
std::vector<std::unique_ptr<GetApiDevicesParams>> pending_enumerations_;
int next_resource_id_ = 0;
Expand Down
25 changes: 10 additions & 15 deletions services/device/hid/hid_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/callback_helpers.h"
#include "base/lazy_instance.h"
#include "base/stl_util.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "services/device/hid/hid_connection_impl.h"

Expand Down Expand Up @@ -50,23 +51,21 @@ void HidManagerImpl::GetDevicesAndSetClient(
}

void HidManagerImpl::GetDevices(GetDevicesCallback callback) {
hid_service_->GetDevices(base::BindOnce(&HidManagerImpl::CreateDeviceList,
weak_factory_.GetWeakPtr(),
std::move(callback), nullptr));
hid_service_->GetDevices(base::BindOnce(
&HidManagerImpl::CreateDeviceList, weak_factory_.GetWeakPtr(),
std::move(callback), mojo::NullAssociatedRemote()));
}

void HidManagerImpl::CreateDeviceList(
GetDevicesCallback callback,
mojom::HidManagerClientAssociatedPtrInfo client,
mojo::PendingAssociatedRemote<mojom::HidManagerClient> client,
std::vector<mojom::HidDeviceInfoPtr> devices) {
std::move(callback).Run(std::move(devices));

if (!client.is_valid())
return;

mojom::HidManagerClientAssociatedPtr client_ptr;
client_ptr.Bind(std::move(client));
clients_.AddPtr(std::move(client_ptr));
clients_.Add(std::move(client));
}

void HidManagerImpl::Connect(
Expand Down Expand Up @@ -98,17 +97,13 @@ void HidManagerImpl::CreateConnection(
}

void HidManagerImpl::OnDeviceAdded(mojom::HidDeviceInfoPtr device) {
mojom::HidDeviceInfo* device_info = device.get();
clients_.ForAllPtrs([device_info](mojom::HidManagerClient* client) {
client->DeviceAdded(device_info->Clone());
});
for (auto& client : clients_)
client->DeviceAdded(device->Clone());
}

void HidManagerImpl::OnDeviceRemoved(mojom::HidDeviceInfoPtr device) {
mojom::HidDeviceInfo* device_info = device.get();
clients_.ForAllPtrs([device_info](mojom::HidManagerClient* client) {
client->DeviceRemoved(device_info->Clone());
});
for (auto& client : clients_)
client->DeviceRemoved(device->Clone());
}

} // namespace device
12 changes: 7 additions & 5 deletions services/device/hid/hid_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observer.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote_set.h"
#include "services/device/hid/hid_device_info.h"
#include "services/device/hid/hid_service.h"
#include "services/device/public/mojom/hid.mojom.h"
Expand Down Expand Up @@ -43,9 +44,10 @@ class HidManagerImpl : public mojom::HidManager, public HidService::Observer {
ConnectCallback callback) override;

private:
void CreateDeviceList(GetDevicesCallback callback,
mojom::HidManagerClientAssociatedPtrInfo client,
std::vector<mojom::HidDeviceInfoPtr> devices);
void CreateDeviceList(
GetDevicesCallback callback,
mojo::PendingAssociatedRemote<mojom::HidManagerClient> client,
std::vector<mojom::HidDeviceInfoPtr> devices);

void CreateConnection(
ConnectCallback callback,
Expand All @@ -58,7 +60,7 @@ class HidManagerImpl : public mojom::HidManager, public HidService::Observer {

std::unique_ptr<HidService> hid_service_;
mojo::ReceiverSet<mojom::HidManager> receivers_;
mojo::AssociatedInterfacePtrSet<mojom::HidManagerClient> clients_;
mojo::AssociatedRemoteSet<mojom::HidManagerClient> clients_;
ScopedObserver<HidService, HidService::Observer> hid_service_observer_;

base::WeakPtrFactory<HidManagerImpl> weak_factory_{this};
Expand Down
20 changes: 11 additions & 9 deletions services/device/hid/hid_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
#include "base/memory/ref_counted.h"
#include "base/run_loop.h"
#include "build/build_config.h"
#include "mojo/public/cpp/bindings/associated_binding.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/device_service_test_base.h"
#include "services/device/hid/hid_manager_impl.h"
Expand All @@ -29,10 +30,11 @@ const char* kTestDeviceIds[] = {"A", "B"};

class MockHidManagerClient : public mojom::HidManagerClient {
public:
MockHidManagerClient() : binding_(this) {}
MockHidManagerClient() = default;
~MockHidManagerClient() override = default;

void Bind(mojom::HidManagerClientAssociatedRequest request) {
binding_.Bind(std::move(request));
void Bind(mojo::PendingAssociatedReceiver<mojom::HidManagerClient> receiver) {
receiver_.Bind(std::move(receiver));
}

void DeviceAdded(mojom::HidDeviceInfoPtr device_info) override {
Expand All @@ -58,7 +60,7 @@ class MockHidManagerClient : public mojom::HidManagerClient {
void SetExpectGUID(std::string guid) { expect_guid_ = guid; }

private:
mojo::AssociatedBinding<mojom::HidManagerClient> binding_;
mojo::AssociatedReceiver<mojom::HidManagerClient> receiver_{this};
mojom::HidConnectionPtr hid_connection_;
base::OnceClosure quit_closure_;
std::string expect_guid_;
Expand Down Expand Up @@ -181,8 +183,8 @@ TEST_F(HidManagerTest, GetDevicesAndSetClient) {
mock_hid_service_->FirstEnumerationComplete();

auto client = std::make_unique<MockHidManagerClient>();
mojom::HidManagerClientAssociatedPtrInfo hid_manager_client;
client->Bind(mojo::MakeRequest(&hid_manager_client));
mojo::PendingAssociatedRemote<mojom::HidManagerClient> hid_manager_client;
client->Bind(hid_manager_client.InitWithNewEndpointAndPassReceiver());

// Call GetDevicesAndSetClient, expect 1 device will be received in
// OnGetDevices().
Expand Down Expand Up @@ -230,8 +232,8 @@ TEST_F(HidManagerTest, TestHidConnectionInterface) {
mock_hid_service_->FirstEnumerationComplete();

auto client = std::make_unique<MockHidManagerClient>();
mojom::HidManagerClientAssociatedPtrInfo hid_manager_client;
client->Bind(mojo::MakeRequest(&hid_manager_client));
mojo::PendingAssociatedRemote<mojom::HidManagerClient> hid_manager_client;
client->Bind(hid_manager_client.InitWithNewEndpointAndPassReceiver());

// Call GetDevicesAndSetClient, expect 1 device will be received in
// OnGetDevices().
Expand Down
Loading

0 comments on commit 657f4b1

Please sign in to comment.