Remove the NetworkUsageAccumulator from the NetworkService.

With https://chromium-review.googlesource.com/c/chromium/src/+/2737395
GetTotalNetworkUsages is no longer needed and can be removed. When
a URLLoader was complete the bytes sent/received were already sent to
the browser so we don't need two ways to relay the same information.

BUG=1173710

Change-Id: I86d5823473c738de202b45a4518882a2af59335b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2737040
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#861775}

content/browser/loader/navigation_url_loader_impl_unittest.cc

@@ -102,9 +102,9 @@ class TestNavigationLoaderInterceptor : public NavigationLoaderInterceptor {
         0 /* keepalive_request_size */,
         false /* require_network_isolation_key */, resource_scheduler_client_,
         nullptr /* keepalive_statistics_recorder */,
-        nullptr /* network_usage_accumulator */, nullptr /* header_client */,
-        nullptr /* origin_policy_manager */, nullptr /* trust_token_helper */,
-        kEmptyOriginAccessList, mojo::NullRemote() /* cookie_observer */,
+        nullptr /* header_client */, nullptr /* origin_policy_manager */,
+        nullptr /* trust_token_helper */, kEmptyOriginAccessList,
+        mojo::NullRemote() /* cookie_observer */,
         mojo::NullRemote() /* url_loader_network_observer */,
         /*devtools_observer=*/mojo::NullRemote());
   }

content/browser/network_service_restart_browsertest.cc

@@ -95,41 +95,6 @@ int LoadBasicRequestOnUIThread(
   return simple_loader->NetError();
 }
 
-std::vector<network::mojom::NetworkUsagePtr> GetTotalNetworkUsages() {
-  std::vector<network::mojom::NetworkUsagePtr> network_usages;
-  base::RunLoop run_loop;
-  GetNetworkService()->GetTotalNetworkUsages(base::BindOnce(
-      [](std::vector<network::mojom::NetworkUsagePtr>* p_network_usages,
-         base::OnceClosure quit_closure,
-         std::vector<network::mojom::NetworkUsagePtr> returned_usages) {
-        *p_network_usages = std::move(returned_usages);
-        std::move(quit_closure).Run();
-      },
-      base::Unretained(&network_usages), run_loop.QuitClosure()));
-  run_loop.Run();
-  return network_usages;
-}
-
-bool CheckContainsProcessID(
-    const std::vector<network::mojom::NetworkUsagePtr>& usages,
-    int process_id) {
-  for (const auto& usage : usages) {
-    if ((int)usage->process_id == process_id)
-      return true;
-  }
-  return false;
-}
-
-// Wait until |condition| returns true.
-void WaitForCondition(base::RepeatingCallback<bool()> condition) {
-  while (!condition.Run()) {
-    base::RunLoop run_loop;
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
-    run_loop.Run();
-  }
-}
-
 class ServiceWorkerStatusObserver : public ServiceWorkerContextCoreObserver {
  public:
   void WaitForStopped() {
@@ -980,48 +945,6 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, MAYBE_SharedWorker) {
   EXPECT_TRUE(service->worker_hosts_.empty());
 }
 
-// Make sure the entry in |NetworkService::GetTotalNetworkUsages()| was cleared
-// after process closed.
-IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest,
-                       GetNetworkUsagesClosed) {
-  if (IsInProcessNetworkService())
-    return;
-  EXPECT_TRUE(NavigateToURL(shell(), GetTestURL()));
-  Shell* shell2 = CreateBrowser();
-  EXPECT_TRUE(NavigateToURL(shell2, GetTestURL()));
-
-  int process_id1 =
-      shell()->web_contents()->GetMainFrame()->GetProcess()->GetID();
-  int process_id2 =
-      shell2->web_contents()->GetMainFrame()->GetProcess()->GetID();
-
-  // Load resource on the renderer to make sure the traffic was recorded.
-  EXPECT_TRUE(CheckCanLoadHttp(shell(), "/title2.html"));
-  EXPECT_TRUE(CheckCanLoadHttp(shell2, "/title3.html"));
-
-  // Both processes should have traffic recorded.
-  auto network_usages = GetTotalNetworkUsages();
-  EXPECT_TRUE(CheckContainsProcessID(network_usages, process_id1));
-  EXPECT_TRUE(CheckContainsProcessID(network_usages, process_id2));
-
-  // Closing |shell2| should cause the entry to be cleared.
-  shell2->Close();
-  shell2 = nullptr;
-
-  // Wait until the Network Service has noticed the change. We don't have a
-  // better way to force a flush on the Network Service side.
-  WaitForCondition(base::BindRepeating(
-      [](int process_id) {
-        auto usages = GetTotalNetworkUsages();
-        return !CheckContainsProcessID(usages, process_id);
-      },
-      process_id2));
-
-  network_usages = GetTotalNetworkUsages();
-  EXPECT_TRUE(CheckContainsProcessID(network_usages, process_id1));
-  EXPECT_FALSE(CheckContainsProcessID(network_usages, process_id2));
-}
-
 // Make sure that kSSLKeyLogFileHistogram is correctly recorded when the
 // network service instance is started and the SSLKEYLOGFILE env var is set or
 // the "--ssl-key-log-file" arg is set.
