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

This is a reland of 95cff9e7

What was broken in the first CL:
  device_unittests failed because of:
    -NintendoDataFetcherTest.AddAndRemoveSwitchPro
    -NintendoDataFetcherTest.UnsupportedDeviceIsIgnored
  services_unittests failed because of:
    -HidManagerTest.TestHidConnectionInterface
    -HidManagerTest.GetDevicesAndSetClient
    -HidManagerTest.GetDevicesOnly
The unit test failed because the mock_hid_service is set in the
unittest code but we din't bind it in the device service.

In this CL, I add IsHidServiceTesting to check the testing status.
It would bind the mock_hid_service when the HidService instance is
set by SetHidServiceForTesting().

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}

Bug: chromium:1109621
Change-Id: I15e77c7e7ccf8ea26bef6f7c65f36c5cafbf743c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367572Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Auto-Submit: joe Chou <yich@google.com>
Commit-Queue: joe Chou <yich@google.com>
Cr-Commit-Position: refs/heads/master@{#803638}

chrome/browser/chromeos/crosapi/ash_chrome_service_impl.cc

@@ -17,6 +17,7 @@
 #include "chromeos/crosapi/mojom/message_center.mojom.h"
 #include "chromeos/crosapi/mojom/screen_manager.mojom.h"
 #include "chromeos/crosapi/mojom/select_file.mojom.h"
+#include "content/public/browser/device_service.h"
 
 namespace crosapi {
 
@@ -52,4 +53,9 @@ void AshChromeServiceImpl::BindScreenManager(
   screen_manager_ash_->BindReceiver(std::move(receiver));
 }
 
+void AshChromeServiceImpl::BindHidManager(
+    mojo::PendingReceiver<device::mojom::HidManager> receiver) {
+  content::GetDeviceService().BindHidManager(std::move(receiver));
+}
+
 }  // namespace crosapi

chrome/browser/chromeos/crosapi/ash_chrome_service_impl.h

@@ -35,6 +35,8 @@ class AshChromeServiceImpl : public mojom::AshChromeService {
       mojo::PendingReceiver<mojom::ScreenManager> receiver) override;
   void BindSelectFile(
       mojo::PendingReceiver<mojom::SelectFile> receiver) override;
+  void BindHidManager(
+      mojo::PendingReceiver<device::mojom::HidManager> receiver) override;
 
  private:
   mojo::Receiver<mojom::AshChromeService> receiver_;

chromeos/crosapi/mojom/BUILD.gn

@@ -14,6 +14,7 @@ mojom("mojom") {
     "screen_manager.mojom",
     "select_file.mojom",
   ]
+  disable_variants = true
 
   cpp_typemaps = [
     {
@@ -34,6 +35,7 @@ mojom("mojom") {
 
   public_deps = [
     "//mojo/public/mojom/base",
+    "//services/device/public/mojom:mojom",
     "//url/mojom:url_mojom_gurl",
   ]
 }

chromeos/crosapi/mojom/crosapi.mojom

@@ -2,12 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // 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;
 
 import "chromeos/crosapi/mojom/attestation.mojom";
 import "chromeos/crosapi/mojom/message_center.mojom";
 import "chromeos/crosapi/mojom/screen_manager.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
 // accessed from lacros-chrome.
@@ -24,6 +28,9 @@ interface AshChromeService {
 
   // Binds the SelectFile interface for open/save dialogs.
   BindSelectFile@0(pending_receiver<SelectFile> receiver);
+
+  // Binds the HidManager interface for support HID devices.
+  BindHidManager@4(pending_receiver<device.mojom.HidManager> receiver);
 };
 
 // LacrosInitParams is a set of parameters for initialization of lacros-chrome,

chromeos/lacros/lacros_chrome_service_impl.cc

@@ -105,6 +105,12 @@ class LacrosChromeServiceNeverBlockingState
     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(
       mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -212,6 +218,15 @@ LacrosChromeServiceImpl::LacrosChromeServiceImpl(
           &LacrosChromeServiceNeverBlockingState::BindAttestationReceiver,
           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);
   g_instance = this;
 }

chromeos/lacros/lacros_chrome_service_impl.h

@@ -20,6 +20,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "services/device/public/mojom/hid.mojom.h"
 
 namespace chromeos {
 
@@ -85,6 +86,13 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
     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.
   void BindScreenManagerReceiver(
       mojo::PendingReceiver<crosapi::mojom::ScreenManager> pending_receiver);
@@ -113,6 +121,7 @@ class COMPONENT_EXPORT(CHROMEOS_LACROS) LacrosChromeServiceImpl {
   // constructor and it is immediately available for use.
   mojo::Remote<crosapi::mojom::MessageCenter> message_center_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
   // member is affine to the affine sequence. It is initialized in the

device/fido/hid/fake_hid_impl_for_testing.h

@@ -111,7 +111,8 @@ class FakeFidoHidManager : public device::mojom::HidManager {
       mojo::PendingRemote<mojom::HidConnectionClient> connection_client,
       mojo::PendingRemote<mojom::HidConnectionWatcher> watcher,
       ConnectCallback callback) override;
-  void AddReceiver(mojo::PendingReceiver<device::mojom::HidManager> receiver);
+  void AddReceiver(
+      mojo::PendingReceiver<device::mojom::HidManager> receiver) override;
   void AddDevice(device::mojom::HidDeviceInfoPtr device);
   void AddDeviceAndSetConnection(
       device::mojom::HidDeviceInfoPtr device,

services/device/BUILD.gn

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 import("//build/config/features.gni")
+import("//testing/test.gni")
 
 if (is_android) {
   import("//build/config/android/rules.gni")
@@ -29,6 +30,7 @@ source_set("lib") {
 
   deps = [
     ":binder_overrides",
+    "//build:lacros_buildflags",
     "//services/device/fingerprint",
     "//services/device/generic_sensor",
     "//services/device/geolocation",
@@ -71,6 +73,13 @@ source_set("lib") {
   if (is_serial_enabled_platform) {
     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,

services/device/DEPS

 include_rules = [
+  "+chromeos/crosapi",
+  "+chromeos/lacros",
   "+device",
   "+services/device/usb/jni_headers",
   "+services/network/public/cpp",

services/device/device_service.cc

@@ -13,6 +13,7 @@
 #include "base/task/thread_pool.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "build/lacros_buildflags.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/system/message_pipe.h"
 #include "services/device/binder_overrides.h"
@@ -47,6 +48,37 @@
 #include "services/device/hid/input_service_linux.h"
 #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));
+#endif
+}
+#endif
+
+}  // namespace
+
 namespace device {
 
 #if defined(OS_ANDROID)
@@ -210,9 +242,13 @@ void DeviceService::BindVibrationManager(
 #if !defined(OS_ANDROID)
 void DeviceService::BindHidManager(
     mojo::PendingReceiver<mojom::HidManager> receiver) {
-  if (!hid_manager_)
-    hid_manager_ = std::make_unique<HidManagerImpl>();
-  hid_manager_->AddReceiver(std::move(receiver));
+  if (IsLaCrOS() && !HidManagerImpl::IsHidServiceTesting()) {
+    BindLaCrOSHidManager(std::move(receiver));
+  } else {
+    if (!hid_manager_)
+      hid_manager_ = std::make_unique<HidManagerImpl>();
+    hid_manager_->AddReceiver(std::move(receiver));
+  }
 }
 #endif
 
@@ -313,6 +349,9 @@ void DeviceService::BindSerialPortManager(
     mojo::PendingReceiver<mojom::SerialPortManager> receiver) {
 #if ((defined(OS_LINUX) || defined(OS_CHROMEOS)) && defined(USE_UDEV)) || \
     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_);
   serial_port_manager_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(&SerialPortManagerImpl::Bind,
@@ -337,6 +376,9 @@ void DeviceService::BindWakeLockProvider(
 
 void DeviceService::BindUsbDeviceManager(
     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_)
     usb_device_manager_ = std::make_unique<usb::DeviceManagerImpl>();
 
@@ -345,6 +387,9 @@ void DeviceService::BindUsbDeviceManager(
 
 void DeviceService::BindUsbDeviceManagerTest(
     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_)
     usb_device_manager_ = std::make_unique<usb::DeviceManagerImpl>();
 

services/device/hid/hid_manager_impl.cc

@@ -38,6 +38,11 @@ void HidManagerImpl::SetHidServiceForTesting(
   g_hid_service.Get() = std::move(hid_service);
 }
 
+// static
+bool HidManagerImpl::IsHidServiceTesting() {
+  return g_hid_service.IsCreated();
+}
+
 void HidManagerImpl::AddReceiver(
     mojo::PendingReceiver<mojom::HidManager> receiver) {
   receivers_.Add(this, std::move(receiver));

services/device/hid/hid_manager_impl.h

@@ -32,7 +32,12 @@ class HidManagerImpl : public mojom::HidManager, public HidService::Observer {
   // passed |hid_service|.
   static void SetHidServiceForTesting(std::unique_ptr<HidService> hid_service);
 
-  void AddReceiver(mojo::PendingReceiver<mojom::HidManager> receiver);
+  // IsHidServiceTesting() will return true when the next call to the
+  // constructor will use the HidService instance set by
+  // SetHidServiceForTesting().
+  static bool IsHidServiceTesting();
+
+  void AddReceiver(mojo::PendingReceiver<mojom::HidManager> receiver) override;
 
   // mojom::HidManager implementation:
   void GetDevicesAndSetClient(

services/device/public/cpp/hid/fake_hid_manager.cc

@@ -100,6 +100,11 @@ void FakeHidManager::Bind(mojo::PendingReceiver<mojom::HidManager> receiver) {
 }
 
 // mojom::HidManager implementation:
+void FakeHidManager::AddReceiver(
+    mojo::PendingReceiver<mojom::HidManager> receiver) {
+  Bind(std::move(receiver));
+}
+
 void FakeHidManager::GetDevicesAndSetClient(
     mojo::PendingAssociatedRemote<mojom::HidManagerClient> client,
     GetDevicesCallback callback) {

services/device/public/cpp/hid/fake_hid_manager.h

@@ -50,6 +50,7 @@ class FakeHidManager : public mojom::HidManager {
   void Bind(mojo::PendingReceiver<mojom::HidManager> receiver);
 
   // mojom::HidManager implementation:
+  void AddReceiver(mojo::PendingReceiver<mojom::HidManager> receiver) override;
   void GetDevicesAndSetClient(
       mojo::PendingAssociatedRemote<mojom::HidManagerClient> client,
       GetDevicesCallback callback) override;

services/device/public/mojom/hid.mojom

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // 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;
 
 enum HidBusType {
@@ -372,6 +375,9 @@ interface HidManager {
           pending_remote<HidConnectionClient>? connection_client,
           pending_remote<HidConnectionWatcher>? watcher)
       => (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