Commit 90a173bc authored by Alexander Hendrich's avatar Alexander Hendrich Committed by Commit Bot

Revert "lacros: gnubby support for LaCrOS (hid_manager)"

This reverts commit 95cff9e7.

Reason for revert: failing tests, see https://crbug.com/1109621#c7

Original change's description:
> lacros: gnubby support for LaCrOS (hid_manager)
> 
> Because Gnubby and power gnubby in chromium would be considered as
> HID devices, and this CL would enable HID devices support for LaCrOS.
> 
> Before this CL: Lacros would try to access /dev/hidraw*, but the
> permission of it is root:hidraw 660, and lacros is running in chronos
> user permission. By the way, CrOS it is using dbus to access the
> permission_broker, and permission_broker would access these files for CrOS.
> 
> In this CL: Open a HidManager mojo between ash-chrome service and lacros service.
> Lacros would use AddReceiver to add a receiver to ash-chrome's HidManager.
> And the HidManager on ash-chrome would use permission_broker to access
> /dev/hidraw* files. In this solution, we wouldn't need to compile the
> permission_broker into lacros.
> 
> BUG=chromium:1109621
> TEST=tested gnubby manually on soraka
> 
> Change-Id: I18cfe9b5e93e260b9347df4b57ebe12a621f44a8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2319089
> Commit-Queue: joe Chou <yich@google.com>
> Reviewed-by: Greg Kerr <kerrnel@chromium.org>
> Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#800003}

TBR=sky@chromium.org,jorgelo@chromium.org,reillyg@chromium.org,erikchen@chromium.org,kerrnel@chromium.org,yich@google.com

