From 0cc4138a4ed59209b4729046993bbed5d2697101 Mon Sep 17 00:00:00 2001 From: Anay Wadhera Date: Fri, 28 Jun 2024 15:40:11 -0700 Subject: [PATCH] Make SetFallbackVisibility private and handle it correctly in settings. PiperOrigin-RevId: 647816713 --- sharing/nearby_sharing_service_impl.cc | 5 - sharing/nearby_sharing_settings.cc | 23 ++-- sharing/nearby_sharing_settings.h | 11 +- sharing/nearby_sharing_settings_test.cc | 144 +++++++++++++++++------- 4 files changed, 125 insertions(+), 58 deletions(-) diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index e995c266..05aa785d 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -4036,11 +4036,6 @@ void NearbySharingServiceImpl::ResetAllSettings(bool logout) { is_temporarily_visible) { settings_->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_HIDDEN); } - - // There is no valid fallback visibility when logged out until we support - // "off" as an option. - settings_->SetFallbackVisibility( - DeviceVisibility::DEVICE_VISIBILITY_EVERYONE); prefs::RegisterNearbySharingPrefs(preference_manager_, /*skip_persistent_ones=*/true); diff --git a/sharing/nearby_sharing_settings.cc b/sharing/nearby_sharing_settings.cc index 2f621eae..3b4641da 100644 --- a/sharing/nearby_sharing_settings.cc +++ b/sharing/nearby_sharing_settings.cc @@ -47,8 +47,6 @@ namespace nearby { namespace sharing { namespace { -using ::location::nearby::proto::sharing::DesktopNotification; -using ::location::nearby::proto::sharing::DesktopTransferEventType; using ::location::nearby::proto::sharing::ShowNotificationStatus; using ::nearby::sharing::api::PreferenceManager; using ::nearby::sharing::proto::DataUsage; @@ -159,17 +157,15 @@ void NearbyShareSettings::StartVisibilityTimer( NL_LOG(INFO) << __func__ << ": start visibility timer. expiration=" << expiration; visibility_expiration_timer_->Start( - expiration / absl::Milliseconds(1), 0, [this]() { + absl::ToInt64Milliseconds(expiration), 0, [this]() { NL_LOG(INFO) << __func__ << ": visibility timer expired."; - int visibility; + proto::DeviceVisibility visibility; { MutexLock lock(&mutex_); visibility_expiration_timer_->Stop(); - visibility = preference_manager_.GetInteger( - prefs::kNearbySharingBackgroundFallbackVisibilityName, - static_cast(prefs::kDefaultFallbackVisibility)); + visibility = GetFallbackVisibility(); } - SetVisibility(DeviceVisibility(visibility)); + SetVisibility(visibility); }); } @@ -363,8 +359,16 @@ void NearbyShareSettings::SetVisibility(DeviceVisibility visibility, preference_manager_.SetInteger( prefs::kNearbySharingBackgroundVisibilityExpirationSeconds, absl::ToUnixSeconds(fallback_visibility_timestamp)); + SetFallbackVisibility(last_visibility); StartVisibilityTimer(expiration); } else { + // Since our UI provides the option to go back to temporary everyone mode, + // we should only clear the fallback visibility when we are not in everyone + // mode. Once we fall back to a non-everyone mode visibility, we should + // clear the fallback visibility. + if (visibility != DeviceVisibility::DEVICE_VISIBILITY_EVERYONE) { + SetFallbackVisibility(DeviceVisibility::DEVICE_VISIBILITY_UNSPECIFIED); + } preference_manager_.SetInteger( prefs::kNearbySharingBackgroundVisibilityExpirationSeconds, 0); } @@ -389,8 +393,7 @@ proto::DeviceVisibility NearbyShareSettings::GetLastVisibility() const { DeviceVisibility NearbyShareSettings::GetFallbackVisibility() const { MutexLock lock(&mutex_); NL_VLOG(1) << __func__ << ": get fallback visibility called."; - return fallback_visibility_.has_value() ? *fallback_visibility_ - : prefs::kDefaultFallbackVisibility; + return fallback_visibility_.value_or(prefs::kDefaultFallbackVisibility); } void NearbyShareSettings::SetFallbackVisibility( diff --git a/sharing/nearby_sharing_settings.h b/sharing/nearby_sharing_settings.h index e1909866..5764f0f7 100644 --- a/sharing/nearby_sharing_settings.h +++ b/sharing/nearby_sharing_settings.h @@ -179,7 +179,6 @@ class NearbyShareSettings absl::Time GetLastVisibilityTimestamp() const; proto::DeviceVisibility GetLastVisibility() const; - proto::DeviceVisibility GetFallbackVisibility() const; bool GetIsTemporarilyVisible() const; void SetIsTemporarilyVisible(bool is_temporarily_visible) const; bool IsOnboardingComplete() const; @@ -208,10 +207,17 @@ class NearbyShareSettings std::function callback); void GetDataUsage(std::function callback); void SetDataUsage(proto::DataUsage data_usage); + proto::DeviceVisibility GetFallbackVisibility() const; void GetVisibility(std::function callback); + // Sets the visibility for the Nearby Sharing service. If the expiration is + // not zero, the visibility will be set temporarily and a fallback will be + // set. If the expiration is zero, the visibility will be set permanently. + // Note: When transitioning between temporary and permanent everyone mode + // visibility, the fallback visibility will not be cleared. However, when the + // temporary timer expires, the fallback visibility will be restored and + // cleared. void SetVisibility(proto::DeviceVisibility visibility, absl::Duration expiration = absl::ZeroDuration()) const; - void SetFallbackVisibility(proto::DeviceVisibility visibility) const; bool GetIsReceiving(); void SetIsReceiving(bool is_receiving) const; bool GetIsAnalyticsEnabled(); @@ -230,6 +236,7 @@ class NearbyShareSettings std::string Dump() const; private: + void SetFallbackVisibility(proto::DeviceVisibility visibility) const; void OnEnabledPrefChanged(); void OnFastInitiationNotificationStatePrefChanged(); void OnDataUsagePrefChanged(); diff --git a/sharing/nearby_sharing_settings_test.cc b/sharing/nearby_sharing_settings_test.cc index c0b39c48..8a2d0e52 100644 --- a/sharing/nearby_sharing_settings_test.cc +++ b/sharing/nearby_sharing_settings_test.cc @@ -41,9 +41,6 @@ namespace nearby { namespace sharing { namespace { - -using ::location::nearby::proto::sharing::DesktopNotification; -using ::location::nearby::proto::sharing::DesktopTransferEventType; using ::nearby::sharing::proto::DataUsage; using ::nearby::sharing::proto::DeviceVisibility; using ::nearby::sharing::proto::FastInitiationNotificationState; @@ -123,11 +120,6 @@ class FakeNearbyShareSettingsObserver : public NearbyShareSettings::Observer { return visibility_; } - const std::vector& allowed_contacts() { - absl::MutexLock lock(&mutex_); - return allowed_contacts_; - } - private: mutable absl::Mutex mutex_; @@ -143,7 +135,6 @@ class FakeNearbyShareSettingsObserver : public NearbyShareSettings::Observer { DataUsage data_usage_ ABSL_GUARDED_BY(mutex_) = DataUsage::UNKNOWN_DATA_USAGE; DeviceVisibility visibility_ ABSL_GUARDED_BY(mutex_) = DeviceVisibility::DEVICE_VISIBILITY_UNSPECIFIED; - std::vector allowed_contacts_ ABSL_GUARDED_BY(mutex_); }; class NearbyShareSettingsTest : public ::testing::Test { @@ -425,48 +416,119 @@ TEST_F(NearbyShareSettingsTest, GetAndSetVisibility) { EXPECT_EQ(visibility, DeviceVisibility::DEVICE_VISIBILITY_EVERYONE); } -TEST_F(NearbyShareSettingsTest, GetAndSetVisibilityWithSelectedContacts) { - EXPECT_EQ(observer_.visibility(), - DeviceVisibility::DEVICE_VISIBILITY_UNSPECIFIED); +TEST_F(NearbyShareSettingsTest, + SetTemporaryVisibilityExpiresAndRestoresOriginal) { + // Set our initial visibility to self share. + settings()->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); + // Set everyone mode temporarily. settings()->SetVisibility( - DeviceVisibility::DEVICE_VISIBILITY_SELECTED_CONTACTS); + DeviceVisibility::DEVICE_VISIBILITY_EVERYONE, + absl::Seconds(prefs::kDefaultMaxVisibilityExpirationSeconds)); + // Fast forward to the expiration time. + FastForward(absl::Seconds(prefs::kDefaultMaxVisibilityExpirationSeconds)); + // Verify that the visibility has expired and we are back to self share. EXPECT_EQ(settings()->GetVisibility(), DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); - Flush(); - EXPECT_EQ(observer_.visibility(), - DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); } -TEST_F(NearbyShareSettingsTest, GetFallbackVisibility) { - EXPECT_EQ(observer_.visibility(), +TEST_F(NearbyShareSettingsTest, + GetFallbackVisibilityReturnsUnspecifiedIfPermanent) { + // Set our initial visibility to self share. + settings()->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); + + // Since this is permanent, we shouldn't have a fallback. + EXPECT_EQ(settings()->GetFallbackVisibility(), DeviceVisibility::DEVICE_VISIBILITY_UNSPECIFIED); - DeviceVisibility visibility = settings()->GetFallbackVisibility(); - EXPECT_EQ(visibility, DeviceVisibility::DEVICE_VISIBILITY_HIDDEN); - settings()->SetFallbackVisibility( - DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS); +} + +TEST_F(NearbyShareSettingsTest, + TransitionToPermanentNonEveryoneClearsFallback) { + // Set our initial visibility to self share. + settings()->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); + // Set everyone mode temporarily. + settings()->SetVisibility( + DeviceVisibility::DEVICE_VISIBILITY_EVERYONE, + absl::Seconds(prefs::kDefaultMaxVisibilityExpirationSeconds)); + // Verify that the fallback visibility is valid and set to self share. + EXPECT_EQ(settings()->GetFallbackVisibility(), + DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); + // Transition to permanent self share. + settings()->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); + // Verify that the fallback visibility is unset. + EXPECT_EQ(settings()->GetFallbackVisibility(), + DeviceVisibility::DEVICE_VISIBILITY_UNSPECIFIED); +} + +TEST_F(NearbyShareSettingsTest, + TransitionToPermanentEveryoneDoesNotClearFallback) { + // Set our initial visibility to self share. + settings()->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); + // Transition to temporary everyone mode. + settings()->SetVisibility( + DeviceVisibility::DEVICE_VISIBILITY_EVERYONE, + absl::Seconds(prefs::kDefaultMaxVisibilityExpirationSeconds)); + // Transition to permanent everyone mode. settings()->SetVisibility(DeviceVisibility::DEVICE_VISIBILITY_EVERYONE); - EXPECT_EQ(settings()->GetVisibility(), - DeviceVisibility::DEVICE_VISIBILITY_EVERYONE); + // Verify that the fallback visibility is intact, since we can go back to + // temporary. EXPECT_EQ(settings()->GetFallbackVisibility(), - DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS); + DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); +} - settings()->SetFallbackVisibility( - DeviceVisibility::DEVICE_VISIBILITY_EVERYONE); - EXPECT_EQ(settings()->GetFallbackVisibility(), - DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS); - Flush(); - EXPECT_EQ(observer_.visibility(), +TEST(NearbyShareVisibilityTest, RestoresFallbackVisibility_ExpiredTimer) { + // Create Nearby Share settings dependencies. + FakeContext context; + FakeDeviceInfo fake_device_info; + FakePreferenceManager preference_manager; + FakeNearbyShareLocalDeviceDataManager local_device_data_manager( + kDefaultDeviceName); + // Set everyone mode temporarily. + preference_manager.SetInteger( + prefs::kNearbySharingBackgroundVisibilityName, + static_cast(DeviceVisibility::DEVICE_VISIBILITY_EVERYONE)); + // Set expiration to 10 seconds ago. + preference_manager.SetInteger( + prefs::kNearbySharingBackgroundVisibilityExpirationSeconds, + absl::ToUnixSeconds(context.GetClock()->Now() - absl::Seconds(10))); + // Set fallback visibility to self share. + preference_manager.SetInteger( + prefs::kNearbySharingBackgroundFallbackVisibilityName, + static_cast(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE)); + // Create a Nearby Share settings instance. + NearbyShareSettings settings(&context, context.GetClock(), fake_device_info, + preference_manager, &local_device_data_manager); + + // Make sure we restore the correct visibility. + EXPECT_EQ(settings.GetVisibility(), + DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE); +} + +TEST(NearbyShareVisibilityTest, RestoresFallbackVisibility_FutureTimer) { + // Create Nearby Share settings dependencies. + FakeContext context; + FakeDeviceInfo fake_device_info; + FakePreferenceManager preference_manager; + FakeNearbyShareLocalDeviceDataManager local_device_data_manager( + kDefaultDeviceName); + // Set everyone mode temporarily. + preference_manager.SetInteger( + prefs::kNearbySharingBackgroundVisibilityName, + static_cast(DeviceVisibility::DEVICE_VISIBILITY_EVERYONE)); + // Set expiration to 10 seconds in the future. + preference_manager.SetInteger( + prefs::kNearbySharingBackgroundVisibilityExpirationSeconds, + absl::ToUnixSeconds(context.GetClock()->Now() + absl::Seconds(10))); + // Set fallback visibility to self share. + preference_manager.SetInteger( + prefs::kNearbySharingBackgroundFallbackVisibilityName, + static_cast(DeviceVisibility::DEVICE_VISIBILITY_SELF_SHARE)); + // Create a Nearby Share settings instance. + NearbyShareSettings settings(&context, context.GetClock(), fake_device_info, + preference_manager, &local_device_data_manager); + + // Make sure we restore the correct visibility. + EXPECT_EQ(settings.GetVisibility(), DeviceVisibility::DEVICE_VISIBILITY_EVERYONE); - - settings()->SetVisibility(settings()->GetVisibility(), absl::Seconds(1)); - Flush(); - FastForward(absl::Seconds(1)); - Flush(); - visibility = DeviceVisibility::DEVICE_VISIBILITY_UNSPECIFIED; - settings()->GetVisibility( - [&visibility](DeviceVisibility result) { visibility = result; }); - EXPECT_EQ(visibility, DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS); - Flush(); } } // namespace