Migrate password_manager::AffiliationFetchThrottler to NetworkConnectionTracker

This migrates password_manager::AffiliationFetchThrottler from
NetworkChangeManager to NetworkConnectionTracker, which works with the
network service enabled.

Bug: 887058
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I04c2529e24ea0bf85ebb0646127ded2b594306c2
Reviewed-on: https://chromium-review.googlesource.com/1239359Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594783}

chrome/browser/password_manager/password_store_factory.cc

@@ -37,6 +37,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/storage_partition.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
@@ -117,7 +118,7 @@ void PasswordStoreFactory::OnPasswordsSyncedStatePotentiallyChanged(
       password_store.get(), sync_service,
       content::BrowserContext::GetDefaultStoragePartition(profile)
           ->GetURLLoaderFactoryForBrowserProcess(),
-      profile->GetPath());
+      content::GetNetworkConnectionTracker(), profile->GetPath());
 }
 
 PasswordStoreFactory::PasswordStoreFactory()

components/password_manager/core/browser/android_affiliation/affiliation_backend.cc

@@ -44,11 +44,12 @@ AffiliationBackend::~AffiliationBackend() {
 void AffiliationBackend::Initialize(
     std::unique_ptr<network::SharedURLLoaderFactoryInfo>
         url_loader_factory_info,
+    network::NetworkConnectionTracker* network_connection_tracker,
     const base::FilePath& db_path) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!throttler_);
-  throttler_.reset(
-      new AffiliationFetchThrottler(this, task_runner_, tick_clock_));
+  throttler_.reset(new AffiliationFetchThrottler(
+      this, task_runner_, network_connection_tracker, tick_clock_));
 
   // TODO(engedy): Currently, when Init() returns false, it always poisons the
   // DB, so subsequent operations will silently fail. Consider either fully

components/password_manager/core/browser/android_affiliation/affiliation_backend.h

@@ -33,6 +33,7 @@ class Time;
 }  // namespace base
 
 namespace network {
+class NetworkConnectionTracker;
 class SharedURLLoaderFactoryInfo;
 }  // namespace network
 
@@ -71,6 +72,7 @@ class AffiliationBackend : public FacetManagerHost,
   // affiliation information locally will be opened/created at |db_path|.
   void Initialize(std::unique_ptr<network::SharedURLLoaderFactoryInfo>
                       url_loader_factory_info,
+                  network::NetworkConnectionTracker* network_connection_tracker,
                   const base::FilePath& db_path);
 
   // Implementations for methods of the same name in AffiliationService. They

components/password_manager/core/browser/android_affiliation/affiliation_backend_unittest.cc

@@ -26,6 +26,7 @@
 #include "net/url_request/url_request_context_getter.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_network_connection_tracker.h"
 #include "services/network/test/test_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -44,7 +45,11 @@ class MockAffiliationFetchThrottler : public AffiliationFetchThrottler {
  public:
   explicit MockAffiliationFetchThrottler(
       AffiliationFetchThrottlerDelegate* delegate)
-      : AffiliationFetchThrottler(delegate, nullptr, nullptr),
+      : AffiliationFetchThrottler(
+            delegate,
+            nullptr,
+            network::TestNetworkConnectionTracker::GetInstance(),
+            nullptr),
         signaled_network_request_needed_(false) {
     EXPECT_CALL(*this, OnInformOfNetworkRequestComplete(testing::_)).Times(0);
   }
@@ -342,7 +347,9 @@ class AffiliationBackendTest : public testing::Test {
     auto test_shared_loader_factory =
         base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
             &test_url_loader_factory_);
-    backend_->Initialize(test_shared_loader_factory->Clone(), db_path());
+    backend_->Initialize(test_shared_loader_factory->Clone(),
+                         network::TestNetworkConnectionTracker::GetInstance(),
+                         db_path());
     auto mock_fetch_throttler =
         std::make_unique<MockAffiliationFetchThrottler>(backend_.get());
     mock_fetch_throttler_ = mock_fetch_throttler.get();

components/password_manager/core/browser/android_affiliation/affiliation_fetch_throttler.cc

@@ -52,9 +52,11 @@ const int64_t AffiliationFetchThrottler::kGracePeriodAfterReconnectMs =
 AffiliationFetchThrottler::AffiliationFetchThrottler(
     AffiliationFetchThrottlerDelegate* delegate,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner,
+    network::NetworkConnectionTracker* network_connection_tracker,
     const base::TickClock* tick_clock)
     : delegate_(delegate),
       task_runner_(task_runner),
