Only ensure _successful_ proxy config fetches are not too frequent

This check is to ensure that when signals suggest the proxy config is
out of date (such as mismatched geos), we don't constantly try to update
it. Handling for backoff on error is done in the fetcher. So when
there's an error, we can fallback to that backoff.

This also fixes the case where, on startup, the user is not signed in
(causing proxy config fetch to fail), but sign-in occurs before the
minimum time. This is only an issue when using OAuth tokens for
ProxyConfig, which is only done for debugging/testing purposes.

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

components/ip_protection/common/ip_protection_proxy_config_manager_impl.cc

@@ -100,7 +100,7 @@ void IpProtectionProxyConfigManagerImpl::RequestRefreshProxyList() {
   // If list is not older than min interval, schedule refresh as soon as
   // possible.
   base::TimeDelta time_since_last_refresh =
-      base::Time::Now() - last_proxy_list_refresh_;
+      base::Time::Now() - last_successful_proxy_list_refresh_;
 
   base::TimeDelta delay = proxy_list_min_age_ - time_since_last_refresh;
   ScheduleRefreshProxyList(delay.is_negative() ? base::TimeDelta() : delay);
@@ -112,7 +112,7 @@ void IpProtectionProxyConfigManagerImpl::RefreshProxyList() {
   }
 
   fetching_proxy_list_ = true;
-  last_proxy_list_refresh_ = base::Time::Now();
+  last_successful_proxy_list_refresh_ = base::Time::Now();
   const base::TimeTicks refresh_start_time_for_metrics = base::TimeTicks::Now();
 
   config_getter_->GetProxyConfig(base::BindOnce(
@@ -146,6 +146,12 @@ void IpProtectionProxyConfigManagerImpl::OnGotProxyList(
       current_geo_id_ = GetGeoIdFromGeoHint(std::move(geo_hint));
       ip_protection_core_->GeoObserved(current_geo_id_);
     }
+  } else {
+    // The request was not successful, so do not count this toward the
+    // minimum time between refreshes. Note that the proxy config fetcher
+    // implements its own backoff on errors, so this does not risk overwhelming
+    // the server.
+    last_successful_proxy_list_refresh_ = base::Time();
   }
 
   base::TimeDelta fuzzed_proxy_list_refresh_interval =
@@ -172,7 +178,8 @@ base::TimeDelta IpProtectionProxyConfigManagerImpl::FuzzProxyListFetchInterval(
 }
 
 bool IpProtectionProxyConfigManagerImpl::IsProxyListOlderThanMinAge() const {
-  return base::Time::Now() - last_proxy_list_refresh_ >= proxy_list_min_age_;
+  return base::Time::Now() - last_successful_proxy_list_refresh_ >=
+         proxy_list_min_age_;
 }
 
 void IpProtectionProxyConfigManagerImpl::

components/ip_protection/common/ip_protection_proxy_config_manager_impl.h

@@ -97,8 +97,8 @@ class IpProtectionProxyConfigManagerImpl
   // Source of proxy list, when needed.
   raw_ref<IpProtectionConfigGetter> config_getter_;
 
-  // The last time this instance began refreshing the proxy list.
-  base::Time last_proxy_list_refresh_;
+  // The last time this instance began refreshing the proxy list successfully.
+  base::Time last_successful_proxy_list_refresh_;
 
   // The min age of the proxy list before an additional refresh is allowed.
   const base::TimeDelta proxy_list_min_age_;

components/ip_protection/common/ip_protection_proxy_config_manager_impl_unittest.cc

@@ -301,6 +301,38 @@ TEST_F(IpProtectionProxyConfigManagerImplTest,
   EXPECT_EQ(ipp_proxy_list_->CurrentGeo(), kMountainViewGeoId);
 }
 
+// If a failure occurs, the minimum time between fetches does not apply.
+TEST_F(IpProtectionProxyConfigManagerImplTest,
+       ProxyListMinTimeIgnoredWhenRefreshFails) {
+  SetUpIpProtectionProxyConfigManager(kEnableTokenCacheByGeo);
+
+  // Called once for the proxy list refresh after the first refresh fails.
+  EXPECT_CALL(mock_core_, GeoObserved(testing::_)).Times(1);
+
+  mock_.ExpectGetProxyConfigCallFailure();
+  QuitClosureOnRefresh();
+  ipp_proxy_list_->RequestRefreshProxyList();
+  WaitTillClosureQuit();
+  ASSERT_TRUE(mock_.GotAllExpectedMockCalls());
+
+  // First refresh was failure, but next refresh should occur when requested.
+  EXPECT_FALSE(ipp_proxy_list_->IsProxyListAvailable());
+
+  GetProxyConfigCall expected_call =
+      GetProxyConfigCall{.proxy_chains = std::vector{MakeChain({"b-proxy"})},
+                         .geo_id = kMountainViewGeoId};
+  mock_.ExpectGetProxyConfigCall(expected_call);
+  QuitClosureOnRefresh();
+  ipp_proxy_list_->RequestRefreshProxyList();
+  WaitTillClosureQuit();
+  ASSERT_TRUE(mock_.GotAllExpectedMockCalls());
+
+  // After a successful call, the proxy list should be available.
+  EXPECT_TRUE(ipp_proxy_list_->IsProxyListAvailable());
+  EXPECT_EQ(ipp_proxy_list_->ProxyList(), expected_call.proxy_chains);
+  EXPECT_EQ(ipp_proxy_list_->CurrentGeo(), kMountainViewGeoId);
+}
+
 // The manager refreshes the proxy list on demand, but only once even if
 // `RequestRefreshProxyList()` is called repeatedly.
 TEST_F(IpProtectionProxyConfigManagerImplTest,