Don't send data use updates to the browser unless the task manager is…

… running.

This is currently used for the task manager and UMA. The latter can be done in the network service. The former should only be done when the task manager is running (which is also never on Android).

Bug: 1425174
Change-Id: Iee9cf892357e47d664cb700a9d5932bc1c16a624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4347741
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Auto-Submit: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118528}

chrome/browser/chrome_content_browser_client.cc

@@ -147,6 +147,7 @@
 #include "chrome/browser/ssl/ssl_client_auth_metrics.h"
 #include "chrome/browser/ssl/ssl_client_certificate_selector.h"
 #include "chrome/browser/ssl/typed_navigation_upgrade_throttle.h"
+#include "chrome/browser/task_manager/sampling/task_manager_impl.h"
 #include "chrome/browser/tracing/chrome_tracing_delegate.h"
 #include "chrome/browser/translate/translate_service.h"
 #include "chrome/browser/ui/blocked_content/blocked_window_params.h"
@@ -336,6 +337,7 @@
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/self_deleting_url_loader_factory.h"
 #include "services/network/public/cpp/web_sandbox_flags.h"
+#include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/public/mojom/web_transport.mojom.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/loader/url_loader_throttle.h"
@@ -6165,6 +6167,13 @@ void ChromeContentBrowserClient::OnNetworkServiceCreated(
 
   SystemNetworkContextManager::GetInstance()->OnNetworkServiceCreated(
       network_service);
+
+#if !BUILDFLAG(IS_ANDROID)
+  if (task_manager::TaskManagerImpl::IsCreated() &&
+      task_manager::TaskManagerImpl::GetInstance()->is_running()) {
+    network_service->EnableDataUseUpdates(true);
+  }
+#endif
 }
 
 void ChromeContentBrowserClient::ConfigureNetworkContextParams(
@@ -6607,8 +6616,11 @@ void ChromeContentBrowserClient::OnNetworkServiceDataUseUpdate(
       render_frame_host_id, recv_bytes, sent_bytes);
 #endif
 
-  ChromeDataUseMeasurement::GetInstance().ReportNetworkServiceDataUse(
-      network_traffic_annotation_id_hash, recv_bytes, sent_bytes);
+  if (!base::FeatureList::IsEnabled(
+          network::features::kLessChattyNetworkService)) {
+    ChromeDataUseMeasurement::GetInstance().ReportNetworkServiceDataUse(
+        network_traffic_annotation_id_hash, recv_bytes, sent_bytes);
+  }
 }
 
 base::FilePath

chrome/browser/data_use_measurement/chrome_data_use_measurement_browsertest.cc

@@ -11,9 +11,11 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/metrics/content/subprocess_metrics_provider.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
+#include "services/network/public/cpp/features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 class ChromeDataUseMeasurementBrowserTest : public InProcessBrowserTest {
@@ -33,6 +35,11 @@ class ChromeDataUseMeasurementBrowserTest : public InProcessBrowserTest {
     do {
       base::ThreadPoolInstance::Get()->FlushForTesting();
       base::RunLoop().RunUntilIdle();
+      if (base::FeatureList::IsEnabled(
+              network::features::kLessChattyNetworkService)) {
+        content::FetchHistogramsFromChildProcesses();
+        metrics::SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
+      }
     } while (GetTotalDataUse() == 0);
   }
 

chrome/browser/task_manager/sampling/task_manager_impl.cc

@@ -36,6 +36,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
 #include "services/network/public/cpp/features.h"
+#include "services/network/public/mojom/network_service.mojom.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -643,6 +644,8 @@ void TaskManagerImpl::StartUpdating() {
 
   is_running_ = true;
 
+  content::GetNetworkService()->EnableDataUseUpdates(true);
+
   for (const auto& provider : task_providers_)
     provider->SetObserver(this);
 
@@ -657,6 +660,8 @@ void TaskManagerImpl::StopUpdating() {
 
   is_running_ = false;
 
+  content::GetNetworkService()->EnableDataUseUpdates(false);
+
   for (const auto& provider : task_providers_)
     provider->ClearObserver();
 

chrome/browser/task_manager/sampling/task_manager_impl.h

@@ -110,6 +110,8 @@ class TaskManagerImpl : public TaskManagerInterface,
       int64_t recv_bytes,
       int64_t sent_bytes);
 
+  bool is_running() const { return is_running_; }
+
  private:
   using PidToTaskGroupMap =
       std::map<base::ProcessId, std::unique_ptr<TaskGroup>>;

services/network/network_service.cc

@@ -771,6 +771,10 @@ void NetworkService::ParseHeaders(
   std::move(callback).Run(PopulateParsedHeaders(headers.get(), url));
 }
 