@@ -1068,43 +991,6 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, SSLKeyLogFileMetrics) {
   }
 }
 
-// Make sure |NetworkService::GetTotalNetworkUsages()| continues to work after
-// crash. See 'network_usage_accumulator_unittest' for quantified tests.
-IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest,
-                       GetNetworkUsagesCrashed) {
-  if (IsInProcessNetworkService())
-    return;
-  EXPECT_TRUE(NavigateToURL(shell(), GetTestURL()));
-  Shell* shell2 = CreateBrowser();
-  EXPECT_TRUE(NavigateToURL(shell2, GetTestURL()));
-
-  int process_id1 =
-      shell()->web_contents()->GetMainFrame()->GetProcess()->GetID();
-  int process_id2 =
-      shell2->web_contents()->GetMainFrame()->GetProcess()->GetID();
-
-  // Load resource on the renderer to make sure the traffic was recorded.
-  EXPECT_TRUE(CheckCanLoadHttp(shell(), "/title2.html"));
-  EXPECT_TRUE(CheckCanLoadHttp(shell2, "/title3.html"));
-
-  // Both processes should have traffic recorded.
-  auto network_usages = GetTotalNetworkUsages();
-  EXPECT_TRUE(CheckContainsProcessID(network_usages, process_id1));
-  EXPECT_TRUE(CheckContainsProcessID(network_usages, process_id2));
-
-  // Crashing Network Service should cause all entries to be cleared.
-  SimulateNetworkServiceCrash();
-  network_usages = GetTotalNetworkUsages();
-  EXPECT_FALSE(CheckContainsProcessID(network_usages, process_id1));
-  EXPECT_FALSE(CheckContainsProcessID(network_usages, process_id2));
-
-  // Should still be able to recored new traffic after crash.
-  EXPECT_TRUE(CheckCanLoadHttp(shell(), "/title2.html"));
-  network_usages = GetTotalNetworkUsages();
-  EXPECT_TRUE(CheckContainsProcessID(network_usages, process_id1));
-  EXPECT_FALSE(CheckContainsProcessID(network_usages, process_id2));
-}
-
 // Make sure cookie access doesn't hang or fail after a network process crash.
 IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, Cookies) {
   if (IsInProcessNetworkService())

services/network/BUILD.gn

@@ -74,8 +74,6 @@ component("network_service") {
     "network_service_network_delegate.h",
     "network_service_proxy_delegate.cc",
     "network_service_proxy_delegate.h",
-    "network_usage_accumulator.cc",
-    "network_usage_accumulator.h",
     "origin_policy/origin_policy_constants.h",
     "origin_policy/origin_policy_fetcher.cc",
     "origin_policy/origin_policy_fetcher.h",
@@ -325,7 +323,6 @@ source_set("tests") {
     "network_quality_estimator_manager_unittest.cc",
     "network_service_proxy_delegate_unittest.cc",
     "network_service_unittest.cc",
-    "network_usage_accumulator_unittest.cc",
     "origin_policy/origin_policy_manager_unittest.cc",
     "origin_policy/origin_policy_parsed_header_unittest.cc",
     "origin_policy/origin_policy_parser_unittest.cc",

services/network/network_context.cc

@@ -83,7 +83,6 @@
 #include "services/network/network_service.h"
 #include "services/network/network_service_network_delegate.h"
 #include "services/network/network_service_proxy_delegate.h"
-#include "services/network/network_usage_accumulator.h"
 #include "services/network/p2p/socket_manager.h"
 #include "services/network/proxy_config_service_mojo.h"
 #include "services/network/proxy_lookup_request.h"
@@ -701,23 +700,9 @@ void NetworkContext::DisableQuic() {
 
 void NetworkContext::DestroyURLLoaderFactory(
     cors::CorsURLLoaderFactory* url_loader_factory) {
-  const int32_t process_id = url_loader_factory->process_id();
-
   auto it = url_loader_factories_.find(url_loader_factory);
   DCHECK(it != url_loader_factories_.end());
   url_loader_factories_.erase(it);
-
-  // Reset bytes transferred for the process if |url_loader_factory| is the
-  // last factory associated with the process.
-  if (network_service() &&
-      std::none_of(url_loader_factories_.cbegin(), url_loader_factories_.cend(),
-                   [process_id](const auto& factory) {
-                     return factory->process_id() == process_id;
-                   })) {
-    network_service()
-        ->network_usage_accumulator()
-        ->ClearBytesTransferredForProcess(process_id);
-  }
 }
 
 void NetworkContext::Remove(QuicTransport* transport) {

services/network/network_service.cc

@@ -58,7 +58,6 @@
 #include "services/network/net_log_exporter.h"
 #include "services/network/net_log_proxy_sink.h"
 #include "services/network/network_context.h"
-#include "services/network/network_usage_accumulator.h"
 #include "services/network/public/cpp/crash_keys.h"
 #include "services/network/public/cpp/cross_origin_read_blocking.h"
 #include "services/network/public/cpp/features.h"
@@ -329,8 +328,6 @@ void NetworkService::Initialize(mojom::NetworkServiceParamsPtr params,
       net::NetworkChangeNotifier::GetSystemDnsConfigNotifier(), net_log_);
   host_resolver_factory_ = std::make_unique<net::HostResolver::Factory>();
 
-  network_usage_accumulator_ = std::make_unique<NetworkUsageAccumulator>();
-
   http_auth_cache_copier_ = std::make_unique<HttpAuthCacheCopier>();
 
   crl_set_distributor_ = std::make_unique<CRLSetDistributor>();
@@ -601,11 +598,6 @@ void NetworkService::GetDnsConfigChangeManager(
   dns_config_change_manager_->AddReceiver(std::move(receiver));
 }
 
-void NetworkService::GetTotalNetworkUsages(
-    mojom::NetworkService::GetTotalNetworkUsagesCallback callback) {
-  std::move(callback).Run(network_usage_accumulator_->GetTotalNetworkUsages());
-}
-
 void NetworkService::GetNetworkList(
     uint32_t policy,
     mojom::NetworkService::GetNetworkListCallback callback) {

services/network/network_service.h

@@ -64,7 +64,6 @@ class HttpAuthCacheCopier;
 class NetLogProxySink;
 class NetworkContext;
 class NetworkService;
-class NetworkUsageAccumulator;
 class SCTAuditingCache;
 
 class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
@@ -149,8 +148,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
       override;
   void GetDnsConfigChangeManager(
       mojo::PendingReceiver<mojom::DnsConfigChangeManager> receiver) override;
-  void GetTotalNetworkUsages(
-      mojom::NetworkService::GetTotalNetworkUsagesCallback callback) override;
   void GetNetworkList(
       uint32_t policy,
       mojom::NetworkService::GetNetworkListCallback callback) override;
@@ -218,9 +215,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
   net::HostResolver::Factory* host_resolver_factory() {
     return host_resolver_factory_.get();
   }
-  NetworkUsageAccumulator* network_usage_accumulator() {
-    return network_usage_accumulator_.get();
-  }
   HttpAuthCacheCopier* http_auth_cache_copier() {
     return http_auth_cache_copier_.get();
   }
@@ -303,7 +297,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 
   std::unique_ptr<net::HostResolverManager> host_resolver_manager_;
   std::unique_ptr<net::HostResolver::Factory> host_resolver_factory_;
-  std::unique_ptr<NetworkUsageAccumulator> network_usage_accumulator_;
   std::unique_ptr<HttpAuthCacheCopier> http_auth_cache_copier_;
 
   // Members that would store the http auth network_service related params.