Migrate NetBenchmarking from URLRequestContext to NetworkContext.

This will allow NetBenchmarking to be used with the network service
enabled. This also adds a NetworkContext::CloseAllConnections method.

Bug: 824974
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I566124d13e6607d579583202f030425ec2f3c598
Reviewed-on: https://chromium-review.googlesource.com/1135715Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577642}

chrome/browser/chrome_content_browser_client.cc

@@ -3453,13 +3453,13 @@ void ChromeContentBrowserClient::ExposeInterfacesToRenderer(
     auto* loading_predictor =
         predictors::LoadingPredictorFactory::GetForProfile(profile);
     auto* predictor = profile->GetNetworkPredictor();
-    net::URLRequestContextGetter* context =
-        render_process_host->GetStoragePartition()->GetURLRequestContext();
-    registry->AddInterface(base::Bind(
-        &NetBenchmarking::Create,
-        loading_predictor ? loading_predictor->GetWeakPtr() : nullptr,
-        predictor ? predictor->GetUIWeakPtr() : nullptr,
-        base::RetainedRef(context)));
+    registry->AddInterface(
+        base::BindRepeating(
+            &NetBenchmarking::Create,
+            loading_predictor ? loading_predictor->GetWeakPtr() : nullptr,
+            predictor ? predictor->GetUIWeakPtr() : nullptr,
+            render_process_host->GetID()),
+        ui_task_runner);
   }
 
 #if defined(SAFE_BROWSING_DB_LOCAL) || defined(SAFE_BROWSING_DB_REMOTE)

chrome/browser/net_benchmarking.cc

@@ -11,11 +11,14 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/time/time.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/predictors/loading_predictor.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/net_benchmarking.mojom.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/storage_partition.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/base/net_errors.h"
 #include "net/disk_cache/disk_cache.h"
@@ -24,19 +27,18 @@
 #include "net/http/http_cache.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 
 using content::BrowserThread;
 
 namespace {
 
-void ClearPredictorCacheOnUIThread(
-    base::WeakPtr<predictors::LoadingPredictor> loading_predictor,
-    base::WeakPtr<chrome_browser_net::Predictor> predictor) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (loading_predictor)
-    loading_predictor->resource_prefetch_predictor()->DeleteAllUrls();
-  if (predictor)
-    predictor->DiscardAllResultsAndClearPrefsOnUIThread();
+network::mojom::NetworkContext* GetNetworkContext(int render_process_id) {
+  content::RenderProcessHost* render_process_host =
+      content::RenderProcessHost::FromID(render_process_id);
+  if (!render_process_host)
+    return nullptr;
+  return render_process_host->GetStoragePartition()->GetNetworkContext();
 }
 
 }  // namespace
@@ -44,28 +46,28 @@ void ClearPredictorCacheOnUIThread(
 NetBenchmarking::NetBenchmarking(
     base::WeakPtr<predictors::LoadingPredictor> loading_predictor,
     base::WeakPtr<chrome_browser_net::Predictor> predictor,
-    net::URLRequestContextGetter* request_context)
+    int render_process_id)
     : loading_predictor_(loading_predictor),
       predictor_(predictor),
-      request_context_(request_context) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+      render_process_id_(render_process_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
 NetBenchmarking::~NetBenchmarking() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
 // static
 void NetBenchmarking::Create(
     base::WeakPtr<predictors::LoadingPredictor> loading_predictor,
     base::WeakPtr<chrome_browser_net::Predictor> predictor,
-    net::URLRequestContextGetter* request_context,
+    int render_process_id,
     chrome::mojom::NetBenchmarkingRequest request) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  mojo::MakeStrongBinding(
-      std::make_unique<NetBenchmarking>(std::move(loading_predictor),
-                                        std::move(predictor), request_context),
-      std::move(request));
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  mojo::MakeStrongBinding(std::make_unique<NetBenchmarking>(
+                              std::move(loading_predictor),
+                              std::move(predictor), render_process_id),
+                          std::move(request));
 }
 
 // static
