Skip to content

Commit

Permalink
[QuickStart] Add QuickStartConnectivityService and related plumbing
Browse files Browse the repository at this point in the history
Add a new KeyedService for Quick Start which is used to fetch a
NearbyConnectionsManager. Quick Start will use this to advertise and
create a NearbyConnection to communicate with the phone.

The service is added to //chrome/browser/ash/nearby in order to avoid
a dependency cycle that would result if :oobe_quick_start depended
on NearbyConnectionsManagerImpl directly. In the future, after we
finish refactoring //chrome/browser/nearby_sharing to extract out the
Nearby-Share-agnostic code into a reusable component, we should then
be able to move the service into the oobe_quick_start directory and
get rid of the dependency injection.

The next CL in this relation chain will perform Nearby Connections
advertising, and the unit tests for that functionality will provide
additional coverage for this code.

TEST=Manually test for regressions in Quick Start and Nearby Share.

Bug: b/234655072
Change-Id: I304fa4bb479d6ed07b641d010139c9372ffe728f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4006775
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Renato Silva <rrsilva@google.com>
Reviewed-by: Michael Hansen <hansenmichael@google.com>
Commit-Queue: Michael Hansen <hansenmichael@google.com>
Cr-Commit-Position: refs/heads/main@{#1085134}
  • Loading branch information
Curt Clemens authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent 326f09b commit 8054c42
Show file tree
Hide file tree
Showing 27 changed files with 301 additions and 29 deletions.
5 changes: 5 additions & 0 deletions chrome/browser/ash/BUILD.gn
Expand Up @@ -1985,6 +1985,10 @@ source_set("ash") {
"nearby/nearby_process_manager_factory.h",
"nearby/nearby_process_manager_impl.cc",
"nearby/nearby_process_manager_impl.h",
"nearby/quick_start_connectivity_service.cc",
"nearby/quick_start_connectivity_service.h",
"nearby/quick_start_connectivity_service_factory.cc",
"nearby/quick_start_connectivity_service_factory.h",
"net/bluetooth_pref_state_observer.cc",
"net/bluetooth_pref_state_observer.h",
"net/client_cert_filter.cc",
Expand Down Expand Up @@ -3696,6 +3700,7 @@ source_set("ash") {
"//chrome/browser/metrics/structured",
"//chrome/browser/nearby_sharing/common",
"//chrome/browser/nearby_sharing/logging",
"//chrome/browser/nearby_sharing/public/cpp",
"//chrome/browser/policy:onc",
"//chrome/browser/profiles",
"//chrome/browser/resources:component_extension_resources",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ash/login/oobe_quick_start/BUILD.gn
Expand Up @@ -43,6 +43,7 @@ source_set("unit_tests") {
"logging:unit_tests",
"//base",
"//base/test:test_support",
"//chrome/test:test_support",
"//chromeos/ash/components/attestation:test_support",
"//chromeos/ash/components/dbus/constants:constants",
"//components/account_id:account_id",
Expand Down
Expand Up @@ -11,6 +11,8 @@ source_set("connectivity") {
":decoder",
"//base",
"//chrome/browser/nearby_sharing/public/cpp",
"//chrome/browser/nearby_sharing/public/cpp",
"//chromeos/ash/services/nearby/public/cpp",
"//components/cbor",
"//crypto",
"//device/bluetooth",
Expand Down
Expand Up @@ -4,25 +4,25 @@

#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker_factory.h"

#include "base/memory/weak_ptr.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/random_session_id.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker_impl.h"

namespace ash::quick_start {

// static
std::unique_ptr<TargetDeviceConnectionBroker>
TargetDeviceConnectionBrokerFactory::Create() {
return Create(RandomSessionId());
}
TargetDeviceConnectionBrokerFactory::Create(
base::WeakPtr<NearbyConnectionsManager> nearby_connections_manager,
absl::optional<RandomSessionId> session_id) {
RandomSessionId id = session_id ? *session_id : RandomSessionId();

// static
std::unique_ptr<TargetDeviceConnectionBroker>
TargetDeviceConnectionBrokerFactory::Create(RandomSessionId session_id) {
if (test_factory_) {
return test_factory_->CreateInstance(session_id);
return test_factory_->CreateInstance(id);
}

return std::make_unique<TargetDeviceConnectionBrokerImpl>(session_id);
return std::make_unique<TargetDeviceConnectionBrokerImpl>(
id, nearby_connections_manager);
}

// static
Expand Down
Expand Up @@ -8,6 +8,9 @@
#include <memory>

#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

class NearbyConnectionsManager;

namespace ash::quick_start {

Expand All @@ -17,11 +20,10 @@ class RandomSessionId;
// Calling code should use the static Create() method.
class TargetDeviceConnectionBrokerFactory {
public:
static std::unique_ptr<TargetDeviceConnectionBroker> Create();

// A RandomSessionId may be provided in order to resume a connection.
static std::unique_ptr<TargetDeviceConnectionBroker> Create(
RandomSessionId session_id);
base::WeakPtr<NearbyConnectionsManager> nearby_connections_manager,
absl::optional<RandomSessionId> session_id);

static void SetFactoryForTesting(
TargetDeviceConnectionBrokerFactory* test_factory);
Expand Down
Expand Up @@ -100,8 +100,10 @@ TargetDeviceConnectionBrokerImpl::BluetoothAdapterFactoryWrapper*
bluetooth_adapter_factory_wrapper_for_testing_ = nullptr;

TargetDeviceConnectionBrokerImpl::TargetDeviceConnectionBrokerImpl(
RandomSessionId session_id)
: random_session_id_(session_id) {
RandomSessionId session_id,
base::WeakPtr<NearbyConnectionsManager> nearby_connections_manager)
: random_session_id_(session_id),
nearby_connections_manager_(nearby_connections_manager) {
GetBluetoothAdapter();
}

Expand Down Expand Up @@ -181,6 +183,7 @@ void TargetDeviceConnectionBrokerImpl::StartAdvertising(
base::SplitOnceCallback(std::move(on_start_advertising_callback));

fast_pair_advertiser_->StartAdvertising(
// TODO(b/234655072): on success, start Nearby Connections advertising.
base::BindOnce(std::move(success_callback), /*success=*/true),
base::BindOnce(
&TargetDeviceConnectionBrokerImpl::OnStartFastPairAdvertisingError,
Expand Down
Expand Up @@ -8,6 +8,7 @@
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/random_session_id.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker.h"
#include "chrome/browser/nearby_sharing/public/cpp/nearby_connections_manager.h"
#include "device/bluetooth/bluetooth_adapter_factory.h"

namespace ash::quick_start {
Expand Down Expand Up @@ -40,7 +41,8 @@ class TargetDeviceConnectionBrokerImpl : public TargetDeviceConnectionBroker {
bluetooth_adapter_factory_wrapper_for_testing_;
};

explicit TargetDeviceConnectionBrokerImpl(RandomSessionId session_id);
TargetDeviceConnectionBrokerImpl(RandomSessionId session_id,
base::WeakPtr<NearbyConnectionsManager>);
TargetDeviceConnectionBrokerImpl(TargetDeviceConnectionBrokerImpl&) = delete;
TargetDeviceConnectionBrokerImpl& operator=(
TargetDeviceConnectionBrokerImpl&) = delete;
Expand All @@ -53,6 +55,8 @@ class TargetDeviceConnectionBrokerImpl : public TargetDeviceConnectionBroker {
void StopAdvertising(base::OnceClosure on_stop_advertising_callback) override;

private:
// Used to access the |random_session_id_| in tests, and to allow testing
// |GenerateEndpointInfo()| directly.
friend class TargetDeviceConnectionBrokerImplTest;

void GetBluetoothAdapter();
Expand All @@ -70,6 +74,8 @@ class TargetDeviceConnectionBrokerImpl : public TargetDeviceConnectionBroker {
std::unique_ptr<FastPairAdvertiser> fast_pair_advertiser_;
RandomSessionId random_session_id_;

base::WeakPtr<NearbyConnectionsManager> nearby_connections_manager_;

base::WeakPtrFactory<TargetDeviceConnectionBrokerImpl> weak_ptr_factory_{
this};
};
Expand Down
Expand Up @@ -12,6 +12,7 @@
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/fast_pair_advertiser.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/random_session_id.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker_factory.h"
#include "chrome/browser/nearby_sharing/fake_nearby_connections_manager.h"
#include "chromeos/constants/devicetype.h"
#include "device/bluetooth/bluetooth_adapter_factory.h"
#include "device/bluetooth/test/mock_bluetooth_adapter.h"
Expand Down Expand Up @@ -218,7 +219,8 @@ class TargetDeviceConnectionBrokerImplTest : public testing::Test {
void CreateConnectionBroker() {
RandomSessionId session_id(kRandomSessionId);
connection_broker_ =
TargetDeviceConnectionBrokerFactory::Create(session_id);
ash::quick_start::TargetDeviceConnectionBrokerFactory::Create(
fake_nearby_connections_manager_.GetWeakPtr(), session_id);
}

void FinishFetchingBluetoothAdapter() {
Expand Down Expand Up @@ -268,6 +270,7 @@ class TargetDeviceConnectionBrokerImplTest : public testing::Test {
bool start_advertising_callback_success_ = false;
bool stop_advertising_callback_called_ = false;
scoped_refptr<NiceMock<device::MockBluetoothAdapter>> mock_bluetooth_adapter_;
FakeNearbyConnectionsManager fake_nearby_connections_manager_;
std::unique_ptr<TargetDeviceConnectionBroker> connection_broker_;
std::unique_ptr<FakeFastPairAdvertiserFactory> fast_pair_advertiser_factory_;
DeferredBluetoothAdapterFactoryWrapper bluetooth_adapter_factory_wrapper_;
Expand Down
Expand Up @@ -12,6 +12,7 @@
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker_factory.h"
#include "components/qr_code_generator/qr_code_generator.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/abseil-cpp/absl/types/variant.h"

namespace ash::quick_start {
Expand All @@ -33,9 +34,11 @@ TargetDeviceBootstrapController::QRCodePixelData GenerateQRCode(

} // namespace

TargetDeviceBootstrapController::TargetDeviceBootstrapController() {
connection_broker_ = TargetDeviceConnectionBrokerFactory::Create();
}
TargetDeviceBootstrapController::TargetDeviceBootstrapController(
base::WeakPtr<NearbyConnectionsManager> nearby_connections_manager)
: connection_broker_(TargetDeviceConnectionBrokerFactory::Create(
nearby_connections_manager,
/*session_id=*/absl::nullopt)) {}

TargetDeviceBootstrapController::~TargetDeviceBootstrapController() = default;

Expand Down
Expand Up @@ -13,6 +13,8 @@
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/target_device_connection_broker.h"
#include "third_party/abseil-cpp/absl/types/variant.h"

class NearbyConnectionsManager;

namespace ash::quick_start {

class AuthenticatedConnection;
Expand All @@ -21,7 +23,8 @@ class IncomingConnection;
class TargetDeviceBootstrapController
: public TargetDeviceConnectionBroker::ConnectionLifecycleListener {
public:
TargetDeviceBootstrapController();
explicit TargetDeviceBootstrapController(
base::WeakPtr<NearbyConnectionsManager> nearby_connections_manager);
TargetDeviceBootstrapController(TargetDeviceBootstrapController&) = delete;
TargetDeviceBootstrapController& operator=(TargetDeviceBootstrapController&) =
delete;
Expand Down
Expand Up @@ -7,6 +7,7 @@

#include "base/test/bind.h"
#include "chrome/browser/ash/login/oobe_quick_start/connectivity/fake_target_device_connection_broker.h"
#include "chrome/browser/nearby_sharing/fake_nearby_connections_manager.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
Expand Down Expand Up @@ -57,7 +58,8 @@ class TargetDeviceBootstrapControllerTest : public testing::Test {
TargetDeviceConnectionBrokerFactory::SetFactoryForTesting(
&connection_broker_factory_);

bootstrap_controller_ = std::make_unique<TargetDeviceBootstrapController>();
bootstrap_controller_ = std::make_unique<TargetDeviceBootstrapController>(
fake_nearby_connections_manager_.GetWeakPtr());
fake_observer_ = std::make_unique<FakeObserver>();
bootstrap_controller_->AddObserver(fake_observer_.get());
}
Expand All @@ -69,6 +71,7 @@ class TargetDeviceBootstrapControllerTest : public testing::Test {

protected:
FakeTargetDeviceConnectionBroker::Factory connection_broker_factory_;
FakeNearbyConnectionsManager fake_nearby_connections_manager_;
std::unique_ptr<FakeObserver> fake_observer_;
std::unique_ptr<TargetDeviceBootstrapController> bootstrap_controller_;
};
Expand Down
12 changes: 11 additions & 1 deletion chrome/browser/ash/login/ui/login_display_host_common.cc
Expand Up @@ -31,6 +31,7 @@
#include "chrome/browser/ash/login/ui/login_feedback.h"
#include "chrome/browser/ash/login/ui/signin_ui.h"
#include "chrome/browser/ash/login/wizard_controller.h"
#include "chrome/browser/ash/nearby/quick_start_connectivity_service_factory.h"
#include "chrome/browser/ash/policy/core/browser_policy_connector_ash.h"
#include "chrome/browser/ash/profiles/profile_helper.h"
#include "chrome/browser/ash/profiles/signin_profile_handler.h"
Expand Down Expand Up @@ -687,8 +688,17 @@ base::WeakPtr<quick_start::TargetDeviceBootstrapController>
LoginDisplayHostCommon::GetQuickStartBootstrapController() {
DCHECK(features::IsOobeQuickStartEnabled());
if (!bootstrap_controller_) {
Profile* profile = ProfileManager::GetActiveUserProfile();
DCHECK(profile);

quick_start::QuickStartConnectivityService* service =
quick_start::QuickStartConnectivityServiceFactory::GetForProfile(
profile);
DCHECK(service);

bootstrap_controller_ =
std::make_unique<ash::quick_start::TargetDeviceBootstrapController>();
std::make_unique<ash::quick_start::TargetDeviceBootstrapController>(
service->GetNearbyConnectionsManager());
}
return bootstrap_controller_->GetAsWeakPtrForClient();
}
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ash/nearby/nearby_dependencies_provider.cc
Expand Up @@ -96,7 +96,6 @@ NearbyDependenciesProvider::NearbyDependenciesProvider(
signin::IdentityManager* identity_manager)
: profile_(profile), identity_manager_(identity_manager) {
DCHECK(profile_);
DCHECK(identity_manager_);
bluetooth_manager_ = std::make_unique<BluetoothAdapterManager>();
}

Expand Down
13 changes: 13 additions & 0 deletions chrome/browser/ash/nearby/nearby_dependencies_provider_factory.cc
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/ash/nearby/nearby_dependencies_provider_factory.h"

#include "ash/constants/ash_features.h"
#include "chrome/browser/ash/nearby/nearby_dependencies_provider.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_manager_factory.h"
Expand Down Expand Up @@ -47,4 +48,16 @@ bool NearbyDependenciesProviderFactory::ServiceIsCreatedWithBrowserContext()
return true;
}

// This needs to be overridden because the default implementation returns
// nullptr for OTR profiles, which would prevent using this with Quick Start.
content::BrowserContext*
NearbyDependenciesProviderFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
if (features::IsOobeQuickStartEnabled()) {
return context;
} else {
return BrowserContextKeyedServiceFactory::GetBrowserContextToUse(context);
}
}

} // namespace ash::nearby
Expand Up @@ -36,6 +36,8 @@ class NearbyDependenciesProviderFactory
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
bool ServiceIsCreatedWithBrowserContext() const override;
content::BrowserContext* GetBrowserContextToUse(
content::BrowserContext* context) const override;
};

} // namespace ash::nearby
Expand Down
20 changes: 16 additions & 4 deletions chrome/browser/ash/nearby/nearby_process_manager_factory.cc
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/ash/nearby/nearby_process_manager_factory.h"

#include "ash/constants/ash_features.h"
#include "chrome/browser/ash/nearby/nearby_dependencies_provider_factory.h"
#include "chrome/browser/ash/nearby/nearby_process_manager_impl.h"
#include "chrome/browser/ash/profiles/profile_helper.h"
Expand All @@ -28,14 +29,24 @@ NearbyProcessManager* NearbyProcessManagerFactory::GetForProfile(

// static
bool NearbyProcessManagerFactory::CanBeLaunchedForProfile(Profile* profile) {
// Guest/incognito profiles cannot use Phone Hub.
if (profile->IsOffTheRecord())
// We allow NearbyProcessManager to be used with the signin profile since it
// is required for OOBE Quick Start.
if (ProfileHelper::IsSigninProfile(profile) &&
features::IsOobeQuickStartEnabled()) {
return true;
}

// Guest/incognito profiles cannot use Nearby Connections.
if (profile->IsOffTheRecord()) {
return false;
}

// Likewise, kiosk users are ineligible.
if (user_manager::UserManager::Get()->IsLoggedInAsAnyKioskApp())
if (user_manager::UserManager::Get()->IsLoggedInAsAnyKioskApp()) {
return false;
}

// Nearby Connections is not supported for secondary profiles.
return ProfileHelper::IsPrimaryProfile(profile);
}

Expand All @@ -52,7 +63,8 @@ void NearbyProcessManagerFactory::SetBypassPrimaryUserCheckForTesting(
}

NearbyProcessManagerFactory::NearbyProcessManagerFactory()
: ProfileKeyedServiceFactory("NearbyProcessManager") {
: ProfileKeyedServiceFactory("NearbyProcessManager",
ProfileSelections::BuildForAllProfiles()) {
DependsOn(NearbyDependenciesProviderFactory::GetInstance());
}

Expand Down
39 changes: 39 additions & 0 deletions chrome/browser/ash/nearby/quick_start_connectivity_service.cc
@@ -0,0 +1,39 @@
// Copyright 2022 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ash/nearby/quick_start_connectivity_service.h"

#include "base/memory/weak_ptr.h"
#include "chrome/browser/nearby_sharing/nearby_connections_manager_impl.h"
#include "chrome/browser/nearby_sharing/public/cpp/nearby_connections_manager.h"
#include "chromeos/ash/services/nearby/public/cpp/nearby_process_manager.h"

namespace ash::quick_start {

namespace {

constexpr char kServiceId[] = "QuickStart";

} // namespace

QuickStartConnectivityService::QuickStartConnectivityService(
nearby::NearbyProcessManager* nearby_process_manager)
: nearby_process_manager_(nearby_process_manager) {}

QuickStartConnectivityService::~QuickStartConnectivityService() = default;

base::WeakPtr<NearbyConnectionsManager>
QuickStartConnectivityService::GetNearbyConnectionsManager() {
DCHECK(nearby_process_manager_);

if (!nearby_connections_manager_) {
nearby_connections_manager_ =
std::make_unique<NearbyConnectionsManagerImpl>(nearby_process_manager_,
kServiceId);
}

return nearby_connections_manager_->GetWeakPtr();
}

} // namespace ash::quick_start

0 comments on commit 8054c42

Please sign in to comment.