Change-Id: I7cd325c2704a0b4cd18522acc977f3b45159b313
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1109621
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366892Reviewed-by: default avatarAlexander Hendrich <hendrich@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800027}
parent 34ca4db6
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#include "chromeos/crosapi/mojom/message_center.mojom.h" #include "chromeos/crosapi/mojom/message_center.mojom.h"
#include "chromeos/crosapi/mojom/screen_manager.mojom.h" #include "chromeos/crosapi/mojom/screen_manager.mojom.h"
#include "chromeos/crosapi/mojom/select_file.mojom.h" #include "chromeos/crosapi/mojom/select_file.mojom.h"
#include "content/public/browser/device_service.h"
namespace crosapi { namespace crosapi {
...@@ -53,9 +52,4 @@ void AshChromeServiceImpl::BindScreenManager( ...@@ -53,9 +52,4 @@ void AshChromeServiceImpl::BindScreenManager(
screen_manager_ash_->BindReceiver(std::move(receiver)); screen_manager_ash_->BindReceiver(std::move(receiver));
} }
void AshChromeServiceImpl::BindHidManager(
mojo::PendingReceiver<device::mojom::HidManager> receiver) {
content::GetDeviceService().BindHidManager(std::move(receiver));
}
} // namespace crosapi } // namespace crosapi
...@@ -35,8 +35,6 @@ class AshChromeServiceImpl : public mojom::AshChromeService { ...@@ -35,8 +35,6 @@ class AshChromeServiceImpl : public mojom::AshChromeService {
mojo::PendingReceiver<mojom::ScreenManager> receiver) override; mojo::PendingReceiver<mojom::ScreenManager> receiver) override;
void BindSelectFile( void BindSelectFile(
mojo::PendingReceiver<mojom::SelectFile> receiver) override; mojo::PendingReceiver<mojom::SelectFile> receiver) override;
void BindHidManager(
mojo::PendingReceiver<device::mojom::HidManager> receiver) override;
private: private:
mojo::Receiver<mojom::AshChromeService> receiver_; mojo::Receiver<mojom::AshChromeService> receiver_;
......
...@@ -14,7 +14,6 @@ mojom("mojom") { ...@@ -14,7 +14,6 @@ mojom("mojom") {
"screen_manager.mojom", "screen_manager.mojom",
"select_file.mojom", "select_file.mojom",
] ]
disable_variants = true
cpp_typemaps = [ cpp_typemaps = [
{ {
...@@ -35,7 +34,6 @@ mojom("mojom") { ...@@ -35,7 +34,6 @@ mojom("mojom") {
public_deps = [ public_deps = [
"//mojo/public/mojom/base", "//mojo/public/mojom/base",
"//services/device/public/mojom:mojom",
"//url/mojom:url_mojom_gurl", "//url/mojom:url_mojom_gurl",
] ]
} }
......
...@@ -2,16 +2,12 @@ ...@@ -2,16 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// TODO(crbug.com/1110184): We have to mark this mojom and its dependencies as
// [Stable] before lacros begins the process of launching.
module crosapi.mojom; module crosapi.mojom;
import "chromeos/crosapi/mojom/attestation.mojom"; import "chromeos/crosapi/mojom/attestation.mojom";
import "chromeos/crosapi/mojom/message_center.mojom"; import "chromeos/crosapi/mojom/message_center.mojom";
import "chromeos/crosapi/mojom/screen_manager.mojom"; import "chromeos/crosapi/mojom/screen_manager.mojom";
import "chromeos/crosapi/mojom/select_file.mojom"; import "chromeos/crosapi/mojom/select_file.mojom";
import "services/device/public/mojom/hid.mojom";
// AshChromeService defines the APIs that live in ash-chrome and are // AshChromeService defines the APIs that live in ash-chrome and are
// accessed from lacros-chrome. // accessed from lacros-chrome.
...@@ -28,9 +24,6 @@ interface AshChromeService { ...@@ -28,9 +24,6 @@ interface AshChromeService {
// Binds the SelectFile interface for open/save dialogs. // Binds the SelectFile interface for open/save dialogs.
BindSelectFile@0(pending_receiver<SelectFile> receiver); BindSelectFile@0(pending_receiver<SelectFile> receiver);
// Binds the HidManager interface for support HID devices.
BindHidManager@4(pending_receiver<device.mojom.HidManager> receiver);
}; };
// LacrosChromeService defines the APIs that live in lacros-chrome and // LacrosChromeService defines the APIs that live in lacros-chrome and
......
...@@ -89,12 +89,6 @@ class LacrosChromeServiceNeverBlockingState ...@@ -89,12 +89,6 @@ class LacrosChromeServiceNeverBlockingState
ash_chrome_service_->BindSelectFile(std::move(pending_receiver)); ash_chrome_service_->BindSelectFile(std::move(pending_receiver));
} }
void BindHidManagerReceiver(
mojo::PendingReceiver<device::mojom::HidManager> pending_receiver) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ash_chrome_service_->BindHidManager(std::move(pending_receiver));
}
void BindScreenManagerReceiver( void BindScreenManagerReceiver(
mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver) { mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
...@@ -194,15 +188,6 @@ LacrosChromeServiceImpl::LacrosChromeServiceImpl( ...@@ -194,15 +188,6 @@ LacrosChromeServiceImpl::LacrosChromeServiceImpl(
&LacrosChromeServiceNeverBlockingState::BindAttestationReceiver, &LacrosChromeServiceNeverBlockingState::BindAttestationReceiver,
weak_sequenced_state_, std::move(attestation_pending_receiver))); weak_sequenced_state_, std::move(attestation_pending_receiver)));
mojo::PendingReceiver<device::mojom::HidManager>
hid_manager_pending_receiver =
hid_manager_remote_.BindNewPipeAndPassReceiver();
never_blocking_sequence_->PostTask(
FROM_HERE,
base::BindOnce(
&LacrosChromeServiceNeverBlockingState::BindHidManagerReceiver,
weak_sequenced_state_, std::move(hid_manager_pending_receiver)));
DCHECK(!g_instance); DCHECK(!g_instance);
g_instance = this; g_instance = this;
} }
......
...@@ -20,7 +20,6 @@ ...@@ -20,7 +20,6 @@
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/hid.mojom.h"
namespace chromeos { namespace chromeos {
...@@ -86,13 +85,6 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl { ...@@ -86,13 +85,6 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
return attestation_remote_; return attestation_remote_;
} }
// This must be called on the affine sequence. It exposes a remote that can
// be used to support HID devices.
mojo::Remote<device::mojom::HidManager>& hid_manager_remote() {
DCHECK_CALLED_ON_VALID_SEQUENCE(affine_sequence_checker_);
return hid_manager_remote_;
}
// This may be called on any thread. // This may be called on any thread.
void BindScreenManagerReceiver( void BindScreenManagerReceiver(
mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver); mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver);
...@@ -114,7 +106,6 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl { ...@@ -114,7 +106,6 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
// constructor and it is immediately available for use. // constructor and it is immediately available for use.
mojo::Remote<crosapi::mojom::MessageCenter> message_center_remote_; mojo::Remote<crosapi::mojom::MessageCenter> message_center_remote_;
mojo::Remote<crosapi::mojom::SelectFile> select_file_remote_; mojo::Remote<crosapi::mojom::SelectFile> select_file_remote_;
mojo::Remote<device::mojom::HidManager> hid_manager_remote_;
// This member allows lacros-chrome to use the Attestation interface. This // This member allows lacros-chrome to use the Attestation interface. This
// member is affine to the affine sequence. It is initialized in the // member is affine to the affine sequence. It is initialized in the
......
...@@ -111,8 +111,7 @@ class FakeFidoHidManager : public device::mojom::HidManager { ...@@ -111,8 +111,7 @@ class FakeFidoHidManager : public device::mojom::HidManager {
mojo::PendingRemote<mojom::HidConnectionClient> connection_client, mojo::PendingRemote<mojom::HidConnectionClient> connection_client,
mojo::PendingRemote<mojom::HidConnectionWatcher> watcher, mojo::PendingRemote<mojom::HidConnectionWatcher> watcher,
ConnectCallback callback) override; ConnectCallback callback) override;
void AddReceiver( void AddReceiver(mojo::PendingReceiver<device::mojom::HidManager> receiver);
mojo::PendingReceiver<device::mojom::HidManager> receiver) override;
void AddDevice(device::mojom::HidDeviceInfoPtr device); void AddDevice(device::mojom::HidDeviceInfoPtr device);
void AddDeviceAndSetConnection( void AddDeviceAndSetConnection(
device::mojom::HidDeviceInfoPtr device, device::mojom::HidDeviceInfoPtr device,
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
# found in the LICENSE file. # found in the LICENSE file.
import("//build/config/features.gni") import("//build/config/features.gni")
import("//testing/test.gni")
if (is_android) { if (is_android) {
import("//build/config/android/rules.gni") import("//build/config/android/rules.gni")
...@@ -30,7 +29,6 @@ source_set("lib") { ...@@ -30,7 +29,6 @@ source_set("lib") {
deps = [ deps = [
":binder_overrides", ":binder_overrides",
"//build:lacros_buildflags",
"//services/device/fingerprint", "//services/device/fingerprint",
"//services/device/generic_sensor", "//services/device/generic_sensor",
"//services/device/geolocation", "//services/device/geolocation",
...@@ -73,13 +71,6 @@ source_set("lib") { ...@@ -73,13 +71,6 @@ source_set("lib") {
if (is_serial_enabled_platform) { if (is_serial_enabled_platform) {
deps += [ "//services/device/serial" ] deps += [ "//services/device/serial" ]
} }
if (chromeos_is_browser_only) {
deps += [
"//chromeos/crosapi/mojom",
"//chromeos/lacros",
]
}
} }
# NOTE: We use a separate component target to support global binder overrides, # NOTE: We use a separate component target to support global binder overrides,
......
include_rules = [ include_rules = [
"+chromeos/crosapi",
"+chromeos/lacros",
"+device", "+device",
"+services/device/usb/jni_headers", "+services/device/usb/jni_headers",
"+services/network/public/cpp", "+services/network/public/cpp",
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "build/lacros_buildflags.h"
#include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/system/message_pipe.h" #include "mojo/public/cpp/system/message_pipe.h"
#include "services/device/binder_overrides.h" #include "services/device/binder_overrides.h"
...@@ -48,39 +47,6 @@ ...@@ -48,39 +47,6 @@
#include "services/device/hid/input_service_linux.h" #include "services/device/hid/input_service_linux.h"
#endif #endif
#if BUILDFLAG(IS_LACROS)
#include "chromeos/lacros/lacros_chrome_service_impl.h"
#endif
namespace {
#if !defined(OS_ANDROID)
constexpr bool IsLaCrOS() {
#if BUILDFLAG(IS_LACROS)
return true;
#else
return false;
#endif
}
#endif
#if !defined(OS_ANDROID)
void BindLaCrOSHidManager(
mojo::PendingReceiver<device::mojom::HidManager> receiver) {
#if BUILDFLAG(IS_LACROS)
// LaCrOS does not have direct access to the permission_broker service over
// D-Bus. Use the HidManager interface from ash-chrome instead.
auto* lacros_chrome_service = chromeos::LacrosChromeServiceImpl::Get();
DCHECK(lacros_chrome_service);
lacros_chrome_service->hid_manager_remote()->AddReceiver(std::move(receiver));
#else
return;
#endif
}
#endif
} // namespace
namespace device { namespace device {
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
...@@ -244,13 +210,9 @@ void DeviceService::BindVibrationManager( ...@@ -244,13 +210,9 @@ void DeviceService::BindVibrationManager(
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
void DeviceService::BindHidManager( void DeviceService::BindHidManager(
mojo::PendingReceiver<mojom::HidManager> receiver) { mojo::PendingReceiver<mojom::HidManager> receiver) {
if (IsLaCrOS()) { if (!hid_manager_)
BindLaCrOSHidManager(std::move(receiver)); hid_manager_ = std::make_unique<HidManagerImpl>();
} else { hid_manager_->AddReceiver(std::move(receiver));
if (!hid_manager_)
hid_manager_ = std::make_unique<HidManagerImpl>();
hid_manager_->AddReceiver(std::move(receiver));
}
} }
#endif #endif
...@@ -351,9 +313,6 @@ void DeviceService::BindSerialPortManager( ...@@ -351,9 +313,6 @@ void DeviceService::BindSerialPortManager(
mojo::PendingReceiver<mojom::SerialPortManager> receiver) { mojo::PendingReceiver<mojom::SerialPortManager> receiver) {
#if ((defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(USE_UDEV)) || \ #if ((defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(USE_UDEV)) || \
defined(OS_WIN) || defined(OS_MAC) defined(OS_WIN) || defined(OS_MAC)
// TODO(crbug.com/1109621): SerialPortManagerImpl depends on the
// permission_broker service on Chromium OS. We will need to redirect
// connections for LaCrOS here.
DCHECK(serial_port_manager_task_runner_); DCHECK(serial_port_manager_task_runner_);
serial_port_manager_task_runner_->PostTask( serial_port_manager_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&SerialPortManagerImpl::Bind, FROM_HERE, base::BindOnce(&SerialPortManagerImpl::Bind,
...@@ -378,9 +337,6 @@ void DeviceService::BindWakeLockProvider( ...@@ -378,9 +337,6 @@ void DeviceService::BindWakeLockProvider(
void DeviceService::BindUsbDeviceManager( void DeviceService::BindUsbDeviceManager(
mojo::PendingReceiver<mojom::UsbDeviceManager> receiver) { mojo::PendingReceiver<mojom::UsbDeviceManager> receiver) {
// TODO(crbug.com/1109621): usb::DeviceManagerImpl depends on the
// permission_broker service on Chromium OS. We will need to redirect
// connections for LaCrOS here.
if (!usb_device_manager_) if (!usb_device_manager_)
usb_device_manager_ = std::make_unique<usb::DeviceManagerImpl>(); usb_device_manager_ = std::make_unique<usb::DeviceManagerImpl>();
...@@ -389,9 +345,6 @@ void DeviceService::BindUsbDeviceManager( ...@@ -389,9 +345,6 @@ void DeviceService::BindUsbDeviceManager(
void DeviceService::BindUsbDeviceManagerTest( void DeviceService::BindUsbDeviceManagerTest(
mojo::PendingReceiver<mojom::UsbDeviceManagerTest> receiver) { mojo::PendingReceiver<mojom::UsbDeviceManagerTest> receiver) {
// TODO(crbug.com/1109621): usb::DeviceManagerImpl depends on the
// permission_broker service on Chromium OS. We will need to redirect
// connections for LaCrOS here.
if (!usb_device_manager_) if (!usb_device_manager_)
usb_device_manager_ = std::make_unique<usb::DeviceManagerImpl>(); usb_device_manager_ = std::make_unique<usb::DeviceManagerImpl>();
......
...@@ -32,7 +32,7 @@ class HidManagerImpl : public mojom::HidManager, public HidService::Observer { ...@@ -32,7 +32,7 @@ class HidManagerImpl : public mojom::HidManager, public HidService::Observer {
// passed |hid_service|. // passed |hid_service|.
static void SetHidServiceForTesting(std::unique_ptr<HidService> hid_service); static void SetHidServiceForTesting(std::unique_ptr<HidService> hid_service);
void AddReceiver(mojo::PendingReceiver<mojom::HidManager> receiver) override; void AddReceiver(mojo::PendingReceiver<mojom::HidManager> receiver);
// mojom::HidManager implementation: // mojom::HidManager implementation:
void GetDevicesAndSetClient( void GetDevicesAndSetClient(
......
...@@ -100,11 +100,6 @@ void FakeHidManager::Bind(mojo::PendingReceiver<mojom::HidManager> receiver) { ...@@ -100,11 +100,6 @@ void FakeHidManager::Bind(mojo::PendingReceiver<mojom::HidManager> receiver) {
} }
// mojom::HidManager implementation: // mojom::HidManager implementation:
void FakeHidManager::AddReceiver(
mojo::PendingReceiver<mojom::HidManager> receiver) {
Bind(std::move(receiver));
}
void FakeHidManager::GetDevicesAndSetClient( void FakeHidManager::GetDevicesAndSetClient(
mojo::PendingAssociatedRemote<mojom::HidManagerClient> client, mojo::PendingAssociatedRemote<mojom::HidManagerClient> client,
GetDevicesCallback callback) { GetDevicesCallback callback) {
......
...@@ -50,7 +50,6 @@ class FakeHidManager : public mojom::HidManager { ...@@ -50,7 +50,6 @@ class FakeHidManager : public mojom::HidManager {
void Bind(mojo::PendingReceiver<mojom::HidManager> receiver); void Bind(mojo::PendingReceiver<mojom::HidManager> receiver);
// mojom::HidManager implementation: // mojom::HidManager implementation:
void AddReceiver(mojo::PendingReceiver<mojom::HidManager> receiver) override;
void GetDevicesAndSetClient( void GetDevicesAndSetClient(
mojo::PendingAssociatedRemote<mojom::HidManagerClient> client, mojo::PendingAssociatedRemote<mojom::HidManagerClient> client,
GetDevicesCallback callback) override; GetDevicesCallback callback) override;
......
...@@ -2,9 +2,6 @@ ...@@ -2,9 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// TODO(crbug.com/1110184): We have to mark this mojom as [Stable] before
// lacros begins the process of launching.
module device.mojom; module device.mojom;
enum HidBusType { enum HidBusType {
...@@ -375,9 +372,6 @@ interface HidManager { ...@@ -375,9 +372,6 @@ interface HidManager {
pending_remote<HidConnectionClient>? connection_client, pending_remote<HidConnectionClient>? connection_client,
pending_remote<HidConnectionWatcher>? watcher) pending_remote<HidConnectionWatcher>? watcher)
=> (pending_remote<HidConnection>? connection); => (pending_remote<HidConnection>? connection);
// Binds a HidManager endpoint.
AddReceiver@3(pending_receiver<HidManager> receiver);
}; };
// Provides an interface for communication with a HID device. The HID spec // Provides an interface for communication with a HID device. The HID spec
......
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