@@ -76,49 +78,38 @@ bool NetBenchmarking::CheckBenchmarkingEnabled() {
 }
 
 void NetBenchmarking::ClearCache(const ClearCacheCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  int rv = -1;
-
-  disk_cache::Backend* backend = request_context_->GetURLRequestContext()
-                                     ->http_transaction_factory()
-                                     ->GetCache()
-                                     ->GetCurrentBackend();
-  if (backend) {
-    rv = backend->DoomAllEntries(callback);
-    if (rv == net::ERR_IO_PENDING) {
-      // Callback is handled by backend.
-      return;
-    }
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto* network_context = GetNetworkContext(render_process_id_);
+  if (network_context) {
+    network_context->ClearHttpCache(base::Time(), base::Time(), nullptr,
+                                    callback);
   }
-  callback.Run(rv);
 }
 
 void NetBenchmarking::ClearHostResolverCache(
     const ClearHostResolverCacheCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  net::HostCache* cache =
-      request_context_->GetURLRequestContext()->host_resolver()->GetHostCache();
-  if (cache)
-    cache->clear();
-  callback.Run();
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto* network_context = GetNetworkContext(render_process_id_);
+  if (network_context) {
+    network_context->ClearHostCache(nullptr, callback);
+  }
 }
 
 void NetBenchmarking::CloseCurrentConnections(
     const CloseCurrentConnectionsCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  request_context_->GetURLRequestContext()
-      ->http_transaction_factory()
-      ->GetCache()
-      ->CloseAllConnections();
-  callback.Run();
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto* network_context = GetNetworkContext(render_process_id_);
+  if (network_context) {
+    network_context->CloseAllConnections(callback);
+  }
 }
 
 void NetBenchmarking::ClearPredictorCache(
     const ClearPredictorCacheCallback& callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  BrowserThread::PostTaskAndReply(
-      BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&ClearPredictorCacheOnUIThread, loading_predictor_,
-                     predictor_),
-      callback);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (loading_predictor_)
+    loading_predictor_->resource_prefetch_predictor()->DeleteAllUrls();
+  if (predictor_)
+    predictor_->DiscardAllResultsAndClearPrefsOnUIThread();
+  callback.Run();
 }

chrome/browser/net_benchmarking.h

@@ -9,10 +9,6 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/common/net_benchmarking.mojom.h"
 
-namespace net {
-class URLRequestContextGetter;
-}
-
 namespace chrome_browser_net {
 class Predictor;
 }
@@ -29,7 +25,7 @@ class NetBenchmarking : public chrome::mojom::NetBenchmarking {
  public:
   NetBenchmarking(base::WeakPtr<predictors::LoadingPredictor> loading_predictor,
                   base::WeakPtr<chrome_browser_net::Predictor> predictor,
-                  net::URLRequestContextGetter* request_context);
+                  int render_process_id);
   ~NetBenchmarking() override;
 
   // Creates a NetBenchmarking instance and connects it strongly to a mojo pipe.
