Skip to content

Commit

Permalink
Convert HidManager to new Mojo types
Browse files Browse the repository at this point in the history
This CL converts HidManager{Ptr, Request} in device, services,
chrome and extensions to the new Mojo type.

Bug: 955171
Change-Id: Iaeb03869b1ed99a769bc88553167c7b265aae32a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804942
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#697041}
  • Loading branch information
MyidShin authored and Commit Bot committed Sep 17, 2019
1 parent 38d6d92 commit 5aa2275
Show file tree
Hide file tree
Showing 20 changed files with 75 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ class DevicePermissionsManagerTest : public testing::Test {
device3_ = fake_usb_manager_.CreateAndAddDevice(0, 0, "Test Manufacturer",
"Test Product", "");

device::mojom::HidManagerPtr hid_manager_ptr;
fake_hid_manager_.Bind(mojo::MakeRequest(&hid_manager_ptr));
mojo::PendingRemote<device::mojom::HidManager> hid_manager;
fake_hid_manager_.Bind(hid_manager.InitWithNewPipeAndPassReceiver());
HidDeviceManager::Get(env_->profile())
->SetFakeHidManagerForTesting(std::move(hid_manager_ptr));
->SetFakeHidManagerForTesting(std::move(hid_manager));
base::RunLoop().RunUntilIdle();

device4_ = fake_hid_manager_.CreateAndAddDevice(
Expand Down
14 changes: 7 additions & 7 deletions chrome/browser/hid/hid_chooser_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ device::mojom::HidManager* HidChooserContext::GetHidManager() {
}

void HidChooserContext::SetHidManagerForTesting(
device::mojom::HidManagerPtr manager) {
mojo::PendingRemote<device::mojom::HidManager> manager) {
SetUpHidManagerConnection(std::move(manager));
}

Expand All @@ -180,16 +180,16 @@ void HidChooserContext::EnsureHidManagerConnection() {
if (hid_manager_)
return;

device::mojom::HidManagerPtr manager;
content::GetSystemConnector()->BindInterface(device::mojom::kServiceName,
mojo::MakeRequest(&manager));
mojo::PendingRemote<device::mojom::HidManager> manager;
content::GetSystemConnector()->Connect(
device::mojom::kServiceName, manager.InitWithNewPipeAndPassReceiver());
SetUpHidManagerConnection(std::move(manager));
}

void HidChooserContext::SetUpHidManagerConnection(
device::mojom::HidManagerPtr manager) {
hid_manager_ = std::move(manager);
hid_manager_.set_connection_error_handler(base::BindOnce(
mojo::PendingRemote<device::mojom::HidManager> manager) {
hid_manager_.Bind(std::move(manager));
hid_manager_.set_disconnect_handler(base::BindOnce(
&HidChooserContext::OnHidManagerConnectionError, base::Unretained(this)));
// TODO(mattreynolds): Register a HidManagerClient to be notified when devices
// are disconnected so that ephemeral permissions can be revoked.
Expand Down
10 changes: 7 additions & 3 deletions chrome/browser/hid/hid_chooser_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "base/memory/weak_ptr.h"
#include "base/unguessable_token.h"
#include "chrome/browser/permissions/chooser_context_base.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/hid.mojom.h"
#include "url/origin.h"

Expand Down Expand Up @@ -55,12 +57,14 @@ class HidChooserContext : public ChooserContextBase {

device::mojom::HidManager* GetHidManager();

void SetHidManagerForTesting(device::mojom::HidManagerPtr manager);
void SetHidManagerForTesting(
mojo::PendingRemote<device::mojom::HidManager> manager);
base::WeakPtr<HidChooserContext> AsWeakPtr();

private:
void EnsureHidManagerConnection();
void SetUpHidManagerConnection(device::mojom::HidManagerPtr manager);
void SetUpHidManagerConnection(
mojo::PendingRemote<device::mojom::HidManager> manager);
void OnHidManagerConnectionError();

const bool is_incognito_;
Expand All @@ -74,7 +78,7 @@ class HidChooserContext : public ChooserContextBase {
// Holds information about devices in |ephemeral_devices_|.
std::map<std::string, base::Value> device_info_;

device::mojom::HidManagerPtr hid_manager_;
mojo::Remote<device::mojom::HidManager> hid_manager_;

base::WeakPtrFactory<HidChooserContext> weak_factory_{this};

Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/ui/hid/hid_chooser_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ class HidChooserControllerTest : public ChromeRenderViewHostTestHarness {
web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl));

// Set fake HID manager for HidChooserContext.
device::mojom::HidManagerPtr hid_manager_ptr;
hid_manager_.Bind(mojo::MakeRequest(&hid_manager_ptr));
mojo::PendingRemote<device::mojom::HidManager> hid_manager;
hid_manager_.Bind(hid_manager.InitWithNewPipeAndPassReceiver());
HidChooserContextFactory::GetForProfile(profile())->SetHidManagerForTesting(
std::move(hid_manager_ptr));
std::move(hid_manager));
}

std::unique_ptr<HidChooserController> CreateHidChooserController(
Expand Down
6 changes: 3 additions & 3 deletions content/browser/webauth/authenticator_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class AuthenticatorImplTest : public AuthenticatorTestBase {
connector_->OverrideBinderForTesting(
service_manager::ServiceFilter::ByName(device::mojom::kServiceName),
device::mojom::HidManager::Name_,
base::Bind(&device::FakeFidoHidManager::AddBinding,
base::Bind(&device::FakeFidoHidManager::AddReceiver,
base::Unretained(fake_hid_manager_.get())));

// Set up a timer for testing.
Expand Down Expand Up @@ -2572,7 +2572,7 @@ class AuthenticatorImplRequestDelegateTest : public AuthenticatorImplTest {
connector_->OverrideBinderForTesting(
service_manager::ServiceFilter::ByName(device::mojom::kServiceName),
device::mojom::HidManager::Name_,
base::Bind(&device::FakeFidoHidManager::AddBinding,
base::Bind(&device::FakeFidoHidManager::AddReceiver,
base::Unretained(fake_hid_manager_.get())));

// Set up a timer for testing.
Expand Down Expand Up @@ -4195,7 +4195,7 @@ class InternalAuthenticatorImplTest : public AuthenticatorTestBase {
connector_->OverrideBinderForTesting(
service_manager::ServiceFilter::ByName(device::mojom::kServiceName),
device::mojom::HidManager::Name_,
base::BindRepeating(&device::FakeFidoHidManager::AddBinding,
base::BindRepeating(&device::FakeFidoHidManager::AddReceiver,
base::Unretained(fake_hid_manager_.get())));

// Set up a timer for testing.
Expand Down
13 changes: 7 additions & 6 deletions device/fido/hid/fake_hid_impl_for_testing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,14 @@ FakeFidoHidManager::FakeFidoHidManager() = default;

FakeFidoHidManager::~FakeFidoHidManager() = default;

void FakeFidoHidManager::AddBinding(mojo::ScopedMessagePipeHandle handle) {
bindings_.AddBinding(this,
device::mojom::HidManagerRequest(std::move(handle)));
void FakeFidoHidManager::AddReceiver(mojo::ScopedMessagePipeHandle handle) {
receivers_.Add(this, mojo::PendingReceiver<device::mojom::HidManager>(
std::move(handle)));
}

void FakeFidoHidManager::AddBinding2(device::mojom::HidManagerRequest request) {
bindings_.AddBinding(this, std::move(request));
void FakeFidoHidManager::AddReceiver2(
mojo::PendingReceiver<device::mojom::HidManager> receiver) {
receivers_.Add(this, std::move(receiver));
}

void FakeFidoHidManager::AddFidoHidDevice(std::string guid) {
Expand Down Expand Up @@ -216,7 +217,7 @@ ScopedFakeFidoHidManager::ScopedFakeFidoHidManager() {
connector_->OverrideBinderForTesting(
service_manager::ServiceFilter::ByName(device::mojom::kServiceName),
device::mojom::HidManager::Name_,
base::BindRepeating(&FakeFidoHidManager::AddBinding,
base::BindRepeating(&FakeFidoHidManager::AddReceiver,
base::Unretained(this)));
}

Expand Down
9 changes: 5 additions & 4 deletions device/fido/hid/fake_hid_impl_for_testing.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "device/fido/fido_constants.h"
#include "mojo/public/cpp/bindings/binding_set.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/strong_binding.h"
#include "services/device/public/mojom/hid.mojom.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -111,8 +112,8 @@ class FakeFidoHidManager : public device::mojom::HidManager {
const std::string& device_guid,
mojo::PendingRemote<mojom::HidConnectionClient> connection_client,
ConnectCallback callback) override;
void AddBinding(mojo::ScopedMessagePipeHandle handle);
void AddBinding2(device::mojom::HidManagerRequest request);
void AddReceiver(mojo::ScopedMessagePipeHandle handle);
void AddReceiver2(mojo::PendingReceiver<device::mojom::HidManager> receiver);
void AddDevice(device::mojom::HidDeviceInfoPtr device);
void AddDeviceAndSetConnection(device::mojom::HidDeviceInfoPtr device,
device::mojom::HidConnectionPtr connection);
Expand All @@ -122,7 +123,7 @@ class FakeFidoHidManager : public device::mojom::HidManager {
std::map<std::string, device::mojom::HidDeviceInfoPtr> devices_;
std::map<std::string, device::mojom::HidConnectionPtr> connections_;
mojo::AssociatedInterfacePtrSet<device::mojom::HidManagerClient> clients_;
mojo::BindingSet<device::mojom::HidManager> bindings_;
mojo::ReceiverSet<device::mojom::HidManager> receivers_;

DISALLOW_COPY_AND_ASSIGN(FakeFidoHidManager);
};
Expand Down
6 changes: 3 additions & 3 deletions device/fido/hid/fido_hid_device_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
#include "device/fido/hid/fake_hid_impl_for_testing.h"
#include "device/fido/hid/fido_hid_message.h"
#include "device/fido/test_callback_receiver.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/interface_request.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"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -223,13 +223,13 @@ class FidoHidDeviceTest : public ::testing::Test {
public:
void SetUp() override {
fake_hid_manager_ = std::make_unique<FakeFidoHidManager>();
fake_hid_manager_->AddBinding2(mojo::MakeRequest(&hid_manager_));
fake_hid_manager_->AddReceiver2(hid_manager_.BindNewPipeAndPassReceiver());
}

protected:
base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
device::mojom::HidManagerPtr hid_manager_;
mojo::Remote<device::mojom::HidManager> hid_manager_;
std::unique_ptr<FakeFidoHidManager> fake_hid_manager_;
};

Expand Down
4 changes: 2 additions & 2 deletions device/fido/hid/fido_hid_discovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ FidoHidDiscovery::~FidoHidDiscovery() = default;

void FidoHidDiscovery::StartInternal() {
DCHECK(connector_);
connector_->BindInterface(device::mojom::kServiceName,
mojo::MakeRequest(&hid_manager_));
connector_->Connect(device::mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());
device::mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));

Expand Down
3 changes: 2 additions & 1 deletion device/fido/hid/fido_hid_discovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#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/remote.h"
#include "services/device/public/cpp/hid/hid_device_filter.h"
#include "services/device/public/mojom/hid.mojom.h"

Expand Down Expand Up @@ -43,7 +44,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoHidDiscovery
void OnGetDevices(std::vector<device::mojom::HidDeviceInfoPtr> devices);

service_manager::Connector* connector_;
device::mojom::HidManagerPtr hid_manager_;
mojo::Remote<device::mojom::HidManager> hid_manager_;
mojo::AssociatedBinding<device::mojom::HidManagerClient> binding_;
HidDeviceFilter filter_;
base::WeakPtrFactory<FidoHidDiscovery> weak_factory_{this};
Expand Down
4 changes: 2 additions & 2 deletions device/gamepad/nintendo_data_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ GamepadSource NintendoDataFetcher::source() {
void NintendoDataFetcher::OnAddedToProvider() {
// Open a connection to the HID service. On a successful connection,
// OnGetDevices will be called with a list of connected HID devices.
connector()->BindInterface(mojom::kServiceName,
mojo::MakeRequest(&hid_manager_));
connector()->Connect(mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());
mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));
hid_manager_->GetDevicesAndSetClient(
Expand Down
3 changes: 2 additions & 1 deletion device/gamepad/nintendo_data_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#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/remote.h"
#include "services/device/public/mojom/hid.mojom.h"

namespace device {
Expand Down Expand Up @@ -110,7 +111,7 @@ class DEVICE_GAMEPAD_EXPORT NintendoDataFetcher : public GamepadDataFetcher,
// A mapping from source ID to connected Nintendo Switch devices.
ControllerMap controllers_;

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

Expand Down
7 changes: 4 additions & 3 deletions extensions/browser/api/device_permissions_prompt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#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/remote.h"
#include "services/device/public/cpp/hid/hid_device_filter.h"
#include "services/device/public/cpp/hid/hid_usage_and_page.h"
#include "services/device/public/cpp/usb/usb_utils.h"
Expand Down Expand Up @@ -217,8 +218,8 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
service_manager::Connector* connector = content::GetSystemConnector();
DCHECK(connector);
connector->BindInterface(device::mojom::kServiceName,
mojo::MakeRequest(&hid_manager_));
connector->Connect(device::mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());

device::mojom::HidManagerClientAssociatedPtrInfo client;
binding_.Bind(mojo::MakeRequest(&client));
Expand Down Expand Up @@ -297,7 +298,7 @@ class HidDevicePermissionsPrompt : public DevicePermissionsPrompt::Prompt,

bool initialized_;
std::vector<HidDeviceFilter> filters_;
device::mojom::HidManagerPtr hid_manager_;
mojo::Remote<device::mojom::HidManager> hid_manager_;
DevicePermissionsPrompt::HidDevicesCallback callback_;
mojo::AssociatedBinding<device::mojom::HidManagerClient> binding_;
};
Expand Down
8 changes: 4 additions & 4 deletions extensions/browser/api/hid/hid_device_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,12 @@ void HidDeviceManager::LazyInitialize() {
if (!hid_manager_) {
// |hid_manager_| is initialized and safe to use whether or not the
// connection is successful.
device::mojom::HidManagerRequest request = mojo::MakeRequest(&hid_manager_);

DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
auto* connector = content::GetSystemConnector();
DCHECK(connector);
connector->BindInterface(device::mojom::kServiceName, std::move(request));
connector->Connect(device::mojom::kServiceName,
hid_manager_.BindNewPipeAndPassReceiver());
}
// Enumerate HID devices and set client.
std::vector<device::mojom::HidDeviceInfoPtr> empty_devices;
Expand All @@ -309,10 +309,10 @@ void HidDeviceManager::LazyInitialize() {
}

void HidDeviceManager::SetFakeHidManagerForTesting(
device::mojom::HidManagerPtr fake_hid_manager) {
mojo::PendingRemote<device::mojom::HidManager> fake_hid_manager) {
DCHECK(!hid_manager_);
DCHECK(fake_hid_manager);
hid_manager_ = std::move(fake_hid_manager);
hid_manager_.Bind(std::move(fake_hid_manager));
LazyInitialize();
}
std::unique_ptr<base::ListValue> HidDeviceManager::CreateApiDeviceList(
Expand Down
6 changes: 4 additions & 2 deletions extensions/browser/api/hid/hid_device_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#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/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/hid.mojom.h"

namespace device {
Expand Down Expand Up @@ -79,7 +81,7 @@ class HidDeviceManager : public BrowserContextKeyedAPI,
virtual void LazyInitialize();

void SetFakeHidManagerForTesting(
device::mojom::HidManagerPtr fake_hid_manager);
mojo::PendingRemote<device::mojom::HidManager> fake_hid_manager);

private:
friend class BrowserContextKeyedAPIFactory<HidDeviceManager>;
Expand Down Expand Up @@ -122,7 +124,7 @@ class HidDeviceManager : public BrowserContextKeyedAPI,
content::BrowserContext* browser_context_ = nullptr;
EventRouter* event_router_ = nullptr;
bool initialized_ = false;
device::mojom::HidManagerPtr hid_manager_;
mojo::Remote<device::mojom::HidManager> hid_manager_;
mojo::AssociatedBinding<device::mojom::HidManagerClient> binding_;
bool enumeration_ready_ = false;
std::vector<std::unique_ptr<GetApiDevicesParams>> pending_enumerations_;
Expand Down
7 changes: 4 additions & 3 deletions services/device/device_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ void DeviceService::OnStart() {
registry_.AddInterface<mojom::BatteryMonitor>(base::Bind(
&DeviceService::BindBatteryMonitorReceiver, base::Unretained(this)));
registry_.AddInterface<mojom::HidManager>(base::Bind(
&DeviceService::BindHidManagerRequest, base::Unretained(this)));
&DeviceService::BindHidManagerReceiver, base::Unretained(this)));
registry_.AddInterface<mojom::NFCProvider>(base::Bind(
&DeviceService::BindNFCProviderReceiver, base::Unretained(this)));
registry_.AddInterface<mojom::VibrationManager>(base::Bind(
Expand Down Expand Up @@ -235,10 +235,11 @@ void DeviceService::BindBatteryMonitorReceiver(
BatteryMonitorImpl::Create(std::move(receiver));
}

void DeviceService::BindHidManagerRequest(mojom::HidManagerRequest request) {
void DeviceService::BindHidManagerReceiver(
mojo::PendingReceiver<mojom::HidManager> receiver) {
if (!hid_manager_)
hid_manager_ = std::make_unique<HidManagerImpl>();
hid_manager_->AddBinding(std::move(request));
hid_manager_->AddReceiver(std::move(receiver));
}

void DeviceService::BindNFCProviderReceiver(
Expand Down
3 changes: 2 additions & 1 deletion services/device/device_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ class DeviceService : public service_manager::Service {
#if !defined(OS_ANDROID)
void BindBatteryMonitorReceiver(
mojo::PendingReceiver<mojom::BatteryMonitor> receiver);
void BindHidManagerRequest(mojom::HidManagerRequest request);
void BindHidManagerReceiver(
mojo::PendingReceiver<mojom::HidManager> receiver);
void BindNFCProviderReceiver(
mojo::PendingReceiver<mojom::NFCProvider> receiver);
void BindVibrationManagerRequest(mojom::VibrationManagerRequest request);
Expand Down
5 changes: 3 additions & 2 deletions services/device/hid/hid_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void HidManagerImpl::SetHidServiceForTesting(
g_hid_service.Get() = std::move(hid_service);
}

void HidManagerImpl::AddBinding(mojom::HidManagerRequest request) {
bindings_.AddBinding(this, std::move(request));
void HidManagerImpl::AddReceiver(
mojo::PendingReceiver<mojom::HidManager> receiver) {
receivers_.Add(this, std::move(receiver));
}

void HidManagerImpl::GetDevicesAndSetClient(
Expand Down
Loading

0 comments on commit 5aa2275

Please sign in to comment.