Update connectivity checker.

1) Use url_loader instead of URLRequest.
2) Use network_connection_tracker instead of
   network change notifier.

Bug: internal b/139539332
Change-Id: Ia84e3d62d22fd97d440ecf7e148ce8e663a9568d
Merge-With: eureka-internal/255804
Merge-With: eureka-internal/305059
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762598
Commit-Queue: Mohit Hotwani <hmohit@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Junbo Ke <juke@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690453}

chromecast/browser/cast_browser_main_parts.cc

@@ -58,6 +58,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_security_policy.h"
+#include "content/public/browser/network_service_instance.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/system_connector.h"
@@ -506,12 +507,16 @@ void CastBrowserMainParts::PreMainMessageLoopRun() {
 
   url_request_context_factory_->InitializeOnUIThread(nullptr);
 
+  cast_browser_process_->SetBrowserContext(
+      std::make_unique<CastBrowserContext>());
+
   cast_browser_process_->SetConnectivityChecker(ConnectivityChecker::Create(
       base::CreateSingleThreadTaskRunner({content::BrowserThread::IO}),
-      url_request_context_factory_->GetSystemGetter()));
+      content::BrowserContext::GetDefaultStoragePartition(
+          cast_browser_process_->browser_context())
+          ->GetURLLoaderFactoryForBrowserProcessIOThread(),
+      content::GetNetworkConnectionTracker()));
 
