Use a single method to request proxy config refresh

Previously, `RefreshProxyListForGeoChange` was specifically for geo
changes, and did nothing if tokens are not cached per-geo. If the
minimum time had not elapsed, it scheduled a refresh for after that
minimum time, and otherwise performed the refresh immediately.

`RequestRefreshProxyList` refreshed if the minimum time had elapsed,
otherwise it did nothing.

The only call to `RefreshProxyListForGeoChange` already checks whether
tokens are cached per-geo, so that conditional was unnecessary. And, the
behavior of scheduling a refresh after the minimum time has elapsed is
always useful.

So, this CL combines the two methods into one.

Bug: 375253808
Change-Id: Ib125254e0b1befc81243fd154736b78927b23768
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5958196
Reviewed-by: Abhi Patel <abhipatel@chromium.org>
Commit-Queue: Dustin Mitchell <djmitche@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1374664}

components/ip_protection/common/ip_protection_core_impl.cc

@@ -244,7 +244,7 @@ void IpProtectionCoreImpl::GeoObserved(const std::string& geo_id) {
 
   if (ipp_proxy_config_manager_ != nullptr &&
       ipp_proxy_config_manager_->CurrentGeo() != geo_id) {
-    ipp_proxy_config_manager_->RefreshProxyListForGeoChange();
+    ipp_proxy_config_manager_->RequestRefreshProxyList();
   }
 
   for (auto& [_, token_manager] : ipp_token_managers_) {

components/ip_protection/common/ip_protection_core_impl_unittest.cc

@@ -108,16 +108,6 @@ class MockIpProtectionProxyConfigManager
     proxy_list_ = std::move(proxy_list);
   }
 
-  // Set the geo id returned from `CurrentGeo()`.
-  void RefreshProxyListForGeoChange() override {
-    if (on_force_refresh_proxy_list_) {
-      if (!geo_id_to_change_on_refresh_.empty()) {
-        geo_id_ = geo_id_to_change_on_refresh_;
-      }
-      std::move(on_force_refresh_proxy_list_).Run();
-    }
-  }
-
   void SetOnRequestRefreshProxyList(
       base::OnceClosure on_force_refresh_proxy_list,
       std::string geo_id = "") {

components/ip_protection/common/ip_protection_proxy_config_manager.h

@@ -37,13 +37,8 @@ class IpProtectionProxyConfigManager {
   // If token caching by geo is disabled, this will always return "EARTH".
   virtual const std::string& CurrentGeo() = 0;
 
-  // Requests a proxy list refresh when a geo change has occurred. This will
-  // either kick off an immediate refresh or schedule a refresh for the soonest
-  // possible time.
-  virtual void RefreshProxyListForGeoChange() = 0;
-
   // Request a refresh of the proxy list. Call this when it's likely that the
-  // proxy list is out of date.
+  // proxy list is out of date or needs an update due to a geo change.
   virtual void RequestRefreshProxyList() = 0;
 };
 

components/ip_protection/common/ip_protection_proxy_config_manager_impl.cc

@@ -91,11 +91,7 @@ const std::string& IpProtectionProxyConfigManagerImpl::CurrentGeo() {
   return current_geo_id_;
 }
 
-void IpProtectionProxyConfigManagerImpl::RefreshProxyListForGeoChange() {
-  if (!enable_token_caching_by_geo_) {
-    return;
-  }
-
+void IpProtectionProxyConfigManagerImpl::RequestRefreshProxyList() {
   if (IsProxyListOlderThanMinAge()) {
     RefreshProxyList();
     return;
@@ -110,15 +106,6 @@ void IpProtectionProxyConfigManagerImpl::RefreshProxyListForGeoChange() {
   ScheduleRefreshProxyList(delay.is_negative() ? base::TimeDelta() : delay);
 }
 
-void IpProtectionProxyConfigManagerImpl::RequestRefreshProxyList() {
-  // Do not refresh the list too frequently.
-  if (!IsProxyListOlderThanMinAge()) {
-    return;
-  }
-
-  RefreshProxyList();
-}
-
 void IpProtectionProxyConfigManagerImpl::RefreshProxyList() {
   if (fetching_proxy_list_) {
     return;

components/ip_protection/common/ip_protection_proxy_config_manager_impl.h

@@ -35,7 +35,6 @@ class IpProtectionProxyConfigManagerImpl
   bool IsProxyListAvailable() override;
   const std::vector<net::ProxyChain>& ProxyList() override;
   const std::string& CurrentGeo() override;
-  void RefreshProxyListForGeoChange() override;
   void RequestRefreshProxyList() override;
 
   // Set a callback to occur when the proxy list has been refreshed.

components/ip_protection/common/ip_protection_proxy_config_manager_impl_unittest.cc

@@ -140,7 +140,7 @@ class IpProtectionProxyConfigManagerImplTest : public testing::Test {
     ON_CALL(mock_core_, GeoObserved(testing::_))
         .WillByDefault([this](const std::string& geo_id) {
           if (ipp_proxy_list_->CurrentGeo() != geo_id) {
-            ipp_proxy_list_->RefreshProxyListForGeoChange();
+            ipp_proxy_list_->RequestRefreshProxyList();
           }
         });
 
@@ -584,22 +584,8 @@ TEST_F(IpProtectionProxyConfigManagerImplTest,
   ASSERT_EQ(ipp_proxy_list_->CurrentGeo(), kMountainViewGeoId);
 }
 
-// If the geo caching feature is disabled, setting the geo should have no effect
-// and should continue returning the default geo.
 TEST_F(IpProtectionProxyConfigManagerImplTest,
-       RefreshProxyListForGeoChangeCachingByGeoDisabledNoRefresh) {
-  SetUpIpProtectionProxyConfigManager(kDisableTokenCacheByGeo);
-
-  ASSERT_EQ(ipp_proxy_list_->CurrentGeo(), kDefaultGeoId);
-
-  ipp_proxy_list_->RefreshProxyListForGeoChange();
-
-  // A refresh does not occur since the feature is disabled.
-  ASSERT_TRUE(mock_.GotAllExpectedMockCalls());
-}
-
-TEST_F(IpProtectionProxyConfigManagerImplTest,
-       RefreshProxyListForGeoChangeCachingByGeoEnabledGeoChanged) {
+       RequestRefreshProxyListCachingByGeoEnabledGeoChanged) {
   SetUpIpProtectionProxyConfigManager(kEnableTokenCacheByGeo);
 
   // Current Geo is not set on initialization, so empty geo should be
@@ -616,7 +602,7 @@ TEST_F(IpProtectionProxyConfigManagerImplTest,
   ASSERT_EQ(ipp_proxy_list_->CurrentGeo(), kMountainViewGeoId);
 
   // Simulate `IpProtectionCore.GeoObserved` being called from
-  // outside this class which results in `RefreshProxyListForGeoChange` being
+  // outside this class which results in `RequestRefreshProxyList` being
   // called. Expected call will contain a different geo.
   expected_call = GetProxyConfigCall{
       .proxy_chains = std::vector{MakeChain({"a-proxy", "b-proxy"})},
@@ -629,7 +615,7 @@ TEST_F(IpProtectionProxyConfigManagerImplTest,
       net::features::kIpPrivacyProxyListMinFetchInterval.Get());
 
   QuitClosureOnRefresh();
-  ipp_proxy_list_->RefreshProxyListForGeoChange();
+  ipp_proxy_list_->RequestRefreshProxyList();
   WaitTillClosureQuit();
 
   ASSERT_TRUE(mock_.GotAllExpectedMockCalls());
@@ -637,10 +623,10 @@ TEST_F(IpProtectionProxyConfigManagerImplTest,
   ASSERT_EQ(ipp_proxy_list_->CurrentGeo(), kSunnyvaleGeoId);
 }
 
-// If `RefreshProxyListForGeoChange` is called multiple times, the refresh is
+// If `RequestRefreshProxyList` is called multiple times, the refresh is
 // only requested once within the default interval.
 TEST_F(IpProtectionProxyConfigManagerImplTest,
-       RefreshProxyListForGeoChangeCachingByGeoEnabledOnlyObservesGeo) {
+       RequestRefreshProxyListCachingByGeoEnabledOnlyObservesGeo) {
   SetUpIpProtectionProxyConfigManager(kEnableTokenCacheByGeo);
 
   GetProxyConfigCall expected_call{
@@ -649,8 +635,8 @@ TEST_F(IpProtectionProxyConfigManagerImplTest,
   mock_.ExpectGetProxyConfigCall(expected_call);
   QuitClosureOnRefresh();
 
-  ipp_proxy_list_->RefreshProxyListForGeoChange();
-  ipp_proxy_list_->RefreshProxyListForGeoChange();
+  ipp_proxy_list_->RequestRefreshProxyList();
+  ipp_proxy_list_->RequestRefreshProxyList();
   WaitTillClosureQuit();
 
   ASSERT_TRUE(mock_.GotAllExpectedMockCalls());