Migrate ReadingListDownloadService to NetworkConnectionTracker

This migrates ReadingListDownloadService from NetworkChangeNotifier to
NetworkConnectionTracker, which works with the network service enabled.

Bug: 887057
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs;luci.chromium.try:linux_mojo
Change-Id: Ic86d3bca9b77a53f97e9039fa35369a8fa9968b1
Reviewed-on: https://chromium-review.googlesource.com/1239339Reviewed-by: Eric Noyau <noyau@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594420}

ios/chrome/browser/reading_list/reading_list_download_service.cc

@@ -17,6 +17,7 @@
 #include "components/reading_list/core/offline_url_utils.h"
 #include "components/reading_list/core/reading_list_entry.h"
 #include "components/reading_list/core/reading_list_model.h"
+#include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/reading_list/reading_list_distiller_page_factory.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
@@ -56,6 +57,7 @@ void CleanUpFiles(base::FilePath root,
     }
   }
 }
+
 }  // namespace
 
 ReadingListDownloadService::ReadingListDownloadService(
@@ -68,7 +70,8 @@ ReadingListDownloadService::ReadingListDownloadService(
         distiller_page_factory)
     : reading_list_model_(reading_list_model),
       chrome_profile_path_(chrome_profile_path),
-      had_connection_(!net::NetworkChangeNotifier::IsOffline()),
+      had_connection_(
+          !GetApplicationContext()->GetNetworkConnectionTracker()->IsOffline()),
       distiller_page_factory_(std::move(distiller_page_factory)),
       distiller_factory_(std::move(distiller_factory)),
       weak_ptr_factory_(this) {
@@ -81,11 +84,16 @@ ReadingListDownloadService::ReadingListDownloadService(
                  base::Unretained(this)),
       base::Bind(&ReadingListDownloadService::OnDeleteEnd,
                  base::Unretained(this)));
-  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
+
+  GetApplicationContext()
+      ->GetNetworkConnectionTracker()
+      ->AddNetworkConnectionObserver(this);
 }
 
 ReadingListDownloadService::~ReadingListDownloadService() {
-  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
+  GetApplicationContext()
+      ->GetNetworkConnectionTracker()
+      ->RemoveNetworkConnectionObserver(this);
 }
 
 void ReadingListDownloadService::Initialize() {
@@ -200,7 +208,7 @@ void ReadingListDownloadService::DownloadEntry(const GURL& url) {
       entry->DistilledState() == ReadingListEntry::PROCESSED || entry->IsRead())
     return;
 
-  if (net::NetworkChangeNotifier::IsOffline()) {
+  if (GetApplicationContext()->GetNetworkConnectionTracker()->IsOffline()) {
     // There is no connection, save it for download only if we did not exceed
     // the maximaxum number of tries.
     if (entry->FailedDownloadCounter() < kNumberOfFailsBeforeWifiOnly)
@@ -219,8 +227,15 @@ void ReadingListDownloadService::DownloadEntry(const GURL& url) {
 
   } else if (entry->FailedDownloadCounter() < kNumberOfFailsBeforeStop) {
     // Try to download the page only if the connection is wifi.
-    if (net::NetworkChangeNotifier::GetConnectionType() ==
-        net::NetworkChangeNotifier::CONNECTION_WIFI) {
+    auto connection_type = network::mojom::ConnectionType::CONNECTION_UNKNOWN;
+    // GetConnectionType will return false if the type isn't known yet, and
+    // connection_type will be unchanged, so we can ignore the return value and
+    // let this treat the connection as non-wifi.
+    GetApplicationContext()->GetNetworkConnectionTracker()->GetConnectionType(
+        &connection_type,
+        base::BindOnce(&ReadingListDownloadService::OnConnectionChanged,
+                       weak_ptr_factory_.GetWeakPtr()));
+    if (connection_type == network::mojom::ConnectionType::CONNECTION_WIFI) {
       // The connection is wifi, download the page.
       reading_list_model_->SetEntryDistilledState(entry->URL(),
                                                   ReadingListEntry::PROCESSING);
@@ -294,9 +309,9 @@ void ReadingListDownloadService::OnDeleteEnd(const GURL& url, bool success) {
   // Nothing to update as this is only called when deleting reading list entries
 }
 
-void ReadingListDownloadService::OnNetworkChanged(
-    net::NetworkChangeNotifier::ConnectionType type) {
-  if (type == net::NetworkChangeNotifier::CONNECTION_NONE) {
+void ReadingListDownloadService::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
+  if (type == network::mojom::ConnectionType::CONNECTION_NONE) {
     had_connection_ = false;
     return;
   }
@@ -307,7 +322,7 @@ void ReadingListDownloadService::OnNetworkChanged(
       ScheduleDownloadEntry(url);
     }
   }
-  if (type == net::NetworkChangeNotifier::CONNECTION_WIFI) {
+  if (type == network::mojom::ConnectionType::CONNECTION_WIFI) {
     for (auto& url : url_to_download_wifi_) {
       ScheduleDownloadEntry(url);
     }

ios/chrome/browser/reading_list/reading_list_download_service.h

@@ -9,7 +9,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/reading_list/core/reading_list_model_observer.h"
 #include "ios/chrome/browser/reading_list/url_downloader.h"
-#include "net/base/network_change_notifier.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 
 class GURL;
 class PrefService;
@@ -29,7 +29,7 @@ class ReadingListDistillerPageFactory;
 class ReadingListDownloadService
     : public KeyedService,
       public ReadingListModelObserver,
-      public net::NetworkChangeNotifier::NetworkChangeObserver {
+      public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   ReadingListDownloadService(
       ReadingListModel* reading_list_model,
@@ -95,9 +95,8 @@ class ReadingListDownloadService
   // Callback for entry deletion.
   void OnDeleteEnd(const GURL& url, bool success);
 
-  // net::NetworkChangeNotifier::NetworkChangeObserver:
-  void OnNetworkChanged(
-      net::NetworkChangeNotifier::ConnectionType type) override;
+  // network::NetworkConnectionTracker::NetworkConnectionObserver:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
 
   ReadingListModel* reading_list_model_;
   base::FilePath chrome_profile_path_;

ios/chrome/test/testing_application_context.h

@@ -13,6 +13,7 @@
 #include "ios/chrome/browser/application_context.h"
 
 namespace network {
+class TestNetworkConnectionTracker;
 class TestURLLoaderFactory;
 class WeakWrapperSharedURLLoaderFactory;
 }  // namespace network
@@ -69,6 +70,8 @@ class TestingApplicationContext : public ApplicationContext {
   std::unique_ptr<network_time::NetworkTimeTracker> network_time_tracker_;
   bool was_last_shutdown_clean_;
   std::unique_ptr<network::TestURLLoaderFactory> test_url_loader_factory_;
+  std::unique_ptr<network::TestNetworkConnectionTracker>
+      test_network_connection_tracker_;
   scoped_refptr<network::WeakWrapperSharedURLLoaderFactory>
       system_shared_url_loader_factory_;
 

ios/chrome/test/testing_application_context.mm

@@ -13,6 +13,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"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -26,6 +27,8 @@ TestingApplicationContext::TestingApplicationContext()
       was_last_shutdown_clean_(false),
       test_url_loader_factory_(
           std::make_unique<network::TestURLLoaderFactory>()),
+      test_network_connection_tracker_(
+          network::TestNetworkConnectionTracker::CreateInstance()),
       system_shared_url_loader_factory_(
           base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
               test_url_loader_factory_.get())) {
@@ -183,5 +186,5 @@ TestingApplicationContext::GetComponentUpdateService() {
 network::NetworkConnectionTracker*
 TestingApplicationContext::GetNetworkConnectionTracker() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return nullptr;
+  return test_network_connection_tracker_.get();
 }

services/network/public/cpp/network_connection_tracker.cc

@@ -84,6 +84,17 @@ bool NetworkConnectionTracker::GetConnectionType(
   return false;
 }
 
+bool NetworkConnectionTracker::IsOffline() {
+  base::subtle::Atomic32 type_value =
+      base::subtle::NoBarrier_Load(&connection_type_);
+  if (type_value != kConnectionTypeInvalid) {
+    auto type = static_cast<network::mojom::ConnectionType>(type_value);
+    return type == network::mojom::ConnectionType::CONNECTION_NONE ||
+           type == network::mojom::ConnectionType::CONNECTION_UNKNOWN;
+  }
+  return true;
+}
+
 // static
 bool NetworkConnectionTracker::IsConnectionCellular(
     network::mojom::ConnectionType type) {

services/network/public/cpp/network_connection_tracker.h

@@ -60,6 +60,9 @@ class COMPONENT_EXPORT(NETWORK_CPP) NetworkConnectionTracker
   virtual bool GetConnectionType(network::mojom::ConnectionType* type,
                                  ConnectionTypeCallback callback);
 
+  // Returns true if the network is currently in an offline or unknown state.
+  bool IsOffline();
+
   // Returns true if |type| is a cellular connection.
   // Returns false if |type| is CONNECTION_UNKNOWN, and thus, depending on the
   // implementation of GetConnectionType(), it is possible that