+void NetworkService::EnableDataUseUpdates(bool enable) {
+  data_use_updates_enabled_ = enable;
+}
+
 #if BUILDFLAG(IS_CT_SUPPORTED)
 void NetworkService::ClearSCTAuditingCache() {
   sct_auditing_cache_->ClearCache();

services/network/network_service.h

@@ -194,6 +194,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
   void ParseHeaders(const GURL& url,
                     const scoped_refptr<net::HttpResponseHeaders>& headers,
                     ParseHeadersCallback callback) override;
+  void EnableDataUseUpdates(bool enable) override;
 #if BUILDFLAG(IS_CT_SUPPORTED)
   void ClearSCTAuditingCache() override;
   void ConfigureSCTAuditing(
@@ -302,6 +303,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 
   base::Time pins_list_update_time() const { return pins_list_update_time_; }
 
+  bool data_use_updates_enabled() const { return data_use_updates_enabled_; }
+
   mojom::URLLoaderNetworkServiceObserver*
   GetDefaultURLLoaderNetworkServiceObserver();
 
@@ -440,6 +443,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 
   base::Time pins_list_update_time_;
 
+  bool data_use_updates_enabled_ = false;
+
   // This is used only in tests. It avoids leaky SystemDnsConfigChangeNotifiers
   // leaking stale listeners between tests.
   std::unique_ptr<net::NetworkChangeNotifier> mock_network_change_notifier_;

services/network/public/mojom/network_service.mojom

@@ -368,4 +368,9 @@ interface NetworkService {
   // Extensions, etc.
   ParseHeaders(url.mojom.Url url, HttpResponseHeaders headers)
       => (ParsedHeaders parsed_headers);
+
+  // Enables or disables whether URLLoaders need to call
+  // URLLoaderNetworkServiceObserver::OnDataUseUpdate. By default it's not
+  // enabled.
+  EnableDataUseUpdates(bool enable);
 };

services/network/test/url_loader_context_for_tests.cc

@@ -70,4 +70,8 @@ corb::PerFactoryState& URLLoaderContextForTests::GetMutableCorbState() {
   return corb_state_;
 }
 
+bool URLLoaderContextForTests::DataUseUpdatesEnabled() {
+  return false;
+}
+
 }  // namespace network

services/network/test/url_loader_context_for_tests.h

@@ -54,6 +54,7 @@ class URLLoaderContextForTests : public URLLoaderContext {
   scoped_refptr<ResourceSchedulerClient> GetResourceSchedulerClient()
       const override;
   corb::PerFactoryState& GetMutableCorbState() override;
+  bool DataUseUpdatesEnabled() override;
 
  private:
   mojom::URLLoaderFactoryParams factory_params_;

services/network/url_loader.cc

@@ -549,7 +549,8 @@ URLLoader::URLLoader(
       allow_http1_for_streaming_upload_(
           request.request_body &&
           request.request_body->AllowHTTP1ForStreamingUpload()),
-      accept_ch_frame_observer_(std::move(accept_ch_frame_observer)) {
+      accept_ch_frame_observer_(std::move(accept_ch_frame_observer)),
+      provide_data_use_updates_(context.DataUseUpdatesEnabled()) {
   TRACE_EVENT("loading", "URLLoader::URLLoader",
               perfetto::Flow::FromPointer(this));
   DCHECK(delete_callback_);
@@ -2046,13 +2047,26 @@ void URLLoader::NotifyCompleted(int error_code) {
     upload_progress_tracker_ = nullptr;
   }
 
-  if (url_loader_network_observer_ &&
-      (url_request_->GetTotalReceivedBytes() > 0 ||
-       url_request_->GetTotalSentBytes() > 0)) {
-    url_loader_network_observer_->OnDataUseUpdate(
-        url_request_->traffic_annotation().unique_id_hash_code,
-        url_request_->GetTotalReceivedBytes(),
-        url_request_->GetTotalSentBytes());
+  auto total_received = url_request_->GetTotalReceivedBytes();
+  auto total_sent = url_request_->GetTotalSentBytes();
+  if (base::FeatureList::IsEnabled(features::kLessChattyNetworkService)) {
+    if (total_received > 0) {
+      base::UmaHistogramCustomCounts("DataUse.BytesReceived3.Delegate",
+                                     total_received, 50, 10 * 1000 * 1000, 50);
+    }
+
+    if (total_sent > 0) {
+      UMA_HISTOGRAM_COUNTS_1M("DataUse.BytesSent3.Delegate", total_sent);
+    }
+  }
+  if ((total_received > 0 || total_sent > 0)) {
+    if (url_loader_network_observer_ &&
+        (!base::FeatureList::IsEnabled(features::kLessChattyNetworkService) ||
+         provide_data_use_updates_)) {
+      url_loader_network_observer_->OnDataUseUpdate(
+          url_request_->traffic_annotation().unique_id_hash_code,
+          total_received, total_sent);
+    }
   }
 
   if (url_loader_client_.Get()) {

services/network/url_loader.h

@@ -681,6 +681,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
   bool allow_cookies_from_browser_ = false;
   std::string cookies_from_browser_;
 
+  const bool provide_data_use_updates_;
+
   base::WeakPtrFactory<URLLoader> weak_ptr_factory_{this};
 };
 

services/network/url_loader_context.h

@@ -53,6 +53,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoaderContext {
   virtual scoped_refptr<ResourceSchedulerClient> GetResourceSchedulerClient()
       const = 0;
   virtual corb::PerFactoryState& GetMutableCorbState() = 0;
+  virtual bool DataUseUpdatesEnabled() = 0;
 
  protected:
   // `protected` destructor = can only destruct via concrete implementations

services/network/url_loader_factory.cc

@@ -172,6 +172,11 @@ corb::PerFactoryState& URLLoaderFactory::GetMutableCorbState() {
   return corb_state_;
 }
 
+bool URLLoaderFactory::DataUseUpdatesEnabled() {
+  return context_->network_service() &&
+         context_->network_service()->data_use_updates_enabled();
+}
+
 void URLLoaderFactory::CreateLoaderAndStartWithSyncClient(
     mojo::PendingReceiver<mojom::URLLoader> receiver,
     int32_t request_id,

services/network/url_loader_factory.h

@@ -84,6 +84,7 @@ class URLLoaderFactory : public mojom::URLLoaderFactory,
   scoped_refptr<ResourceSchedulerClient> GetResourceSchedulerClient()
       const override;
   corb::PerFactoryState& GetMutableCorbState() override;
+  bool DataUseUpdatesEnabled() override;
 
   // Allows starting a URLLoader with a synchronous URLLoaderClient as an
   // optimization.