diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 6683d2be7f3f9..9d2a1bc1236b7 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/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 diff --git a/chrome/browser/data_use_measurement/chrome_data_use_measurement_browsertest.cc b/chrome/browser/data_use_measurement/chrome_data_use_measurement_browsertest.cc index 79ffa12626af5..31b582e4aefc6 100644 --- a/chrome/browser/data_use_measurement/chrome_data_use_measurement_browsertest.cc +++ b/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); } diff --git a/chrome/browser/task_manager/sampling/task_manager_impl.cc b/chrome/browser/task_manager/sampling/task_manager_impl.cc index b0692b9846095..286d0aa53d092 100644 --- a/chrome/browser/task_manager/sampling/task_manager_impl.cc +++ b/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(); diff --git a/chrome/browser/task_manager/sampling/task_manager_impl.h b/chrome/browser/task_manager/sampling/task_manager_impl.h index c9f35f4becdc6..3d33db4b100d7 100644 --- a/chrome/browser/task_manager/sampling/task_manager_impl.h +++ b/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>; diff --git a/services/network/network_service.cc b/services/network/network_service.cc index c12342762a35e..a5df705bbaeff 100644 --- a/services/network/network_service.cc +++ b/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(); diff --git a/services/network/network_service.h b/services/network/network_service.h index 73516a0aae1c1..9b3ad0c2b5cc8 100644 --- a/services/network/network_service.h +++ b/services/network/network_service.h @@ -194,6 +194,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService void ParseHeaders(const GURL& url, const scoped_refptr& 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 mock_network_change_notifier_; diff --git a/services/network/public/mojom/network_service.mojom b/services/network/public/mojom/network_service.mojom index 0a556bf46e7b4..41e18bbf3f924 100644 --- a/services/network/public/mojom/network_service.mojom +++ b/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); }; diff --git a/services/network/test/url_loader_context_for_tests.cc b/services/network/test/url_loader_context_for_tests.cc index c4969e12c53fd..805f89a15d7ae 100644 --- a/services/network/test/url_loader_context_for_tests.cc +++ b/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 diff --git a/services/network/test/url_loader_context_for_tests.h b/services/network/test/url_loader_context_for_tests.h index d25cc252dfade..5a2f2db2d6b2d 100644 --- a/services/network/test/url_loader_context_for_tests.h +++ b/services/network/test/url_loader_context_for_tests.h @@ -54,6 +54,7 @@ class URLLoaderContextForTests : public URLLoaderContext { scoped_refptr GetResourceSchedulerClient() const override; corb::PerFactoryState& GetMutableCorbState() override; + bool DataUseUpdatesEnabled() override; private: mojom::URLLoaderFactoryParams factory_params_; diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc index 260c0d761c657..d385292aa95cc 100644 --- a/services/network/url_loader.cc +++ b/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()) { diff --git a/services/network/url_loader.h b/services/network/url_loader.h index 7293516425c03..4b2017860a110 100644 --- a/services/network/url_loader.h +++ b/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 weak_ptr_factory_{this}; }; diff --git a/services/network/url_loader_context.h b/services/network/url_loader_context.h index 4b61d5aa48a71..c8fafa2605362 100644 --- a/services/network/url_loader_context.h +++ b/services/network/url_loader_context.h @@ -53,6 +53,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoaderContext { virtual scoped_refptr GetResourceSchedulerClient() const = 0; virtual corb::PerFactoryState& GetMutableCorbState() = 0; + virtual bool DataUseUpdatesEnabled() = 0; protected: // `protected` destructor = can only destruct via concrete implementations diff --git a/services/network/url_loader_factory.cc b/services/network/url_loader_factory.cc index b86541a4b61d5..b6b7b1335f559 100644 --- a/services/network/url_loader_factory.cc +++ b/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 receiver, int32_t request_id, diff --git a/services/network/url_loader_factory.h b/services/network/url_loader_factory.h index de281673dcaa5..2c2e820882419 100644 --- a/services/network/url_loader_factory.h +++ b/services/network/url_loader_factory.h @@ -84,6 +84,7 @@ class URLLoaderFactory : public mojom::URLLoaderFactory, scoped_refptr GetResourceSchedulerClient() const override; corb::PerFactoryState& GetMutableCorbState() override; + bool DataUseUpdatesEnabled() override; // Allows starting a URLLoader with a synchronous URLLoaderClient as an // optimization.