+      network_connection_tracker_(network_connection_tracker),
       tick_clock_(tick_clock),
       state_(IDLE),
       has_network_connectivity_(false),
@@ -64,12 +66,12 @@ AffiliationFetchThrottler::AffiliationFetchThrottler(
   DCHECK(delegate);
   // Start observing before querying the current connectivity state, so that if
   // the state changes concurrently in-between, it will not go unnoticed.
-  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
-  has_network_connectivity_ = !net::NetworkChangeNotifier::IsOffline();
+  network_connection_tracker_->AddNetworkConnectionObserver(this);
+  has_network_connectivity_ = !network_connection_tracker_->IsOffline();
 }
 
 AffiliationFetchThrottler::~AffiliationFetchThrottler() {
-  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+  network_connection_tracker_->RemoveNetworkConnectionObserver(this);
 }
 
 void AffiliationFetchThrottler::SignalNetworkRequestNeeded() {
@@ -121,11 +123,14 @@ void AffiliationFetchThrottler::OnBackoffDelayExpiredCallback() {
     state_ = delegate_->OnCanSendNetworkRequest() ? FETCH_IN_FLIGHT : IDLE;
 }
 
-void AffiliationFetchThrottler::OnNetworkChanged(
-    net::NetworkChangeNotifier::ConnectionType type) {
+void AffiliationFetchThrottler::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
   bool old_has_network_connectivity = has_network_connectivity_;
-  has_network_connectivity_ =
-      (type != net::NetworkChangeNotifier::CONNECTION_NONE);
+  // We reread the connection type here instead of relying on |type| because
+  // NetworkConnectionTracker will call this function an extra time with
+  // CONNECTION_NONE whenever the connection changes to an online state, even
+  // if it was already in a different online state.
+  has_network_connectivity_ = !network_connection_tracker_->IsOffline();
 
   // Only react when network connectivity has been reestablished.
   if (!has_network_connectivity_ || old_has_network_connectivity)

components/password_manager/core/browser/android_affiliation/affiliation_fetch_throttler.h

@@ -14,7 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "net/base/backoff_entry.h"
-#include "net/base/network_change_notifier.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 
 namespace base {
 class TickClock;
@@ -50,7 +50,7 @@ class AffiliationFetchThrottlerDelegate;
 // periods, so that requests will not be held back for too long after
 // connectivity is restored.
 class AffiliationFetchThrottler
-    : public net::NetworkChangeNotifier::NetworkChangeObserver {
+    : public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   // Creates an instance that will use |tick_clock| as its tick source, and will
   // post to |task_runner| to call the |delegate|'s OnSendNetworkRequest(). The
@@ -58,6 +58,7 @@ class AffiliationFetchThrottler
   AffiliationFetchThrottler(
       AffiliationFetchThrottlerDelegate* delegate,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner,
+      network::NetworkConnectionTracker* network_connection_tracker,
       const base::TickClock* tick_clock);
   ~AffiliationFetchThrottler() override;
 
@@ -114,11 +115,11 @@ class AffiliationFetchThrottler
   // Called back when the |exponential_backoff_| delay expires.
   void OnBackoffDelayExpiredCallback();
 
-  // net::NetworkChangeNotifier::NetworkChangeObserver:
-  void OnNetworkChanged(
-      net::NetworkChangeNotifier::ConnectionType type) override;
+  // network::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
 
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  network::NetworkConnectionTracker* network_connection_tracker_;
   const base::TickClock* tick_clock_;
   State state_;
   bool has_network_connectivity_;

components/password_manager/core/browser/android_affiliation/affiliation_fetch_throttler_unittest.cc

@@ -19,7 +19,7 @@
 #include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "components/password_manager/core/browser/android_affiliation/affiliation_fetch_throttler_delegate.h"
-#include "net/base/network_change_notifier.h"
+#include "services/network/test/test_network_connection_tracker.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace password_manager {
@@ -64,20 +64,24 @@ class MockAffiliationFetchThrottlerDelegate
 class AffiliationFetchThrottlerTest : public testing::Test {
  public:
   AffiliationFetchThrottlerTest()
-      : network_change_notifier_(net::NetworkChangeNotifier::CreateMock()),
-        task_runner_(new base::TestMockTimeTaskRunner),
-        mock_delegate_(task_runner_->GetMockTickClock()) {}
+      : task_runner_(new base::TestMockTimeTaskRunner),
+        mock_delegate_(task_runner_->GetMockTickClock()) {
+    SimulateHasNetworkConnectivity(true);
+  }
+
   ~AffiliationFetchThrottlerTest() override {}
 
   std::unique_ptr<AffiliationFetchThrottler> CreateThrottler() {
     return std::make_unique<AffiliationFetchThrottler>(
-        &mock_delegate_, task_runner_, task_runner_->GetMockTickClock());
+        &mock_delegate_, task_runner_,
+        network::TestNetworkConnectionTracker::GetInstance(),
+        task_runner_->GetMockTickClock());
   }
 
   void SimulateHasNetworkConnectivity(bool has_connectivity) {
-    net::NetworkChangeNotifier::NotifyObserversOfConnectionTypeChangeForTests(
-        has_connectivity ? net::NetworkChangeNotifier::CONNECTION_UNKNOWN
-                         : net::NetworkChangeNotifier::CONNECTION_NONE);
+    network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+        has_connectivity ? network::mojom::ConnectionType::CONNECTION_ETHERNET
+                         : network::mojom::ConnectionType::CONNECTION_NONE);
     scoped_task_environment_.RunUntilIdle();
   }
 
@@ -121,10 +125,9 @@ class AffiliationFetchThrottlerTest : public testing::Test {
   }
 
  private:
-  // Needed because NetworkChangeNotifier uses base::ObserverList, which
+  // Needed because NetworkConnectionTracker uses base::ObserverList, which
   // notifies observers on the sequence from which they have registered.
   base::test::ScopedTaskEnvironment scoped_task_environment_;
-  std::unique_ptr<net::NetworkChangeNotifier> network_change_notifier_;
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
   MockAffiliationFetchThrottlerDelegate mock_delegate_;
 

components/password_manager/core/browser/android_affiliation/affiliation_service.cc

@@ -35,6 +35,7 @@ AffiliationService::~AffiliationService() {
 
 void AffiliationService::Initialize(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    network::NetworkConnectionTracker* network_connection_tracker,
     const base::FilePath& db_path) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!backend_);
@@ -43,9 +44,10 @@ void AffiliationService::Initialize(
                                     base::DefaultTickClock::GetInstance());
 
   backend_task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(&AffiliationBackend::Initialize,
-                                base::Unretained(backend_),
-                                url_loader_factory->Clone(), db_path));
+      FROM_HERE,
+      base::BindOnce(&AffiliationBackend::Initialize,
+                     base::Unretained(backend_), url_loader_factory->Clone(),
+                     base::Unretained(network_connection_tracker), db_path));
 }
 
 void AffiliationService::GetAffiliationsAndBranding(

components/password_manager/core/browser/android_affiliation/affiliation_service.h

@@ -21,6 +21,7 @@ class SequencedTaskRunner;
 }  // namespace base
 
 namespace network {
+class NetworkConnectionTracker;
 class SharedURLLoaderFactory;
 }  // namespace network
 
