Clear device list if policy is turned off.

1) Listen to changes to both kAccessCodeCastEnabled and kAccessCodeCastDeviceDuration
2) When kAccessCodeCastDeviceDuration changes, destroys all of the existing timers, recreate new timers based on new duration / expire devices as necessary
3) When kAccessCodeCastEnabled switches state, if transitioning from enabled -> disabled, all timers are destroyed and all of the saved devices are cleared.

Bug: b/223249300
Change-Id: I764d938a687de736a8ac22a040c6304e82a60b5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3640408
Reviewed-by: Benjamin Zielinski <bzielinski@google.com>
Commit-Queue: George Benz <gbj@google.com>
Cr-Commit-Position: refs/heads/main@{#1002809}

chrome/browser/media/router/discovery/access_code/BUILD.gn

@@ -60,6 +60,7 @@ if (!is_android) {
       "//components/cast_channel:cast_channel",
       "//components/keyed_service/content:content",
       "//components/leveldb_proto:leveldb_proto",
+      "//components/pref_registry",
       "//components/prefs:prefs",
       "//components/signin/public/base:base",
       "//components/signin/public/identity_manager:identity_manager",

chrome/browser/media/router/discovery/access_code/access_code_cast_pref_updater.cc

@@ -143,6 +143,25 @@ void AccessCodeCastPrefUpdater::RemoveSinkIdFromDeviceAddedTimeDict(
   device_time_pref->Remove(sink_id);
 }
 
+void AccessCodeCastPrefUpdater::ClearDevicesDict() {
+  ScopedDictionaryPrefUpdate update(pref_service_,
+                                    prefs::kAccessCodeCastDevices);
+  std::unique_ptr<DictionaryValueUpdate> devices_pref = update.Get();
+  DCHECK(devices_pref) << "The " << prefs::kAccessCodeCastDevices
+                       << " pref does not exist.";
+  devices_pref->Clear();
+}
+
+void AccessCodeCastPrefUpdater::ClearDeviceAddedTimeDict() {
+  ScopedDictionaryPrefUpdate update(pref_service_,
+                                    prefs::kAccessCodeCastDeviceAdditionTime);
+
+  std::unique_ptr<DictionaryValueUpdate> device_time_pref = update.Get();
+  DCHECK(device_time_pref) << "The " << prefs::kAccessCodeCastDeviceAdditionTime
+                           << " pref does not exist.";
+  device_time_pref->Clear();
+}
+
 base::WeakPtr<AccessCodeCastPrefUpdater>
 AccessCodeCastPrefUpdater::GetWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();

chrome/browser/media/router/discovery/access_code/access_code_cast_pref_updater.h

@@ -64,6 +64,9 @@ class AccessCodeCastPrefUpdater {
   // not there in the first place.
   void RemoveSinkIdFromDeviceAddedTimeDict(const MediaSink::Id sink_id);
 
+  void ClearDevicesDict();
+  void ClearDeviceAddedTimeDict();
+
   base::WeakPtr<AccessCodeCastPrefUpdater> GetWeakPtr();
 
  private:

chrome/browser/media/router/discovery/access_code/access_code_cast_pref_updater_unittest.cc

@@ -170,4 +170,32 @@ TEST_F(AccessCodeCastPrefUpdaterTest, TestGetSinkIdsFromDevicesDict) {
   EXPECT_EQ(pref_updater()->GetSinkIdsFromDevicesDict(), expected_sink_ids);
 }
 
+TEST_F(AccessCodeCastPrefUpdaterTest, TestClearDevicesDict) {
+  MediaSinkInternal cast_sink = CreateCastSink(1);
+  MediaSinkInternal cast_sink2 = CreateCastSink(2);
+
+  pref_updater()->UpdateDevicesDict(cast_sink);
+  pref_updater()->UpdateDevicesDict(cast_sink2);
+
+  EXPECT_FALSE(pref_updater()->GetDevicesDict()->GetDict().empty());
+
+  pref_updater()->ClearDevicesDict();
+
+  EXPECT_TRUE(pref_updater()->GetDevicesDict()->GetDict().empty());
+}
+
+TEST_F(AccessCodeCastPrefUpdaterTest, TestClearDeviceAddedTimeDict) {
+  MediaSinkInternal cast_sink = CreateCastSink(1);
+  MediaSinkInternal cast_sink2 = CreateCastSink(2);
+
+  pref_updater()->UpdateDeviceAddedTimeDict(cast_sink.id());
+  pref_updater()->UpdateDeviceAddedTimeDict(cast_sink2.id());
+
+  EXPECT_FALSE(pref_updater()->GetDeviceAddedTimeDict()->GetDict().empty());
+
+  pref_updater()->ClearDeviceAddedTimeDict();
+
+  EXPECT_TRUE(pref_updater()->GetDeviceAddedTimeDict()->GetDict().empty());
+}
+
 }  // namespace media_router

chrome/browser/media/router/discovery/access_code/access_code_cast_sink_service.cc

@@ -48,7 +48,6 @@ using ChannelOpenedCallback = base::OnceCallback<void(bool)>;
 constexpr char kLoggerComponent[] = "AccessCodeCastSinkService";
 
 const base::TimeDelta kExpirationDelay = base::Milliseconds(250);
-const base::TimeDelta kExpirationTimerDelay = base::Seconds(20);
 
 }  // namespace
 
@@ -85,7 +84,7 @@ AccessCodeCastSinkService::AccessCodeCastSinkService(
       network_monitor_(network_monitor),
       prefs_(prefs) {
   DCHECK(profile_) << "The profile does not exist.";
-  DCHECK(prefs)
+  DCHECK(prefs_)
       << "Prefs could not be fetched from the profile for some reason.";
   backoff_policy_ = {
       // Number of initial errors (in sequence) to ignore before going into
@@ -119,9 +118,19 @@ AccessCodeCastSinkService::AccessCodeCastSinkService(
     // We don't need to post this task per the DiscoveryNetworkMonitor's
     // promise: "All observers will be notified of network changes on the thread
     // from which they registered."
-    pref_updater_ = std::make_unique<AccessCodeCastPrefUpdater>(prefs);
+    pref_updater_ = std::make_unique<AccessCodeCastPrefUpdater>(prefs_);
     network_monitor_->AddObserver(this);
     InitAllStoredDevices();
+    user_prefs_registrar_ = std::make_unique<PrefChangeRegistrar>();
+    user_prefs_registrar_->Init(prefs_);
+    user_prefs_registrar_->Add(
+        prefs::kAccessCodeCastDeviceDuration,
+        base::BindRepeating(&AccessCodeCastSinkService::OnDurationPrefChange,
+                            base::Unretained(this)));
+    user_prefs_registrar_->Add(
+        prefs::kAccessCodeCastEnabled,
+        base::BindRepeating(&AccessCodeCastSinkService::OnEnabledPrefChange,
+                            base::Unretained(this)));
   }
 }
 
@@ -424,21 +433,7 @@ void AccessCodeCastSinkService::StoreSinkInPrefs(
 }
 
 void AccessCodeCastSinkService::InitAllStoredDevices() {
-  auto sink_ids = FetchStoredDevices();
-  if (sink_ids.empty()) {
-    media_router_->GetLogger()->LogInfo(
-        mojom::LogCategory::kDiscovery, kLoggerComponent,
-        "There are no saved Access Code Cast devices for this profile.", "", "",
-        "");
-    return;
-  }
-  media_router_->GetLogger()->LogInfo(
-      mojom::LogCategory::kDiscovery, kLoggerComponent,
-      "Found Access Code Cast devices for this profile: " +
-          sink_ids.DebugString() +
-          ". Attempting to validate and then add these cast devices.",
-      "", "", "");
-  auto validated_devices = ValidateStoredDevices(sink_ids);
+  auto validated_devices = FetchAndValidateStoredDevices();
   if (validated_devices.empty()) {
     // We don't need anymore logging here since it is already handled in the
     // ValidateStoredDevices function.
@@ -499,7 +494,8 @@ void AccessCodeCastSinkService::SetExpirationTimer(
   // the sink is not removed before the route is created.
   expiration_timer->Start(
       FROM_HERE,
-      CalculateDurationTillExpiration(sink->id()) + kExpirationTimerDelay,
+      CalculateDurationTillExpiration(sink->id()) +
+          AccessCodeCastSinkService::kExpirationTimerDelay,
       base::BindOnce(&AccessCodeCastSinkService::OnExpiration,
                      weak_ptr_factory_.GetWeakPtr(), *sink));
 
@@ -569,6 +565,25 @@ AccessCodeCastSinkService::ValidateStoredDevices(
   return cast_sinks;
 }
 
+const std::vector<MediaSinkInternal>
+AccessCodeCastSinkService::FetchAndValidateStoredDevices() {
+  auto sink_ids = FetchStoredDevices();
+  if (sink_ids.empty()) {
+    media_router_->GetLogger()->LogInfo(
+        mojom::LogCategory::kDiscovery, kLoggerComponent,
+        "There are no saved Access Code Cast devices for this profile.", "", "",
+        "");
+    return {};
+  }
+  media_router_->GetLogger()->LogInfo(
+      mojom::LogCategory::kDiscovery, kLoggerComponent,
+      "Found Access Code Cast devices for this profile: " +
+          sink_ids.DebugString() +
+          ". Attempting to validate and then add these cast devices.",
+      "", "", "");
+  return ValidateStoredDevices(sink_ids);
+}
+
 void AccessCodeCastSinkService::AddStoredDevicesToMediaRouter(
     const std::vector<MediaSinkInternal> cast_sinks) {
   // Let the media router handle addition.
@@ -700,12 +715,30 @@ void AccessCodeCastSinkService::OnNetworksChanged(
   }
 }
 
+void AccessCodeCastSinkService::OnDurationPrefChange() {
+  ResetExpirationTimers();
+  InitExpirationTimers(FetchAndValidateStoredDevices());
+}
+
+void AccessCodeCastSinkService::OnEnabledPrefChange() {
+  if (!GetAccessCodeCastEnabledPref(prefs_)) {
+    RemoveExistingSinksOnNetwork();
+    ResetExpirationTimers();
+    pending_expirations_.clear();
+    pref_updater_->ClearDevicesDict();
+    pref_updater_->ClearDeviceAddedTimeDict();
+  }
+}
+
 void AccessCodeCastSinkService::Shutdown() {
   // There's no guarantee that MediaRouter is still in the
   // MediaRoutesObserver. |media_routes_observer_| accesses MediaRouter in its
   // dtor. Since MediaRouter and |this| are both KeyedServices, we must not
   // access MediaRouter in the dtor of |this|, so we do it here.
   media_routes_observer_.reset();
+  if (user_prefs_registrar_)
+    user_prefs_registrar_->RemoveAll();
+  user_prefs_registrar_.reset();
   ResetExpirationTimers();
 }
 

chrome/browser/media/router/discovery/access_code/access_code_cast_sink_service.h

@@ -20,6 +20,7 @@
 #include "components/media_router/browser/media_routes_observer.h"
 #include "components/media_router/common/discovery/media_sink_internal.h"
 #include "components/media_router/common/discovery/media_sink_service_base.h"
+#include "components/prefs/pref_change_registrar.h"
 #include "net/base/backoff_entry.h"
 
 namespace media_router {
@@ -71,6 +72,8 @@ class AccessCodeCastSinkService : public KeyedService,
 
   void StoreSinkAndSetExpirationTimer(const MediaSink::Id sink_id);
 
+  static constexpr base::TimeDelta kExpirationTimerDelay = base::Seconds(20);
+
  private:
   class AccessCodeMediaRoutesObserver : public MediaRoutesObserver {
    public:
@@ -153,6 +156,10 @@ class AccessCodeCastSinkService : public KeyedService,
                            TestResetExpirationTimersNetworkChange);
   FRIEND_TEST_ALL_PREFIXES(AccessCodeCastSinkServiceTest,
                            TestResetExpirationTimersShutdown);
+  FRIEND_TEST_ALL_PREFIXES(AccessCodeCastSinkServiceTest,
+                           TestChangeEnabledPref);
+  FRIEND_TEST_ALL_PREFIXES(AccessCodeCastSinkServiceTest,
+                           TestChangeDurationPref);
 
   // Constructor used for testing.
   AccessCodeCastSinkService(
@@ -200,6 +207,9 @@ class AccessCodeCastSinkService : public KeyedService,
   // this function could return an empty vector.
   const std::vector<MediaSinkInternal> ValidateStoredDevices(
       const base::Value::List& sink_ids);
+
+  const std::vector<MediaSinkInternal> FetchAndValidateStoredDevices();
+
   void AddStoredDevicesToMediaRouter(
       const std::vector<MediaSinkInternal> cast_sinks);
 
@@ -218,6 +228,9 @@ class AccessCodeCastSinkService : public KeyedService,
   // DiscoveryNetworkMonitor::Observer implementation
   void OnNetworksChanged(const std::string& network_id) override;
 
+  void OnDurationPrefChange();
+  void OnEnabledPrefChange();
+
   cast_channel::CastSocketOpenParams CreateCastSocketOpenParams(
       const MediaSinkInternal& sink);
 
@@ -271,6 +284,9 @@ class AccessCodeCastSinkService : public KeyedService,
 
   PrefService* prefs_;
 
+  // This registrar monitors for user prefs changes.
+  std::unique_ptr<PrefChangeRegistrar> user_prefs_registrar_;
+
   base::WeakPtrFactory<AccessCodeCastSinkService> weak_ptr_factory_{this};
 };
 

chrome/browser/media/router/discovery/access_code/access_code_cast_sink_service_unittest.cc

@@ -116,10 +116,10 @@ class AccessCodeCastSinkServiceTest : public testing::Test {
   }
 
   void TearDown() override {
-    profile_manager_->DeleteAllTestingProfiles();
-    profile_manager_.reset();
     access_code_cast_sink_service_->Shutdown();
     access_code_cast_sink_service_.reset();
+    profile_manager_->DeleteAllTestingProfiles();
+    profile_manager_.reset();
     router_.reset();
     pref_service_.reset();
     mock_time_task_runner()->ClearPendingTasks();
@@ -1003,4 +1003,78 @@ TEST_F(AccessCodeCastSinkServiceTest, TestResetExpirationTimersShutdown) {
   content::RunAllTasksUntilIdle();
 }
 
+TEST_F(AccessCodeCastSinkServiceTest, TestChangeEnabledPref) {
+  // Test to ensure that all existing sinks are removed, all timers are reset,
+  // and all prefs related to access code casting are removed.
+  SetDeviceDurationPrefForTest(base::Seconds(10000));
+  FastForwardUiAndIoTasks();
+
+  const MediaSinkInternal cast_sink1 = CreateCastSink(1);
+  access_code_cast_sink_service_->StoreSinkInPrefs(&cast_sink1);
+  mock_cast_media_sink_service_impl()->AddSinkForTest(cast_sink1);
+
+  FastForwardUiAndIoTasks();
+  access_code_cast_sink_service_->SetExpirationTimer(&cast_sink1);
+  EXPECT_TRUE(access_code_cast_sink_service_
+                  ->current_session_expiration_timers_[cast_sink1.id()]
+                  ->IsRunning());
+  EXPECT_FALSE(
+      access_code_cast_sink_service_->pref_updater_->GetDeviceAddedTimeDict()
+          ->GetDict()
+          .empty());
+  EXPECT_FALSE(access_code_cast_sink_service_->pref_updater_->GetDevicesDict()
+                   ->GetDict()
+                   .empty());
+
+  EXPECT_CALL(*mock_cast_media_sink_service_impl(),
+              DisconnectAndRemoveSink(cast_sink1));
+
+  pref_service_->SetUserPref(prefs::kAccessCodeCastEnabled, base::Value(false));
+
+  EXPECT_TRUE(access_code_cast_sink_service_->current_session_expiration_timers_
+                  .empty());
+  EXPECT_TRUE(
+      access_code_cast_sink_service_->pref_updater_->GetDeviceAddedTimeDict()
+          ->GetDict()
+          .empty());
+  EXPECT_TRUE(access_code_cast_sink_service_->pref_updater_->GetDevicesDict()
+                  ->GetDict()
+                  .empty());
+  FastForwardUiAndIoTasks();
+  content::RunAllTasksUntilIdle();
+  mock_time_task_runner_->FastForwardUntilNoTasksRemain();
+}
+
+TEST_F(AccessCodeCastSinkServiceTest, TestChangeDurationPref) {
+  // Test to ensure timers are reset whenever the duration pref changes.
+  SetDeviceDurationPrefForTest(base::Seconds(10000));
+  FastForwardUiAndIoTasks();
+
+  const MediaSinkInternal cast_sink1 = CreateCastSink(1);
+  access_code_cast_sink_service_->StoreSinkInPrefs(&cast_sink1);
+
+  FastForwardUiAndIoTasks();
+  access_code_cast_sink_service_->SetExpirationTimer(&cast_sink1);
+  EXPECT_TRUE(access_code_cast_sink_service_
+                  ->current_session_expiration_timers_[cast_sink1.id()]
+                  ->IsRunning());
+  EXPECT_EQ(
+      base::Seconds(10000) + AccessCodeCastSinkService::kExpirationTimerDelay,
+      access_code_cast_sink_service_
+          ->current_session_expiration_timers_[cast_sink1.id()]
+          ->GetCurrentDelay());
+
+  pref_service_->SetUserPref(prefs::kAccessCodeCastDeviceDuration,
+                             base::Value(100));
+
+  EXPECT_TRUE(access_code_cast_sink_service_
+                  ->current_session_expiration_timers_[cast_sink1.id()]
+                  ->IsRunning());
+  EXPECT_EQ(
+      base::Seconds(100) + AccessCodeCastSinkService::kExpirationTimerDelay,
+      access_code_cast_sink_service_
+          ->current_session_expiration_timers_[cast_sink1.id()]
+          ->GetCurrentDelay());
+}
+
 }  // namespace media_router