@@ -37,7 +33,7 @@ class NetBenchmarking : public chrome::mojom::NetBenchmarking {
   static void Create(
       base::WeakPtr<predictors::LoadingPredictor> loading_predictor,
       base::WeakPtr<chrome_browser_net::Predictor> predictor,
-      net::URLRequestContextGetter* request_context,
+      int render_process_id,
       chrome::mojom::NetBenchmarkingRequest request);
 
   // This method is thread-safe.
@@ -56,8 +52,7 @@ class NetBenchmarking : public chrome::mojom::NetBenchmarking {
   // These weak pointers should be dereferenced only on the UI thread.
   base::WeakPtr<predictors::LoadingPredictor> loading_predictor_;
   base::WeakPtr<chrome_browser_net::Predictor> predictor_;
-
-  scoped_refptr<net::URLRequestContextGetter> request_context_;
+  const int render_process_id_;
 
   DISALLOW_COPY_AND_ASSIGN(NetBenchmarking);
 };

chrome/common/net_benchmarking.mojom

@@ -12,15 +12,14 @@ interface NetBenchmarking {
 
   // Message sent from the renderer to the browser to request that the browser
   // clear the cache.  Used for debugging/testing.
-  // |result| is the returned status from the operation.
   [Sync]
-  ClearCache() => (int32 result);
+  ClearCache() => ();
 
   // Message sent from the renderer to the browser to request that the browser
   // clear the host cache.  Used for debugging/testing.
   [Sync]
   ClearHostResolverCache() => ();
-  
+
   // Message sent from the renderer to the browser to request that the browser
   // clear the predictor cache.  Used for debugging/testing.
   [Sync]

chrome/renderer/net_benchmarking_extension.cc

@@ -78,8 +78,7 @@ class NetBenchmarkingWrapper : public v8::Extension {
   }
 
   static void ClearCache(const v8::FunctionCallbackInfo<v8::Value>& args) {
-    int rv;
-    GetNetBenchmarking().ClearCache(&rv);
+    GetNetBenchmarking().ClearCache();
     WebCache::Clear();
   }
 

services/network/network_context.cc

@@ -676,6 +676,16 @@ void NetworkContext::ClearNetworkErrorLogging(
 }
 #endif  // BUILDFLAG(ENABLE_REPORTING)
 
+void NetworkContext::CloseAllConnections(CloseAllConnectionsCallback callback) {
+  net::HttpNetworkSession* http_session =
+      url_request_context_->http_transaction_factory()->GetSession();
+  DCHECK(http_session);
+
+  http_session->CloseAllConnections();
+
+  std::move(callback).Run();
+}
+
 void NetworkContext::SetNetworkConditions(
     const base::UnguessableToken& throttling_profile_id,
     mojom::NetworkConditionsPtr conditions) {

services/network/network_context.h

@@ -164,6 +164,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
   void ClearNetworkErrorLogging(
       mojom::ClearDataFilterPtr filter,
       ClearNetworkErrorLoggingCallback callback) override;
+  void CloseAllConnections(CloseAllConnectionsCallback callback) override;
   void SetNetworkConditions(const base::UnguessableToken& throttling_profile_id,
                             mojom::NetworkConditionsPtr conditions) override;
   void SetAcceptLanguage(const std::string& new_accept_language) override;

services/network/network_context_unittest.cc

@@ -194,6 +194,20 @@ class NetworkContextTest : public testing::Test,
 
   void OnSSLConfigChanged() override { ++ssl_config_changed_count_; }
 
+  // Looks up a value with the given name from the NetworkContext's
+  // TransportSocketPool info dictionary.
+  int GetSocketPoolInfo(NetworkContext* context, base::StringPiece name) {
+    int value;
+    context->url_request_context()
+        ->http_transaction_factory()
+        ->GetSession()
+        ->GetTransportSocketPool(
+            net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
+        ->GetInfoAsValue("", "", false)
+        ->GetInteger(name, &value);
+    return value;
+  }
+
  protected:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   std::unique_ptr<NetworkService> network_service_;
@@ -2504,7 +2518,7 @@ class ConnectionListener
   // Get called from the EmbeddedTestServer thread to be notified that
   // a connection was read from.
   void ReadFromSocket(const net::StreamSocket& connection, int rv) override {
-    NOTREACHED();
+    EXPECT_EQ(net::OK, rv);
   }
 
   // Wait for exactly |n| items in |sockets_|. |n| must be greater than 0.
@@ -2611,23 +2625,11 @@ TEST_F(NetworkContextTest, PreconnectZero) {
                                      net::LOAD_NORMAL, true);
   base::RunLoop().RunUntilIdle();
 
-  int num_sockets;
-  network_context->url_request_context()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->GetTransportSocketPool(
-          net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
-      ->GetInfoAsValue("", "", false)
-      ->GetInteger("idle_socket_count", &num_sockets);
+  int num_sockets =
+      GetSocketPoolInfo(network_context.get(), "idle_socket_count");
   ASSERT_EQ(num_sockets, 0);
-  int num_connecting_sockets;
-  network_context->url_request_context()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->GetTransportSocketPool(
-          net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
-      ->GetInfoAsValue("", "", false)
-      ->GetInteger("connecting_socket_count", &num_connecting_sockets);
+  int num_connecting_sockets =
+      GetSocketPoolInfo(network_context.get(), "connecting_socket_count");
   ASSERT_EQ(num_connecting_sockets, 0);
 }
 
@@ -2644,14 +2646,8 @@ TEST_F(NetworkContextTest, PreconnectTwo) {
                                      net::LOAD_NORMAL, true);
   connection_listener.WaitForAcceptedConnections(2u);
 
-  int num_sockets;
-  network_context->url_request_context()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->GetTransportSocketPool(
-          net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
-      ->GetInfoAsValue("", "", false)
-      ->GetInteger("idle_socket_count", &num_sockets);
+  int num_sockets =
+      GetSocketPoolInfo(network_context.get(), "idle_socket_count");
   ASSERT_EQ(num_sockets, 2);
 }
 
@@ -2669,14 +2665,8 @@ TEST_F(NetworkContextTest, PreconnectFour) {
 
   connection_listener.WaitForAcceptedConnections(4u);
 
-  int num_sockets;
-  network_context->url_request_context()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->GetTransportSocketPool(
-          net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
-      ->GetInfoAsValue("", "", false)
-      ->GetInteger("idle_socket_count", &num_sockets);
+  int num_sockets =
+      GetSocketPoolInfo(network_context.get(), "idle_socket_count");
   ASSERT_EQ(num_sockets, 4);
 }
 
@@ -2689,30 +2679,44 @@ TEST_F(NetworkContextTest, PreconnectMax) {
   test_server.SetConnectionListener(&connection_listener);
   ASSERT_TRUE(test_server.Start());
 
-  int num_sockets, max_num_sockets;
-  network_context->url_request_context()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->GetTransportSocketPool(
-          net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
-      ->GetInfoAsValue("", "", false)
-      ->GetInteger("max_sockets_per_group", &max_num_sockets);
+  int max_num_sockets =
+      GetSocketPoolInfo(network_context.get(), "max_sockets_per_group");
   EXPECT_GT(76, max_num_sockets);
 
   network_context->PreconnectSockets(76, test_server.base_url(),
                                      net::LOAD_NORMAL, true);
   base::RunLoop().RunUntilIdle();
 
-  network_context->url_request_context()
-      ->http_transaction_factory()
-      ->GetSession()
-      ->GetTransportSocketPool(
-          net::HttpNetworkSession::SocketPoolType::NORMAL_SOCKET_POOL)
-      ->GetInfoAsValue("", "", false)
-      ->GetInteger("idle_socket_count", &num_sockets);
+  int num_sockets =
+      GetSocketPoolInfo(network_context.get(), "idle_socket_count");
   ASSERT_EQ(num_sockets, max_num_sockets);
 }
 
+TEST_F(NetworkContextTest, CloseAllConnections) {
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+
+  ConnectionListener connection_listener;
+  net::EmbeddedTestServer test_server;
+  test_server.SetConnectionListener(&connection_listener);
+  ASSERT_TRUE(test_server.Start());
+
+  network_context->PreconnectSockets(2, test_server.base_url(),
+                                     net::LOAD_NORMAL, true);
+  connection_listener.WaitForAcceptedConnections(2u);
+
+  int num_sockets =
+      GetSocketPoolInfo(network_context.get(), "idle_socket_count");
+  EXPECT_EQ(num_sockets, 2);
+
+  base::RunLoop run_loop;
+  network_context->CloseAllConnections(run_loop.QuitClosure());
+  run_loop.Run();
+
+  num_sockets = GetSocketPoolInfo(network_context.get(), "idle_socket_count");
+  EXPECT_EQ(num_sockets, 0);
+}
+
 }  // namespace
 
 }  // namespace network

services/network/public/mojom/network_context.mojom

@@ -419,6 +419,9 @@ interface NetworkContext {
   // filter.
   ClearNetworkErrorLogging(ClearDataFilter? filter) => ();
 
+  // Closes all open connections within this context.
+  CloseAllConnections() => ();
+
   // Configures network conditions for the specified throttling profile.
   // The throttling will be applied only to requests that have maching
   // throttling_profile_id.

services/network/test/test_network_context.h

@@ -68,6 +68,7 @@ class TestNetworkContext : public mojom::NetworkContext {
   void ClearNetworkErrorLogging(
       mojom::ClearDataFilterPtr filter,
       ClearNetworkErrorLoggingCallback callback) override {}
+  void CloseAllConnections(CloseAllConnectionsCallback callback) override {}
   void SetNetworkConditions(const base::UnguessableToken& throttling_profile_id,
                             mojom::NetworkConditionsPtr conditions) override {}
   void SetAcceptLanguage(const std::string& new_accept_language) override {}