@@ -107,6 +108,7 @@ class AffiliationService : public KeyedService {
   // thread corresponding to |backend_task_runner_|.
   void Initialize(
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      network::NetworkConnectionTracker* network_connection_tracker,
       const base::FilePath& db_path);
 
   // Looks up facets affiliated with the facet identified by |facet_uri| and

components/password_manager/core/browser/android_affiliation/affiliation_service_unittest.cc

@@ -20,6 +20,7 @@
 #include "components/password_manager/core/browser/android_affiliation/mock_affiliation_consumer.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_network_connection_tracker.h"
 #include "services/network/test/test_url_loader_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -79,8 +80,13 @@ class AffiliationServiceTest : public testing::Test {
   void SetUp() override {
     base::FilePath database_path;
     ASSERT_TRUE(CreateTemporaryFile(&database_path));
+    network::TestNetworkConnectionTracker* network_connection_tracker =
+        network::TestNetworkConnectionTracker::GetInstance();
+    network_connection_tracker->SetConnectionType(
+        network::mojom::ConnectionType::CONNECTION_ETHERNET);
     service_.reset(new AffiliationService(background_task_runner()));
-    service_->Initialize(test_shared_loader_factory_, database_path);
+    service_->Initialize(test_shared_loader_factory_,
+                         network_connection_tracker, database_path);
     // Note: the background task runner is purposely not pumped here, so that
     // the tests also verify that the service can be used synchronously right
     // away after having been constructed.

components/password_manager/core/browser/password_store_factory_util.cc

@@ -30,6 +30,7 @@ bool ShouldAffiliationBasedMatchingBeActive(syncer::SyncService* sync_service) {
 void ActivateAffiliationBasedMatching(
     PasswordStore* password_store,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    network::NetworkConnectionTracker* network_connection_tracker,
     const base::FilePath& db_path) {
   // Subsequent instances of the AffiliationService must use the same sequenced
   // task runner for their backends. This guarantees that the backend of the
@@ -46,7 +47,8 @@ void ActivateAffiliationBasedMatching(
   // is owned by the PasswordStore.
   std::unique_ptr<AffiliationService> affiliation_service(
       new AffiliationService(backend_task_runner));
-  affiliation_service->Initialize(std::move(url_loader_factory), db_path);
+  affiliation_service->Initialize(std::move(url_loader_factory),
+                                  network_connection_tracker, db_path);
   std::unique_ptr<AffiliatedMatchHelper> affiliated_match_helper(
       new AffiliatedMatchHelper(password_store,
                                 std::move(affiliation_service)));
@@ -67,6 +69,7 @@ void ToggleAffiliationBasedMatchingBasedOnPasswordSyncedState(
     PasswordStore* password_store,
     syncer::SyncService* sync_service,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    network::NetworkConnectionTracker* network_connection_tracker,
     const base::FilePath& profile_path) {
   DCHECK(password_store);
 
@@ -76,9 +79,9 @@ void ToggleAffiliationBasedMatchingBasedOnPasswordSyncedState(
       password_store->affiliated_match_helper() != nullptr;
 
   if (matching_should_be_active && !matching_is_active) {
-    ActivateAffiliationBasedMatching(password_store,
-                                     std::move(url_loader_factory),
-                                     GetAffiliationDatabasePath(profile_path));
+    ActivateAffiliationBasedMatching(
+        password_store, std::move(url_loader_factory),
+        network_connection_tracker, GetAffiliationDatabasePath(profile_path));
   } else if (!matching_should_be_active && matching_is_active) {
     password_store->SetAffiliatedMatchHelper(nullptr);
   }

components/password_manager/core/browser/password_store_factory_util.h

@@ -17,6 +17,10 @@
 #include "components/sync/model/syncable_service.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
+namespace network {
+class NetworkConnectionTracker;
+}  // namespace network
+
 namespace password_manager {
 
 // Activates or deactivates affiliation-based matching for |password_store|,
@@ -28,6 +32,7 @@ void ToggleAffiliationBasedMatchingBasedOnPasswordSyncedState(
     PasswordStore* password_store,
     syncer::SyncService* sync_service,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    network::NetworkConnectionTracker* network_connection_tracker,
     const base::FilePath& profile_path);
 
 // Creates a LoginDatabase. Looks in |profile_path| for the database file.

ios/chrome/browser/passwords/ios_chrome_password_store_factory.cc

@@ -23,6 +23,7 @@
 #include "components/password_manager/core/browser/password_store_default.h"
 #include "components/password_manager/core/browser/password_store_factory_util.h"
 #include "components/sync/driver/sync_service.h"
+#include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/browser_state_otr_helper.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/sync/glue/sync_start_util.h"
@@ -60,6 +61,7 @@ void IOSChromePasswordStoreFactory::OnPasswordsSyncedStatePotentiallyChanged(
   password_manager::ToggleAffiliationBasedMatchingBasedOnPasswordSyncedState(
       password_store.get(), sync_service,
       browser_state->GetSharedURLLoaderFactory(),
+      GetApplicationContext()->GetNetworkConnectionTracker(),
       browser_state->GetStatePath());
 }
 

ios/web_view/internal/passwords/web_view_password_store_factory.mm

@@ -22,6 +22,7 @@
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_store_default.h"
 #include "components/password_manager/core/browser/password_store_factory_util.h"
+#include "ios/web_view/internal/app/application_context.h"
 #import "ios/web_view/internal/sync/web_view_profile_sync_service_factory.h"
 #include "ios/web_view/internal/web_view_browser_state.h"
 #include "ios/web_view/internal/webdata_services/web_view_web_data_service_wrapper_factory.h"
@@ -63,6 +64,7 @@ void WebViewPasswordStoreFactory::OnPasswordsSyncedStatePotentiallyChanged(
   password_manager::ToggleAffiliationBasedMatchingBasedOnPasswordSyncedState(
       password_store.get(), sync_service,
       browser_state->GetSharedURLLoaderFactory(),
+      ApplicationContext::GetInstance()->GetNetworkConnectionTracker(),
       browser_state->GetStatePath());
 }