From fbc581f266080cde2f2b09316946c9b99a551aa4 Mon Sep 17 00:00:00 2001 From: Michael Hansen Date: Fri, 30 Jul 2021 23:30:35 +0000 Subject: [PATCH] [Nearby] Trigger onboarding notification via observer interface. The triggers the "Device nearby is sharing" notification via observer methods on NearbySharingService rather than by calling ShowOnboarding() directly. This is part of the cleanup of the Background Scanning prototype and aligns better with the existing pattern that we use to show other notifications. Bug: 1223888 Change-Id: I0e4f29010b932b2620344b24e434082280728922 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3063982 Reviewed-by: Ryan Hansberry Commit-Queue: Michael Hansen Cr-Commit-Position: refs/heads/master@{#907314} --- .../mock_nearby_sharing_service.h | 1 + .../nearby_notification_manager.cc | 14 ++++++ .../nearby_notification_manager.h | 10 ++-- .../nearby_notification_manager_unittest.cc | 49 +++++++++++++++++++ .../nearby_sharing/nearby_sharing_service.h | 8 +++ .../nearby_sharing_service_impl.cc | 25 +++++----- .../nearby_sharing_service_impl.h | 1 + 7 files changed, 94 insertions(+), 14 deletions(-) diff --git a/chrome/browser/nearby_sharing/mock_nearby_sharing_service.h b/chrome/browser/nearby_sharing/mock_nearby_sharing_service.h index 6c4cdefeadbd2d..9528bcd41d997a 100644 --- a/chrome/browser/nearby_sharing/mock_nearby_sharing_service.h +++ b/chrome/browser/nearby_sharing/mock_nearby_sharing_service.h @@ -89,6 +89,7 @@ class MockNearbySharingService : public NearbySharingService { GetCertificateManager, (), (override)); + MOCK_METHOD(bool, AreFastInitiationDevicesDetected, (), (const override)); }; #endif // CHROME_BROWSER_NEARBY_SHARING_MOCK_NEARBY_SHARING_SERVICE_H_ diff --git a/chrome/browser/nearby_sharing/nearby_notification_manager.cc b/chrome/browser/nearby_sharing/nearby_notification_manager.cc index c3ec320821c28a..5f7cdbdc45473e 100644 --- a/chrome/browser/nearby_sharing/nearby_notification_manager.cc +++ b/chrome/browser/nearby_sharing/nearby_notification_manager.cc @@ -720,6 +720,20 @@ void NearbyNotificationManager::OnNearbyProcessStopped() { last_transfer_status_ = absl::nullopt; } +void NearbyNotificationManager::OnFastInitiationDeviceFound() { + ShowOnboarding(); +} + +void NearbyNotificationManager::OnFastInitiationDeviceLost() { + if (!nearby_service_->AreFastInitiationDevicesDetected()) { + CloseOnboarding(); + } +} + +void NearbyNotificationManager::OnFastInitiationScanningStopped() { + CloseOnboarding(); +} + void NearbyNotificationManager::ShowProgress( const ShareTarget& share_target, const TransferMetadata& transfer_metadata) { diff --git a/chrome/browser/nearby_sharing/nearby_notification_manager.h b/chrome/browser/nearby_sharing/nearby_notification_manager.h index 1bbcad5bbf3715..1c0f01b4083b05 100644 --- a/chrome/browser/nearby_sharing/nearby_notification_manager.h +++ b/chrome/browser/nearby_sharing/nearby_notification_manager.h @@ -67,6 +67,9 @@ class NearbyNotificationManager : public TransferUpdateCallback, void OnHighVisibilityChanged(bool in_high_visibility) override {} void OnNearbyProcessStopped() override; void OnShutdown() override {} + void OnFastInitiationDeviceFound() override; + void OnFastInitiationDeviceLost() override; + void OnFastInitiationScanningStopped() override; // Shows a progress notification of the data being transferred to or from // |share_target|. Has a cancel action to cancel the transfer. @@ -80,8 +83,8 @@ class NearbyNotificationManager : public TransferUpdateCallback, const TransferMetadata& transfer_metadata); // Shows an onboarding notification when a nearby device is attempting to - // share. Clicking it will make the local device visible to all nearby - // devices. + // share. Clicking it will take the user through the onboarding flow if needed + // and then make the local device visible to all nearby devices. void ShowOnboarding(); // Shows a notification for send or receive success. @@ -98,7 +101,8 @@ class NearbyNotificationManager : public TransferUpdateCallback, // connection). void CloseTransfer(); - // Closes any currently shown onboarding notification. + // Closes any currently shown onboarding notification. It does not have any + // effect on the actual onboarding UI or the high visibility mode UI. void CloseOnboarding(); // Gets the currently registered delegate for |notification_id|. diff --git a/chrome/browser/nearby_sharing/nearby_notification_manager_unittest.cc b/chrome/browser/nearby_sharing/nearby_notification_manager_unittest.cc index 9f2fcac8dc6874..c96f51c0579d17 100644 --- a/chrome/browser/nearby_sharing/nearby_notification_manager_unittest.cc +++ b/chrome/browser/nearby_sharing/nearby_notification_manager_unittest.cc @@ -60,6 +60,8 @@ #include "ui/gfx/codec/png_codec.h" #include "ui/strings/grit/ui_strings.h" +using testing::Return; + namespace { const char kTextBody[] = "text body"; @@ -688,6 +690,53 @@ TEST_F(NearbyNotificationManagerTest, ShowOnboarding_ShowsNotification) { EXPECT_EQ(0u, notification.buttons().size()); } +TEST_F(NearbyNotificationManagerTest, + FastInitiationDeviceFound_ShowsOnboarding) { + manager()->OnFastInitiationDeviceFound(); + + std::vector notifications = + GetDisplayedNotifications(); + ASSERT_EQ(1u, notifications.size()); + + // Minimum to confirm it's actually the onboarding notification. + const message_center::Notification& notification = notifications[0]; + EXPECT_EQ(message_center::NOTIFICATION_TYPE_SIMPLE, notification.type()); + EXPECT_EQ(l10n_util::GetStringUTF16(IDS_NEARBY_NOTIFICATION_ONBOARDING_TITLE), + notification.title()); + EXPECT_EQ( + l10n_util::GetStringUTF16(IDS_NEARBY_NOTIFICATION_ONBOARDING_MESSAGE), + notification.message()); +} + +TEST_F(NearbyNotificationManagerTest, + FastInitiationDeviceLost_ClosesOnboarding) { + manager()->OnFastInitiationDeviceFound(); + EXPECT_EQ(1u, GetDisplayedNotifications().size()); + + // The notification should remain visible while there are still Fast + // Initiation devices detected. + ON_CALL(*nearby_service_, AreFastInitiationDevicesDetected()) + .WillByDefault(Return(true)); + manager()->OnFastInitiationDeviceLost(); + EXPECT_EQ(1u, GetDisplayedNotifications().size()); + + // The notification should be dismissed if Fast Initiation devices are no + // longer detected. + ON_CALL(*nearby_service_, AreFastInitiationDevicesDetected()) + .WillByDefault(Return(false)); + manager()->OnFastInitiationDeviceLost(); + EXPECT_EQ(0u, GetDisplayedNotifications().size()); +} + +TEST_F(NearbyNotificationManagerTest, + FastInitiationScanningStopped_ClosesOnboarding) { + manager()->OnFastInitiationDeviceFound(); + EXPECT_EQ(1u, GetDisplayedNotifications().size()); + + manager()->OnFastInitiationScanningStopped(); + EXPECT_EQ(0u, GetDisplayedNotifications().size()); +} + TEST_F(NearbyNotificationManagerTest, ShowSuccess_ShowsNotification) { manager()->ShowSuccess(ShareTarget()); diff --git a/chrome/browser/nearby_sharing/nearby_sharing_service.h b/chrome/browser/nearby_sharing/nearby_sharing_service.h index fa61e550112640..71085982b880b6 100644 --- a/chrome/browser/nearby_sharing/nearby_sharing_service.h +++ b/chrome/browser/nearby_sharing/nearby_sharing_service.h @@ -76,6 +76,10 @@ class NearbySharingService : public KeyedService { virtual void OnStartAdvertisingFailure() {} virtual void OnStartDiscoveryResult(bool success) {} + virtual void OnFastInitiationDeviceFound() {} + virtual void OnFastInitiationDeviceLost() {} + virtual void OnFastInitiationScanningStopped() {} + // Called during the |KeyedService| shutdown, but before everything has been // cleaned up. It is safe to remove any observers on this event. virtual void OnShutdown() = 0; @@ -174,6 +178,10 @@ class NearbySharingService : public KeyedService { virtual NearbyShareLocalDeviceDataManager* GetLocalDeviceDataManager() = 0; virtual NearbyShareContactManager* GetContactManager() = 0; virtual NearbyShareCertificateManager* GetCertificateManager() = 0; + + // Returns true if we currently detect any devices nearby that are attempting + // to share. + virtual bool AreFastInitiationDevicesDetected() const = 0; }; #endif // CHROME_BROWSER_NEARBY_SHARING_NEARBY_SHARING_SERVICE_H_ diff --git a/chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc b/chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc index 9f61183007da39..ac7242d5f473e5 100644 --- a/chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc +++ b/chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc @@ -993,6 +993,11 @@ NearbySharingServiceImpl::GetCertificateManager() { return certificate_manager_.get(); } +bool NearbySharingServiceImpl::AreFastInitiationDevicesDetected() const { + return fast_initiation_scanner_ && + fast_initiation_scanner_->AreFastInitiationDevicesDetected(); +} + void NearbySharingServiceImpl::OnNearbyProcessStopped( NearbyProcessShutdownReason shutdown_reason) { DCHECK(process_reference_); @@ -2182,20 +2187,15 @@ void NearbySharingServiceImpl::StartFastInitiationScanning() { void NearbySharingServiceImpl::OnFastInitiationDeviceFound() { NS_LOG(VERBOSE) << __func__; - - // This shows a notification indicating that a device nearby is attempting to - // share. When the notification is clicked it will take the user through the - // onboarding flow if needed and then enable high visibility mode. - nearby_notification_manager_->ShowOnboarding(); + for (auto& observer : observers_) { + observer.OnFastInitiationDeviceFound(); + } } void NearbySharingServiceImpl::OnFastInitiationDeviceLost() { NS_LOG(VERBOSE) << __func__; - - // This will just dismiss the "onboarding" notification, it does not have any - // effect on the actual onboarding or high visibility UI. - if (!fast_initiation_scanner_->AreFastInitiationDevicesDetected()) { - nearby_notification_manager_->CloseOnboarding(); + for (auto& observer : observers_) { + observer.OnFastInitiationDeviceLost(); } } @@ -2204,8 +2204,11 @@ void NearbySharingServiceImpl::StopFastInitiationScanning() { NS_LOG(VERBOSE) << __func__ << ": Ignoring, not background scanning."; return; } - nearby_notification_manager_->CloseOnboarding(); + fast_initiation_scanner_.reset(); + for (auto& observer : observers_) { + observer.OnFastInitiationScanningStopped(); + } NS_LOG(VERBOSE) << __func__ << ": Stopped background scanning."; } diff --git a/chrome/browser/nearby_sharing/nearby_sharing_service_impl.h b/chrome/browser/nearby_sharing/nearby_sharing_service_impl.h index 58ed061d0b7921..cff1c6efc11fcc 100644 --- a/chrome/browser/nearby_sharing/nearby_sharing_service_impl.h +++ b/chrome/browser/nearby_sharing/nearby_sharing_service_impl.h @@ -133,6 +133,7 @@ class NearbySharingServiceImpl NearbyShareLocalDeviceDataManager* GetLocalDeviceDataManager() override; NearbyShareContactManager* GetContactManager() override; NearbyShareCertificateManager* GetCertificateManager() override; + bool AreFastInitiationDevicesDetected() const override; // NearbyConnectionsManager::IncomingConnectionListener: void OnIncomingConnection(const std::string& endpoint_id,