fido: factor out a FidoDiscoveryFactory class

This moves the static factory methods for instantiating subclasses of
FidoDeviceDiscovery out of FidoDeviceDiscovery and into a new class called
FidoDiscoveryFactory. The ScopedFidoDeviceDiscoveryFactory is moved along with
it.

This will simplify changes that let us instantiate non-device authenticators
(Windows, Touch ID) through the existing factory methods. Also, instantiating
different kinds of FidoDiscovery is becoming complex enough that centralizing
this code in a single place, separate from any of the FidoDiscovery
implementations seems sensible.

Bug: 898718
Change-Id: I6c114c5aa9e8db4538a003108c4e7acec7100fbd
Reviewed-on: https://chromium-review.googlesource.com/c/1311841
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604806}

content/browser/webauth/scoped_virtual_authenticator_environment.cc

@@ -96,7 +96,7 @@ void ScopedVirtualAuthenticatorEnvironment::ClearAuthenticators(
   std::move(callback).Run();
 }
 
-std::unique_ptr<::device::FidoDeviceDiscovery>
+std::unique_ptr<::device::FidoDiscoveryBase>
 ScopedVirtualAuthenticatorEnvironment::CreateFidoDiscovery(
     device::FidoTransportProtocol transport,
     ::service_manager::Connector* connector) {

content/browser/webauth/scoped_virtual_authenticator_environment.h

@@ -13,7 +13,7 @@
 #include "base/macros.h"
 #include "base/no_destructor.h"
 #include "content/common/content_export.h"
-#include "device/fido/fido_device_discovery.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "third_party/blink/public/platform/modules/webauthn/virtual_authenticator.mojom.h"
 
@@ -53,7 +53,7 @@ class CONTENT_EXPORT ScopedVirtualAuthenticatorEnvironment
   void ClearAuthenticators(ClearAuthenticatorsCallback callback) override;
 
   // ScopedFidoDiscoveryFactory:
-  std::unique_ptr<::device::FidoDeviceDiscovery> CreateFidoDiscovery(
+  std::unique_ptr<::device::FidoDiscoveryBase> CreateFidoDiscovery(
       device::FidoTransportProtocol transport,
       ::service_manager::Connector* connector) override;
 

content/browser/webauth/webauth_browsertest.cc

@@ -32,6 +32,7 @@
 #include "content/test/did_commit_provisional_load_interceptor.h"
 #include "device/base/features.h"
 #include "device/fido/fake_fido_discovery.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "device/fido/fido_test_data.h"
 #include "device/fido/hid/fake_hid_impl_for_testing.h"
 #include "device/fido/mock_fido_device.h"
@@ -591,7 +592,7 @@ IN_PROC_BROWSER_TEST_F(WebAuthLocalClientBrowserTest,
   // factory as one of the first steps. Here, the request should not have been
   // serviced at all, so the fake request should still be pending on the fake
   // factory.
-  auto hid_discovery = ::device::FidoDeviceDiscovery::Create(
+  auto hid_discovery = ::device::FidoDiscoveryFactory::Create(
       ::device::FidoTransportProtocol::kUsbHumanInterfaceDevice, nullptr);
   ASSERT_TRUE(!!hid_discovery);
 

content/test/BUILD.gn

@@ -1162,6 +1162,7 @@ test("content_browsertests") {
     sources += [ "../browser/webauth/webauth_browsertest.cc" ]
     deps += [
       "//device/base",
+      "//device/fido:fido",
       "//device/fido:mocks",
       "//device/fido:test_support",
     ]

device/fido/BUILD.gn

@@ -84,6 +84,8 @@ component("fido") {
     "fido_device_discovery.h",
     "fido_discovery_base.cc",
     "fido_discovery_base.h",
+    "fido_discovery_factory.cc",
+    "fido_discovery_factory.h",
     "fido_parsing_utils.cc",
     "fido_parsing_utils.h",
     "fido_request_handler.h",

device/fido/fake_fido_discovery.cc

@@ -80,7 +80,7 @@ FakeFidoDiscovery* ScopedFakeFidoDiscoveryFactory::ForgeNextCableDiscovery(
   return next_cable_discovery_.get();
 }
 
-std::unique_ptr<FidoDeviceDiscovery>
+std::unique_ptr<FidoDiscoveryBase>
 ScopedFakeFidoDiscoveryFactory::CreateFidoDiscovery(
     FidoTransportProtocol transport,
     ::service_manager::Connector* connector) {

device/fido/fake_fido_discovery.h

@@ -11,7 +11,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
-#include "device/fido/fido_device_discovery.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "device/fido/fido_transport_protocol.h"
 
 namespace service_manager {
@@ -117,7 +117,7 @@ class ScopedFakeFidoDiscoveryFactory
       StartMode mode = StartMode::kManual);
 
  protected:
-  std::unique_ptr<FidoDeviceDiscovery> CreateFidoDiscovery(
+  std::unique_ptr<FidoDiscoveryBase> CreateFidoDiscovery(
       FidoTransportProtocol transport,
       ::service_manager::Connector* connector) override;
 

device/fido/fake_fido_discovery_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/test/scoped_task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "device/fido/fido_test_data.h"
 #include "device/fido/mock_fido_device.h"
 #include "device/fido/mock_fido_discovery_observer.h"
@@ -150,7 +151,7 @@ TEST_F(ScopedFakeFidoDiscoveryFactoryTest,
   auto* injected_fake_discovery = factory.ForgeNextHidDiscovery();
   ASSERT_EQ(FidoTransportProtocol::kUsbHumanInterfaceDevice,
             injected_fake_discovery->transport());
-  auto produced_discovery = FidoDeviceDiscovery::Create(
+  auto produced_discovery = FidoDiscoveryFactory::Create(
       FidoTransportProtocol::kUsbHumanInterfaceDevice, nullptr);
   EXPECT_TRUE(produced_discovery);
   EXPECT_EQ(injected_fake_discovery, produced_discovery.get());
@@ -164,14 +165,14 @@ TEST_F(ScopedFakeFidoDiscoveryFactoryTest,
   auto* injected_fake_discovery_1 = factory.ForgeNextBleDiscovery();
   ASSERT_EQ(FidoTransportProtocol::kBluetoothLowEnergy,
             injected_fake_discovery_1->transport());
-  auto produced_discovery_1 = FidoDeviceDiscovery::Create(
+  auto produced_discovery_1 = FidoDiscoveryFactory::Create(
       FidoTransportProtocol::kBluetoothLowEnergy, nullptr);
   EXPECT_EQ(injected_fake_discovery_1, produced_discovery_1.get());
 
   auto* injected_fake_discovery_2 = factory.ForgeNextBleDiscovery();
   ASSERT_EQ(FidoTransportProtocol::kBluetoothLowEnergy,
             injected_fake_discovery_2->transport());
-  auto produced_discovery_2 = FidoDeviceDiscovery::Create(
+  auto produced_discovery_2 = FidoDiscoveryFactory::Create(
       FidoTransportProtocol::kBluetoothLowEnergy, nullptr);
   EXPECT_EQ(injected_fake_discovery_2, produced_discovery_2.get());
 }

device/fido/fido_device_discovery.cc

@@ -8,81 +8,14 @@
 
 #include "base/bind.h"
 #include "base/threading/sequenced_task_runner_handle.h"
-#include "build/build_config.h"
-#include "device/fido/ble/fido_ble_discovery.h"
-#include "device/fido/cable/fido_cable_discovery.h"
 #include "device/fido/fido_authenticator.h"
 #include "device/fido/fido_device.h"
 #include "device/fido/fido_device_authenticator.h"
 
-// HID is not supported on Android.
-#if !defined(OS_ANDROID)
-#include "device/fido/hid/fido_hid_discovery.h"
-#endif  // !defined(OS_ANDROID)
-
 namespace device {
 
-namespace {
-
-std::unique_ptr<FidoDeviceDiscovery> CreateFidoDiscoveryImpl(
-    FidoTransportProtocol transport,
-    service_manager::Connector* connector) {
-  switch (transport) {
-    case FidoTransportProtocol::kUsbHumanInterfaceDevice:
-#if !defined(OS_ANDROID)
-      DCHECK(connector);
-      return std::make_unique<FidoHidDiscovery>(connector);
-#else
-      NOTREACHED() << "USB HID not supported on Android.";
-      return nullptr;
-#endif  // !defined(OS_ANDROID)
-    case FidoTransportProtocol::kBluetoothLowEnergy:
-      return std::make_unique<FidoBleDiscovery>();
-    case FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy:
-      NOTREACHED() << "Cable discovery is constructed using the dedicated "
-                      "factory method.";
-      return nullptr;
-    case FidoTransportProtocol::kNearFieldCommunication:
-      // TODO(https://crbug.com/825949): Add NFC support.
-      return nullptr;
-    case FidoTransportProtocol::kInternal:
-      NOTREACHED() << "Internal authenticators should be handled separately.";
-      return nullptr;
-  }
-  NOTREACHED() << "Unhandled transport type";
-  return nullptr;
-}
-
-std::unique_ptr<FidoDeviceDiscovery> CreateCableDiscoveryImpl(
-    std::vector<CableDiscoveryData> cable_data) {
-  return std::make_unique<FidoCableDiscovery>(std::move(cable_data));
-}
-
-}  // namespace
-
 FidoDeviceDiscovery::Observer::~Observer() = default;
 
-// static
-FidoDeviceDiscovery::FactoryFuncPtr FidoDeviceDiscovery::g_factory_func_ =
-    &CreateFidoDiscoveryImpl;
-
-// static
-FidoDeviceDiscovery::CableFactoryFuncPtr
-    FidoDeviceDiscovery::g_cable_factory_func_ = &CreateCableDiscoveryImpl;
-
-// static
-std::unique_ptr<FidoDeviceDiscovery> FidoDeviceDiscovery::Create(
-    FidoTransportProtocol transport,
-    service_manager::Connector* connector) {
-  return (*g_factory_func_)(transport, connector);
-}
-
-//  static
-std::unique_ptr<FidoDeviceDiscovery> FidoDeviceDiscovery::CreateCable(
-    std::vector<CableDiscoveryData> cable_data) {
-  return (*g_cable_factory_func_)(std::move(cable_data));
-}
-
 FidoDeviceDiscovery::FidoDeviceDiscovery(FidoTransportProtocol transport)
     : FidoDiscoveryBase(transport), weak_factory_(this) {}
 
@@ -177,50 +110,4 @@ bool FidoDeviceDiscovery::RemoveDevice(base::StringPiece device_id) {
   return true;
 }
 
-// ScopedFidoDiscoveryFactory -------------------------------------------------
-
-namespace internal {
-
-ScopedFidoDiscoveryFactory::ScopedFidoDiscoveryFactory() {
-  DCHECK(!g_current_factory);
-  g_current_factory = this;
-  original_factory_func_ =
-      std::exchange(FidoDeviceDiscovery::g_factory_func_,
-                    &ForwardCreateFidoDiscoveryToCurrentFactory);
-  original_cable_factory_func_ =
-      std::exchange(FidoDeviceDiscovery::g_cable_factory_func_,
-                    &ForwardCreateCableDiscoveryToCurrentFactory);
-}
-
-ScopedFidoDiscoveryFactory::~ScopedFidoDiscoveryFactory() {
-  g_current_factory = nullptr;
-  FidoDeviceDiscovery::g_factory_func_ = original_factory_func_;
-  FidoDeviceDiscovery::g_cable_factory_func_ = original_cable_factory_func_;
-}
-
-// static
-std::unique_ptr<FidoDeviceDiscovery>
-ScopedFidoDiscoveryFactory::ForwardCreateFidoDiscoveryToCurrentFactory(
-    FidoTransportProtocol transport,
-    ::service_manager::Connector* connector) {
-  DCHECK(g_current_factory);
-  return g_current_factory->CreateFidoDiscovery(transport, connector);
-}
-
-// static
-std::unique_ptr<FidoDeviceDiscovery>
-ScopedFidoDiscoveryFactory::ForwardCreateCableDiscoveryToCurrentFactory(
-    std::vector<CableDiscoveryData> cable_data) {
-  DCHECK(g_current_factory);
-  g_current_factory->set_last_cable_data(std::move(cable_data));
-  return g_current_factory->CreateFidoDiscovery(
-      FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy,
-      nullptr /* connector */);
-}
-
-// static
-ScopedFidoDiscoveryFactory* ScopedFidoDiscoveryFactory::g_current_factory =
-    nullptr;
-
-}  // namespace internal
 }  // namespace device

device/fido/fido_device_discovery.h

@@ -17,23 +17,14 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string_piece.h"
-#include "device/fido/cable/cable_discovery_data.h"
 #include "device/fido/fido_discovery_base.h"
 #include "device/fido/fido_transport_protocol.h"
 
-namespace service_manager {
-class Connector;
-}
-
 namespace device {
 
 class FidoDevice;
 class FidoDeviceAuthenticator;
 
-namespace internal {
-class ScopedFidoDiscoveryFactory;
-}
-
 class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceDiscovery
     : public FidoDiscoveryBase {
  public:
@@ -43,18 +34,6 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceDiscovery
     kRunning,
   };
 
-  // Factory functions to construct an instance that discovers authenticators on
-  // the given |transport| protocol. The first variant is for everything except
-  // for cloud-assisted BLE which is handled by the second variant.
-  //
-  // FidoTransportProtocol::kUsbHumanInterfaceDevice requires specifying a valid
-  // |connector| on Desktop, and is not valid on Android.
-  static std::unique_ptr<FidoDeviceDiscovery> Create(
-      FidoTransportProtocol transport,
-      ::service_manager::Connector* connector);
-  static std::unique_ptr<FidoDeviceDiscovery> CreateCable(
-      std::vector<CableDiscoveryData> cable_data);
-
   ~FidoDeviceDiscovery() override;
 
   bool is_start_requested() const { return state_ != State::kIdle; }
@@ -94,67 +73,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceDiscovery
       authenticators_;
 
  private:
-  friend class internal::ScopedFidoDiscoveryFactory;
-
-  // Factory function can be overridden by tests to construct fakes.
-  using FactoryFuncPtr = decltype(&Create);
-  using CableFactoryFuncPtr = decltype(&CreateCable);
-  static FactoryFuncPtr g_factory_func_;
-  static CableFactoryFuncPtr g_cable_factory_func_;
-
   State state_ = State::kIdle;
   base::WeakPtrFactory<FidoDeviceDiscovery> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FidoDeviceDiscovery);
 };
 
-namespace internal {
-
-// Base class for a scoped override of FidoDeviceDiscovery::Create, used in unit
-// tests, layout tests, and when running with the Web Authn Testing API enabled.
-//
-// While there is a subclass instance in scope, calls to the factory method will
-// be hijacked such that the derived class's CreateFidoDiscovery method will be
-// invoked instead.
-class COMPONENT_EXPORT(DEVICE_FIDO) ScopedFidoDiscoveryFactory {
- public:
-  // There should be at most one instance of any subclass in scope at a time.
-  ScopedFidoDiscoveryFactory();
-  virtual ~ScopedFidoDiscoveryFactory();
-
-  const std::vector<CableDiscoveryData>& last_cable_data() const {
-    return last_cable_data_;
-  }
-
- protected:
-  void set_last_cable_data(std::vector<CableDiscoveryData> cable_data) {
-    last_cable_data_ = std::move(cable_data);
-  }
-
-  virtual std::unique_ptr<FidoDeviceDiscovery> CreateFidoDiscovery(
-      FidoTransportProtocol transport,
-      ::service_manager::Connector* connector) = 0;
-
- private:
-  static std::unique_ptr<FidoDeviceDiscovery>
-  ForwardCreateFidoDiscoveryToCurrentFactory(
-      FidoTransportProtocol transport,
-      ::service_manager::Connector* connector);
-
-  static std::unique_ptr<FidoDeviceDiscovery>
-  ForwardCreateCableDiscoveryToCurrentFactory(
-      std::vector<CableDiscoveryData> cable_data);
-
-  static ScopedFidoDiscoveryFactory* g_current_factory;
-
-  FidoDeviceDiscovery::FactoryFuncPtr original_factory_func_;
-  FidoDeviceDiscovery::CableFactoryFuncPtr original_cable_factory_func_;
-  std::vector<CableDiscoveryData> last_cable_data_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedFidoDiscoveryFactory);
-};
-
-}  // namespace internal
 }  // namespace device
 
 #endif  // DEVICE_FIDO_FIDO_DEVICE_DISCOVERY_H_

device/fido/fido_discovery_factory.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/fido/fido_discovery_factory.h"
+
+#include "base/logging.h"
+#include "build/build_config.h"
+#include "device/fido/ble/fido_ble_discovery.h"
+#include "device/fido/cable/fido_cable_discovery.h"
+#include "device/fido/fido_discovery_base.h"
+
+// HID is not supported on Android.
+#if !defined(OS_ANDROID)
+#include "device/fido/hid/fido_hid_discovery.h"
+#endif  // !defined(OS_ANDROID)
+
+namespace device {
+namespace {
+std::unique_ptr<FidoDiscoveryBase> CreateFidoDiscoveryImpl(
+    FidoTransportProtocol transport,
+    service_manager::Connector* connector) {
+  switch (transport) {
+    case FidoTransportProtocol::kUsbHumanInterfaceDevice:
+#if !defined(OS_ANDROID)
+      DCHECK(connector);
+      return std::make_unique<FidoHidDiscovery>(connector);
+#else
+      NOTREACHED() << "USB HID not supported on Android.";
+      return nullptr;
+#endif  // !defined(OS_ANDROID)
+    case FidoTransportProtocol::kBluetoothLowEnergy:
+      return std::make_unique<FidoBleDiscovery>();
+    case FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy:
+      NOTREACHED() << "Cable discovery is constructed using the dedicated "
+                      "factory method.";
+      return nullptr;
+    case FidoTransportProtocol::kNearFieldCommunication:
+      // TODO(https://crbug.com/825949): Add NFC support.
+      return nullptr;
+    case FidoTransportProtocol::kInternal:
+      NOTREACHED() << "Internal authenticators should be handled separately.";
+      return nullptr;
+  }
+  NOTREACHED() << "Unhandled transport type";
+  return nullptr;
+}
+}  // namespace
+
+std::unique_ptr<FidoDiscoveryBase> CreateCableDiscoveryImpl(
+    std::vector<CableDiscoveryData> cable_data) {
+  return std::make_unique<FidoCableDiscovery>(std::move(cable_data));
+}
+
+// static
+FidoDiscoveryFactory::FactoryFuncPtr FidoDiscoveryFactory::g_factory_func_ =
+    &CreateFidoDiscoveryImpl;
+
+// static
+FidoDiscoveryFactory::CableFactoryFuncPtr
+    FidoDiscoveryFactory::g_cable_factory_func_ = &CreateCableDiscoveryImpl;
+
+// static
+std::unique_ptr<FidoDiscoveryBase> FidoDiscoveryFactory::Create(
+    FidoTransportProtocol transport,
+    service_manager::Connector* connector) {
+  return (*g_factory_func_)(transport, connector);
+}
+
+//  static
+std::unique_ptr<FidoDiscoveryBase> FidoDiscoveryFactory::CreateCable(
+    std::vector<CableDiscoveryData> cable_data) {
+  return (*g_cable_factory_func_)(std::move(cable_data));
+}
+
+// ScopedFidoDiscoveryFactory -------------------------------------------------
+
+namespace internal {
+
+ScopedFidoDiscoveryFactory::ScopedFidoDiscoveryFactory() {
+  DCHECK(!g_current_factory);
+  g_current_factory = this;
+  original_factory_func_ =
+      std::exchange(FidoDiscoveryFactory::g_factory_func_,
+                    &ForwardCreateFidoDiscoveryToCurrentFactory);
+  original_cable_factory_func_ =
+      std::exchange(FidoDiscoveryFactory::g_cable_factory_func_,
+                    &ForwardCreateCableDiscoveryToCurrentFactory);
+}
+
+ScopedFidoDiscoveryFactory::~ScopedFidoDiscoveryFactory() {
+  g_current_factory = nullptr;
+  FidoDiscoveryFactory::g_factory_func_ = original_factory_func_;
+  FidoDiscoveryFactory::g_cable_factory_func_ = original_cable_factory_func_;
+}
+
+// static
+std::unique_ptr<FidoDiscoveryBase>
+ScopedFidoDiscoveryFactory::ForwardCreateFidoDiscoveryToCurrentFactory(
+    FidoTransportProtocol transport,
+    ::service_manager::Connector* connector) {
+  DCHECK(g_current_factory);
+  return g_current_factory->CreateFidoDiscovery(transport, connector);
+}
+
+// static
+std::unique_ptr<FidoDiscoveryBase>
+ScopedFidoDiscoveryFactory::ForwardCreateCableDiscoveryToCurrentFactory(
+    std::vector<CableDiscoveryData> cable_data) {
+  DCHECK(g_current_factory);
+  g_current_factory->set_last_cable_data(std::move(cable_data));
+  return g_current_factory->CreateFidoDiscovery(
+      FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy,
+      nullptr /* connector */);
+}
+
+// static
+ScopedFidoDiscoveryFactory* ScopedFidoDiscoveryFactory::g_current_factory =
+    nullptr;
+
+}  // namespace internal
+}  // namespace device

device/fido/fido_discovery_factory.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_FIDO_FIDO_DISCOVERY_FACTORY_H_
+#define DEVICE_FIDO_FIDO_DISCOVERY_FACTORY_H_
+
+#include <memory>
+#include <vector>
+
+#include "base/component_export.h"
+#include "device/fido/cable/cable_discovery_data.h"
+#include "device/fido/fido_device_discovery.h"
+#include "device/fido/fido_discovery_base.h"
+#include "device/fido/fido_transport_protocol.h"
+
+namespace service_manager {
+class Connector;
+}
+
+namespace device {
+
+namespace internal {
+class ScopedFidoDiscoveryFactory;
+}
+
+// FidoDiscoveryFactory offers methods to construct instances of
+// FidoDiscoveryBase for a given |transport| protocol.
+class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryFactory {
+ public:
+  // Instantiates a FidoDiscoveryBase for all protocols except caBLE and
+  // internal/platform.
+  //
+  // FidoTransportProtocol::kUsbHumanInterfaceDevice requires specifying a
+  // valid |connector| on Desktop, and is not valid on Android.
+  static std::unique_ptr<FidoDiscoveryBase> Create(
+      FidoTransportProtocol transport,
+      ::service_manager::Connector* connector);
+  // Instantiates a FidoDiscovery for caBLE.
+  static std::unique_ptr<FidoDiscoveryBase> CreateCable(
+      std::vector<CableDiscoveryData> cable_data);
+
+ private:
+  friend class internal::ScopedFidoDiscoveryFactory;
+
+  // Factory function can be overridden by tests to construct fakes.
+  using FactoryFuncPtr = decltype(&Create);
+  using CableFactoryFuncPtr = decltype(&CreateCable);
+  static FactoryFuncPtr g_factory_func_;
+  static CableFactoryFuncPtr g_cable_factory_func_;
+};
+
+namespace internal {
+// Base class for a scoped override of FidoDiscoveryFactory::Create, used in
+// unit tests, layout tests, and when running with the Web Authn Testing API
+// enabled.
+//
+// While there is a subclass instance in scope, calls to the factory method will
+// be hijacked such that the derived class's CreateFidoDiscovery method will be
+// invoked instead.
+class COMPONENT_EXPORT(DEVICE_FIDO) ScopedFidoDiscoveryFactory {
+ public:
+  // There should be at most one instance of any subclass in scope at a time.
+  ScopedFidoDiscoveryFactory();
+  virtual ~ScopedFidoDiscoveryFactory();
+
+  const std::vector<CableDiscoveryData>& last_cable_data() const {
+    return last_cable_data_;
+  }
+
+ protected:
+  void set_last_cable_data(std::vector<CableDiscoveryData> cable_data) {
+    last_cable_data_ = std::move(cable_data);
+  }
+
+  virtual std::unique_ptr<FidoDiscoveryBase> CreateFidoDiscovery(
+      FidoTransportProtocol transport,
+      ::service_manager::Connector* connector) = 0;
+
+ private:
+  static std::unique_ptr<FidoDiscoveryBase>
+  ForwardCreateFidoDiscoveryToCurrentFactory(
+      FidoTransportProtocol transport,
+      ::service_manager::Connector* connector);
+
+  static std::unique_ptr<FidoDiscoveryBase>
+  ForwardCreateCableDiscoveryToCurrentFactory(
+      std::vector<CableDiscoveryData> cable_data);
+
+  static ScopedFidoDiscoveryFactory* g_current_factory;
+
+  FidoDiscoveryFactory::FactoryFuncPtr original_factory_func_;
+  FidoDiscoveryFactory::CableFactoryFuncPtr original_cable_factory_func_;
+  std::vector<CableDiscoveryData> last_cable_data_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedFidoDiscoveryFactory);
+};
+
+}  // namespace internal
+}  // namespace device
+
+#endif  // DEVICE_FIDO_FIDO_DISCOVERY_FACTORY_H_

device/fido/fido_request_handler_base.cc

@@ -14,6 +14,7 @@
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
 #include "device/fido/ble_adapter_manager.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "services/service_manager/public/cpp/connector.h"
 
 namespace device {
@@ -84,7 +85,7 @@ FidoRequestHandlerBase::FidoRequestHandlerBase(
       continue;
     }
 
-    auto discovery = FidoDeviceDiscovery::Create(transport, connector);
+    auto discovery = FidoDiscoveryFactory::Create(transport, connector);
     if (discovery == nullptr) {
       // This can occur in tests when a ScopedVirtualU2fDevice is in effect and
       // HID transports are not configured.

device/fido/fido_request_handler_base.h

@@ -20,7 +20,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string_piece_forward.h"
 #include "device/fido/fido_device_authenticator.h"
-#include "device/fido/fido_device_discovery.h"
+#include "device/fido/fido_discovery_base.h"
 #include "device/fido/fido_transport_protocol.h"
 
 namespace service_manager {

device/fido/get_assertion_request_handler.cc

@@ -13,6 +13,7 @@
 #include "device/fido/authenticator_get_assertion_response.h"
 #include "device/fido/cable/fido_cable_discovery.h"
 #include "device/fido/fido_authenticator.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "device/fido/get_assertion_task.h"
 
 namespace device {
@@ -176,7 +177,7 @@ GetAssertionRequestHandler::GetAssertionRequestHandler(
           FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy)) {
     DCHECK(request_.cable_extension());
     auto discovery =
-        FidoDeviceDiscovery::CreateCable(*request_.cable_extension());
+        FidoDiscoveryFactory::CreateCable(*request_.cable_extension());
     discovery->set_observer(this);
     discoveries().push_back(std::move(discovery));
   }

device/fido/scoped_virtual_fido_device.cc

@@ -65,8 +65,7 @@ VirtualFidoDevice::State* ScopedVirtualFidoDevice::mutable_state() {
   return state_.get();
 }
 
-std::unique_ptr<FidoDeviceDiscovery>
-ScopedVirtualFidoDevice::CreateFidoDiscovery(
+std::unique_ptr<FidoDiscoveryBase> ScopedVirtualFidoDevice::CreateFidoDiscovery(
     FidoTransportProtocol transport,
     ::service_manager::Connector* connector) {
   if (transport != FidoTransportProtocol::kUsbHumanInterfaceDevice) {

device/fido/scoped_virtual_fido_device.h

@@ -9,7 +9,7 @@
 
 #include "base/macros.h"
 #include "device/fido/fido_constants.h"
-#include "device/fido/fido_device_discovery.h"
+#include "device/fido/fido_discovery_factory.h"
 #include "device/fido/virtual_fido_device.h"
 
 namespace device {
@@ -28,7 +28,7 @@ class ScopedVirtualFidoDevice
   VirtualFidoDevice::State* mutable_state();
 
  protected:
-  std::unique_ptr<FidoDeviceDiscovery> CreateFidoDiscovery(
+  std::unique_ptr<FidoDiscoveryBase> CreateFidoDiscovery(
       FidoTransportProtocol transport,
       ::service_manager::Connector* connector) override;