-  cast_browser_process_->SetBrowserContext(
-      std::make_unique<CastBrowserContext>());
   cast_browser_process_->SetMetricsServiceClient(
       std::make_unique<metrics::CastMetricsServiceClient>(
           cast_browser_process_->browser_client(),

chromecast/net/BUILD.gn

@@ -34,6 +34,8 @@ cast_source_set("net") {
     "//chromecast/public",
     "//components/proxy_config",
     "//net",
+    "//services/network/public/cpp",
+    "//services/network/public/mojom",
   ]
 
   if (is_fuchsia) {

chromecast/net/DEPS

 include_rules = [
   "+components/proxy_config",
   "+net",
+  "+services/network/public/cpp",
+  "+services/network/public/mojom",
 ]

chromecast/net/connectivity_checker.cc

@@ -6,13 +6,16 @@
 
 #include "base/single_thread_task_runner.h"
 #include "chromecast/net/connectivity_checker_impl.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace chromecast {
 
-ConnectivityChecker::ConnectivityChecker()
-    : connectivity_observer_list_(
-          new base::ObserverListThreadSafe<ConnectivityObserver>()) {
-}
+ConnectivityChecker::ConnectivityChecker(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+    : RefCountedDeleteOnSequence(std::move(task_runner)),
+      connectivity_observer_list_(
+          base::MakeRefCounted<
+              base::ObserverListThreadSafe<ConnectivityObserver>>()) {}
 
 ConnectivityChecker::~ConnectivityChecker() {
 }
@@ -36,9 +39,12 @@ void ConnectivityChecker::Notify(bool connected) {
 // static
 scoped_refptr<ConnectivityChecker> ConnectivityChecker::Create(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    net::URLRequestContextGetter* url_request_context_getter) {
+    std::unique_ptr<network::SharedURLLoaderFactoryInfo>
+        url_loader_factory_info,
+    network::NetworkConnectionTracker* network_connection_tracker) {
   return ConnectivityCheckerImpl::Create(task_runner,
-                                         url_request_context_getter);
+                                         std::move(url_loader_factory_info),
+                                         network_connection_tracker);
 }
 
 }  // namespace chromecast

chromecast/net/connectivity_checker.h

@@ -8,22 +8,24 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/ref_counted_delete_on_sequence.h"
 #include "base/observer_list_threadsafe.h"
+#include "base/sequenced_task_runner_helpers.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 }
 
-namespace net {
-class URLRequestContextGetter;
-}
+namespace network {
+class SharedURLLoaderFactoryInfo;
+class NetworkConnectionTracker;
+}  // namespace network
 
 namespace chromecast {
 
 // Checks if internet connectivity is available.
 class ConnectivityChecker
-    : public base::RefCountedThreadSafe<ConnectivityChecker> {
+    : public base::RefCountedDeleteOnSequence<ConnectivityChecker> {
  public:
   class ConnectivityObserver {
    public:
@@ -40,9 +42,9 @@ class ConnectivityChecker
 
   static scoped_refptr<ConnectivityChecker> Create(
       const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-      net::URLRequestContextGetter* url_request_context_getter);
-
-  ConnectivityChecker();
+      std::unique_ptr<network::SharedURLLoaderFactoryInfo>
+          url_loader_factory_info,
+      network::NetworkConnectionTracker* network_connection_tracker);
 
   void AddConnectivityObserver(ConnectivityObserver* observer);
   void RemoveConnectivityObserver(ConnectivityObserver* observer);
@@ -54,13 +56,16 @@ class ConnectivityChecker
   virtual void Check() = 0;
 
  protected:
+  explicit ConnectivityChecker(
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   virtual ~ConnectivityChecker();
 
   // Notifies observes that connectivity has changed.
   void Notify(bool connected);
 
  private:
-  friend class base::RefCountedThreadSafe<ConnectivityChecker>;
+  friend class base::RefCountedDeleteOnSequence<ConnectivityChecker>;
+  friend class base::DeleteHelper<ConnectivityChecker>;
 
   const scoped_refptr<base::ObserverListThreadSafe<ConnectivityObserver>>
       connectivity_observer_list_;

chromecast/net/connectivity_checker_impl.cc

@@ -22,9 +22,10 @@
 #include "net/http/http_status_code.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/socket/ssl_client_socket.h"
-#include "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_context_builder.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+#include "services/network/public/mojom/network_change_manager.mojom.h"
 
 namespace chromecast {
 
@@ -64,50 +65,57 @@ const char kMetricNameNetworkConnectivityCheckingErrorType[] =
 // static
 scoped_refptr<ConnectivityCheckerImpl> ConnectivityCheckerImpl::Create(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-    net::URLRequestContextGetter* url_request_context_getter) {
+    std::unique_ptr<network::SharedURLLoaderFactoryInfo>
+        url_loader_factory_info,
+    network::NetworkConnectionTracker* network_connection_tracker) {
   DCHECK(task_runner);
 
-  auto connectivity_checker =
-      base::WrapRefCounted(new ConnectivityCheckerImpl(task_runner));
+  auto connectivity_checker = base::WrapRefCounted(
+      new ConnectivityCheckerImpl(task_runner, network_connection_tracker));
   task_runner->PostTask(
       FROM_HERE,
       base::BindOnce(&ConnectivityCheckerImpl::Initialize, connectivity_checker,
-                     base::RetainedRef(url_request_context_getter)));
+                     std::move(url_loader_factory_info)));
   return connectivity_checker;
 }
 
 ConnectivityCheckerImpl::ConnectivityCheckerImpl(
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : ConnectivityChecker(),
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    network::NetworkConnectionTracker* network_connection_tracker)
+    : ConnectivityChecker(task_runner),
       task_runner_(std::move(task_runner)),
+      network_connection_tracker_(network_connection_tracker),
       connected_(false),
-      connection_type_(net::NetworkChangeNotifier::CONNECTION_NONE),
+      connection_type_(network::mojom::ConnectionType::CONNECTION_NONE),
       check_errors_(0),
       network_changed_pending_(false) {
   DCHECK(task_runner_.get());
+  DCHECK(network_connection_tracker_);
 }
 
 void ConnectivityCheckerImpl::Initialize(
-    net::URLRequestContextGetter* url_request_context_getter) {
-  url_request_context_getter_ = url_request_context_getter;
+    std::unique_ptr<network::SharedURLLoaderFactoryInfo>
+        url_loader_factory_info) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(url_loader_factory_info);
+  url_loader_factory_ = network::SharedURLLoaderFactory::Create(
+      std::move(url_loader_factory_info));
+
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   base::CommandLine::StringType check_url_str =
       command_line->GetSwitchValueNative(switches::kConnectivityCheckUrl);
   connectivity_check_url_.reset(new GURL(
       check_url_str.empty() ? kDefaultConnectivityCheckUrl : check_url_str));
 
-  url_request_context_ = url_request_context_getter->GetURLRequestContext();
-
-  net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
+  network_connection_tracker_->AddNetworkConnectionObserver(this);
   task_runner_->PostTask(FROM_HERE,
                          base::BindOnce(&ConnectivityCheckerImpl::Check, this));
 }
 
 ConnectivityCheckerImpl::~ConnectivityCheckerImpl() {
   DCHECK(task_runner_.get());
-  net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
-  task_runner_->DeleteSoon(FROM_HERE, url_request_.release());
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  network_connection_tracker_->RemoveNetworkConnectionObserver(this);
 }
 
 bool ConnectivityCheckerImpl::Connected() const {
@@ -117,18 +125,17 @@ bool ConnectivityCheckerImpl::Connected() const {
 
 void ConnectivityCheckerImpl::SetConnected(bool connected) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  if (connected_ == connected)
-    return;
   {
     base::AutoLock auto_lock(connected_lock_);
+    if (connected_ == connected)
+      return;
     connected_ = connected;
   }
 
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   base::CommandLine::StringType check_url_str =
       command_line->GetSwitchValueNative(switches::kConnectivityCheckUrl);
-  if (check_url_str.empty())
-  {
+  if (check_url_str.empty()) {
     connectivity_check_url_.reset(new GURL(
       connected ? kHttpConnectivityCheckUrl : kDefaultConnectivityCheckUrl));
     LOG(INFO) << "Change check url=" << *connectivity_check_url_;
@@ -145,24 +152,33 @@ void ConnectivityCheckerImpl::Check() {
 
 void ConnectivityCheckerImpl::CheckInternal() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(url_request_context_);
+  DCHECK(url_loader_factory_);
 
   // Don't check connectivity if network is offline, because Internet could be
   // accessible via netifs ignored.
-  if (net::NetworkChangeNotifier::IsOffline())
+  if (network_connection_tracker_->IsOffline())
     return;
 
-  // If url_request_ is non-null, there is already a check going on. Don't
+  // If url_loader_ is non-null, there is already a check going on. Don't
   // start another.
-  if (url_request_.get())
+  if (url_loader_)
     return;
 
-  DVLOG(1) << "Connectivity check: url=" << *connectivity_check_url_;
-  url_request_ = url_request_context_->CreateRequest(
-      *connectivity_check_url_, net::MAXIMUM_PRIORITY, this,
+  VLOG(1) << "Connectivity check: url=" << *connectivity_check_url_;
+  auto resource_request = std::make_unique<network::ResourceRequest>();
+  resource_request->url = GURL(*connectivity_check_url_);
+  resource_request->method = "HEAD";
+  resource_request->priority = net::MAXIMUM_PRIORITY;
+  // To enable ssl_info in the response.
+  resource_request->report_raw_headers = true;
+
+  url_loader_ = network::SimpleURLLoader::Create(std::move(resource_request),
                                                  MISSING_TRAFFIC_ANNOTATION);
-  url_request_->set_method("HEAD");
-  url_request_->Start();
+  network::SimpleURLLoader::HeadersOnlyCallback callback =
+      base::BindOnce(&ConnectivityCheckerImpl::OnConnectivityCheckComplete,
+                     base::Unretained(this));
+  url_loader_->DownloadHeadersOnly(url_loader_factory_.get(),
+                                   std::move(callback));
 
   timeout_.Reset(base::Bind(&ConnectivityCheckerImpl::OnUrlRequestTimeout,
                             this));
@@ -173,8 +189,8 @@ void ConnectivityCheckerImpl::CheckInternal() {
       FROM_HERE, timeout_.callback(), base::TimeDelta::FromSeconds(timeout));
 }
 
-void ConnectivityCheckerImpl::OnNetworkChanged(
-    net::NetworkChangeNotifier::ConnectionType type) {
+void ConnectivityCheckerImpl::OnConnectionChanged(
+    network::mojom::ConnectionType type) {
   DVLOG(2) << "OnNetworkChanged " << type;
   connection_type_ = type;
 
@@ -183,15 +199,16 @@ void ConnectivityCheckerImpl::OnNetworkChanged(
   network_changed_pending_ = true;
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
-      base::BindOnce(&ConnectivityCheckerImpl::OnNetworkChangedInternal, this),
+      base::BindOnce(&ConnectivityCheckerImpl::OnConnectionChangedInternal,
+                     this),
       base::TimeDelta::FromSeconds(kNetworkChangedDelayInSeconds));
 }
 
-void ConnectivityCheckerImpl::OnNetworkChangedInternal() {
+void ConnectivityCheckerImpl::OnConnectionChangedInternal() {
   network_changed_pending_ = false;
   Cancel();
 
-  if (connection_type_ == net::NetworkChangeNotifier::CONNECTION_NONE) {
+  if (connection_type_ == network::mojom::ConnectionType::CONNECTION_NONE) {
     SetConnected(false);
     return;
   }
@@ -199,18 +216,25 @@ void ConnectivityCheckerImpl::OnNetworkChangedInternal() {
   Check();
 }
 
-void ConnectivityCheckerImpl::OnResponseStarted(net::URLRequest* request,
-                                                int net_error) {
+void ConnectivityCheckerImpl::OnConnectivityCheckComplete(
+    scoped_refptr<net::HttpResponseHeaders> headers) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  int http_response_code =
-      (net_error == net::OK &&
-       request->response_info().headers.get() != nullptr)
-          ? request->response_info().headers->response_code()
+  DCHECK(url_loader_);
+  timeout_.Cancel();
+  int error = url_loader_->NetError();
+  if (error == net::ERR_INSECURE_RESPONSE && url_loader_->ResponseInfo() &&
+      url_loader_->ResponseInfo()->ssl_info) {
+    LOG(ERROR) << "OnSSLCertificateError: cert_status="
+               << url_loader_->ResponseInfo()->ssl_info->cert_status;
+    OnUrlRequestError(ErrorType::SSL_CERTIFICATE_ERROR);
+    return;
+  }
+  int http_response_code = (error == net::OK && headers)
+                               ? headers->response_code()
                                : net::HTTP_BAD_REQUEST;
 
   // Clears resources.
-  // URLRequest::Cancel() is called in destructor.
-  url_request_.reset(nullptr);
+  url_loader_.reset(nullptr);
 
   if (http_response_code < 400) {
     DVLOG(1) << "Connectivity check succeeded";
@@ -222,31 +246,10 @@ void ConnectivityCheckerImpl::OnResponseStarted(net::URLRequest* request,
         FROM_HERE,
         base::BindOnce(&ConnectivityCheckerImpl::CheckInternal, this),
         base::TimeDelta::FromSeconds(kConnectivitySuccessPeriodSeconds));
-    timeout_.Cancel();
     return;
   }
   LOG(ERROR) << "Connectivity check failed: " << http_response_code;
   OnUrlRequestError(ErrorType::BAD_HTTP_STATUS);
-  timeout_.Cancel();
-}
-
-void ConnectivityCheckerImpl::OnReadCompleted(net::URLRequest* request,
-                                              int bytes_read) {
-  NOTREACHED();
-}
-
-void ConnectivityCheckerImpl::OnSSLCertificateError(
-    net::URLRequest* request,
-    int net_error,
-    const net::SSLInfo& ssl_info,
-    bool fatal) {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  LOG(ERROR) << "OnSSLCertificateError: cert_status=" << ssl_info.cert_status;
-  url_request_context_->http_transaction_factory()
-      ->GetSession()
-      ->ClearSSLSessionCache();
-  OnUrlRequestError(ErrorType::SSL_CERTIFICATE_ERROR);
-  timeout_.Cancel();
 }
 
 void ConnectivityCheckerImpl::OnUrlRequestError(ErrorType type) {
@@ -262,7 +265,7 @@ void ConnectivityCheckerImpl::OnUrlRequestError(ErrorType type) {
     check_errors_ = kNumErrorsToNotifyOffline;
     SetConnected(false);
   }
-  url_request_.reset(nullptr);
+  url_loader_.reset(nullptr);
   // Check again.
   task_runner_->PostDelayedTask(
       FROM_HERE, base::BindOnce(&ConnectivityCheckerImpl::Check, this),
@@ -277,10 +280,10 @@ void ConnectivityCheckerImpl::OnUrlRequestTimeout() {
 
 void ConnectivityCheckerImpl::Cancel() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  if (!url_request_.get())
+  if (!url_loader_)
     return;
-  DVLOG(2) << "Cancel connectivity check in progress";
-  url_request_.reset(nullptr);  // URLRequest::Cancel() is called in destructor.
+  VLOG(2) << "Cancel connectivity check in progress";
+  url_loader_.reset(nullptr);
   timeout_.Cancel();
 }
 

chromecast/net/connectivity_checker_impl.h

@@ -5,24 +5,28 @@
 #ifndef CHROMECAST_NET_CONNECTIVITY_CHECKER_IMPL_H_
 #define CHROMECAST_NET_CONNECTIVITY_CHECKER_IMPL_H_
 
+#include <memory>
+
 #include "base/cancelable_callback.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "chromecast/net/connectivity_checker.h"
-#include "net/base/network_change_notifier.h"
-#include "net/url_request/url_request.h"
+#include "services/network/public/cpp/network_connection_tracker.h"
 
 class GURL;
 
 namespace base {
 class SingleThreadTaskRunner;
-}
+}  // namespace base
 
 namespace net {
-class SSLInfo;
-class URLRequest;
-class URLRequestContext;
-class URLRequestContextGetter;
-}
+class HttpResponseHeaders;
+}  // namespace net
+
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
+}  // namespace network
 
 namespace chromecast {
 
@@ -30,13 +34,14 @@ namespace chromecast {
 // to given url.
 class ConnectivityCheckerImpl
     : public ConnectivityChecker,
-      public net::URLRequest::Delegate,
-      public net::NetworkChangeNotifier::NetworkChangeObserver {
+      public network::NetworkConnectionTracker::NetworkConnectionObserver {
  public:
   // Connectivity checking and initialization will run on task_runner.
   static scoped_refptr<ConnectivityCheckerImpl> Create(
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-      net::URLRequestContextGetter* url_request_context_getter);
+      std::unique_ptr<network::SharedURLLoaderFactoryInfo>
+          url_loader_factory_info,
+      network::NetworkConnectionTracker* network_connection_tracker);
 
   // ConnectivityChecker implementation:
   bool Connected() const override;
@@ -44,26 +49,23 @@ class ConnectivityCheckerImpl
 
  protected:
   explicit ConnectivityCheckerImpl(
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+      network::NetworkConnectionTracker* network_connection_tracker);
   ~ConnectivityCheckerImpl() override;
 
  private:
-  // UrlRequest::Delegate implementation:
-  void OnResponseStarted(net::URLRequest* request, int net_error) override;
-  void OnReadCompleted(net::URLRequest* request, int bytes_read) override;
-  void OnSSLCertificateError(net::URLRequest* request,
-                             int net_error,
-                             const net::SSLInfo& ssl_info,
-                             bool fatal) override;
-
   // Initializes ConnectivityChecker
-  void Initialize(net::URLRequestContextGetter* url_request_context_getter);
+  void Initialize(std::unique_ptr<network::SharedURLLoaderFactoryInfo>
+                      url_loader_factory_info);
+
+  // network::NetworkConnectionTracker::NetworkConnectionObserver
+  // implementation:
+  void OnConnectionChanged(network::mojom::ConnectionType type) override;
 
-  // net::NetworkChangeNotifier::NetworkChangeObserver implementation:
-  void OnNetworkChanged(
-      net::NetworkChangeNotifier::ConnectionType type) override;
+  void OnConnectionChangedInternal();
 
-  void OnNetworkChangedInternal();
+  void OnConnectivityCheckComplete(
+      scoped_refptr<net::HttpResponseHeaders> headers);
 
   // Cancels current connectivity checking in progress.
   void Cancel();
@@ -86,17 +88,17 @@ class ConnectivityCheckerImpl
   void CheckInternal();
 
   std::unique_ptr<GURL> connectivity_check_url_;
-  scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
-  net::URLRequestContext* url_request_context_;
-  std::unique_ptr<net::URLRequest> url_request_;
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  std::unique_ptr<network::SimpleURLLoader> url_loader_;
   const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  network::NetworkConnectionTracker* const network_connection_tracker_;
 
   // connected_lock_ protects access to connected_ which is shared across
   // threads.
   mutable base::Lock connected_lock_;
   bool connected_;
 
-  net::NetworkChangeNotifier::ConnectionType connection_type_;
+  network::mojom::ConnectionType connection_type_;
   // Number of connectivity check errors.
   unsigned int check_errors_;
   bool network_changed_pending_;

chromecast/net/fake_connectivity_checker.cc

@@ -3,13 +3,13 @@
 // found in the LICENSE file.
 
 #include "chromecast/net/fake_connectivity_checker.h"
+#include "base/threading/thread_task_runner_handle.h"
 
 namespace chromecast {
 
 FakeConnectivityChecker::FakeConnectivityChecker()
-    : ConnectivityChecker(),
-      connected_(true) {
-}
+    : ConnectivityChecker(base::ThreadTaskRunnerHandle::Get()),
+      connected_(true) {}
 
 FakeConnectivityChecker::~FakeConnectivityChecker() {}