Add KeepaliveRequestFinished UKM event

Adding a UKM similar to Extensions.WebRequest.KeepaliveRequestState.
Proposal: https://docs.google.com/document/d/1XYsa2W1zjWIvrpuAq7IA1wEGZIvbJLoiPF6rMob01Tg/edit

Bug: 1098251
Change-Id: I6bccb85935646bb9ed1879ec19fb409323807124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2262402
Auto-Submit: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782836}

extensions/browser/api/DEPS

 include_rules = [
   "+components/device_event_log",
+  "+components/ukm",
   "+services/device/public",
+  "+services/metrics/public/cpp",
   "+storage/browser/file_system",
   "+storage/common/file_system",
   "+third_party/openscreen/src/cast/common/certificate/proto",

extensions/browser/api/web_request/BUILD.gn

@@ -43,6 +43,7 @@ source_set("web_request") {
   ]
 
   deps = [
+    "//components/ukm/content",
     "//components/web_cache/browser",
     "//content/public/browser",
     "//content/public/common",

extensions/browser/api/web_request/web_request_api.cc

@@ -26,6 +26,7 @@
 #include "base/task/post_task.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "components/ukm/content/source_url_recorder.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -689,11 +690,11 @@ bool WebRequestAPI::MaybeProxyURLLoaderFactory(
     mojo::PendingRemote<network::mojom::TrustedURLLoaderHeaderClient>*
         header_client) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto* web_contents = content::WebContents::FromRenderFrameHost(frame);
   if (!MayHaveProxies()) {
     bool skip_proxy = true;
     // There are a few internal WebUIs that use WebView tag that are whitelisted
     // for webRequest.
-    auto* web_contents = content::WebContents::FromRenderFrameHost(frame);
     if (web_contents && WebViewGuest::IsGuest(web_contents)) {
       auto* guest_web_contents =
           WebViewGuest::GetTopLevelWebContents(web_contents);
@@ -742,12 +743,15 @@ bool WebRequestAPI::MaybeProxyURLLoaderFactory(
          (browser_context->IsOffTheRecord() &&
           ExtensionsBrowserClient::Get()->GetOriginalContext(browser_context) ==
               browser_context_));
+  const ukm::SourceId ukm_source_id =
+      web_contents ? ukm::GetSourceIdForWebContentsDocument(web_contents)
+                   : ukm::kInvalidSourceId;
   WebRequestProxyingURLLoaderFactory::StartProxying(
       browser_context, is_navigation ? -1 : render_process_id,
       &request_id_generator_, std::move(navigation_ui_data),
       std::move(navigation_id), std::move(proxied_receiver),
       std::move(target_factory_remote), std::move(header_client_receiver),
-      proxies_.get(), type);
+      proxies_.get(), type, ukm_source_id);
   return true;
 }
 

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc

@@ -22,6 +22,7 @@
 #include "extensions/browser/api/web_request/permission_helper.h"
 #include "extensions/browser/extension_navigation_ui_data.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension_features.h"
 #include "extensions/common/manifest_handlers/web_accessible_resources_info.h"
 #include "net/base/completion_repeating_callback.h"
 #include "net/http/http_response_headers.h"
@@ -30,6 +31,9 @@
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/redirect_util.h"
 #include "net/url_request/url_request.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
+#include "services/metrics/public/cpp/ukm_recorder.h"
+#include "services/metrics/public/cpp/ukm_source_id.h"
 #include "services/network/public/cpp/features.h"
 #include "third_party/blink/public/common/loader/throttling_url_loader.h"
 #include "third_party/blink/public/platform/resource_request_blocked_reason.h"
@@ -94,6 +98,7 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
     int32_t network_service_request_id,
     int32_t routing_id,
     uint32_t options,
+    ukm::SourceId ukm_source_id,
     const network::ResourceRequest& request,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     mojo::PendingReceiver<network::mojom::URLLoader> loader_receiver,
@@ -105,6 +110,7 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
       network_service_request_id_(network_service_request_id),
       routing_id_(routing_id),
       options_(options),
+      ukm_source_id_(ukm_source_id),
       traffic_annotation_(traffic_annotation),
       proxied_loader_receiver_(this, std::move(loader_receiver)),
       target_client_(std::move(client)),
@@ -132,6 +138,7 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::InProgressRequest(
       request_(request),
       original_initiator_(request.request_initiator),
       request_id_(request_id),
+      ukm_source_id_(ukm::kInvalidSourceId),
       proxied_loader_receiver_(this),
       for_cors_preflight_(true),
       has_any_extra_headers_listeners_(
@@ -143,6 +150,13 @@ WebRequestProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() {
   if (request_.keepalive && !for_cors_preflight_) {
     UMA_HISTOGRAM_ENUMERATION("Extensions.WebRequest.KeepaliveRequestState",
                               state_);
+    if (base::FeatureList::IsEnabled(
+            extensions_features::kReportKeepaliveUkm)) {
+      ukm::builders::Extensions_WebRequest_KeepaliveRequestFinished(
+          ukm_source_id_)
+          .SetState(state_)
+          .Record(ukm::UkmRecorder::Get());
+    }
   }
   // This is important to ensure that no outstanding blocking requests continue
   // to reference state owned by this object.
@@ -1088,14 +1102,16 @@ WebRequestProxyingURLLoaderFactory::WebRequestProxyingURLLoaderFactory(
     mojo::PendingReceiver<network::mojom::TrustedURLLoaderHeaderClient>
         header_client_receiver,
     WebRequestAPI::ProxySet* proxies,
-    content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type)
+    content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type,
+    ukm::SourceId ukm_source_id)
     : browser_context_(browser_context),
       render_process_id_(render_process_id),
       request_id_generator_(request_id_generator),
       navigation_ui_data_(std::move(navigation_ui_data)),
       navigation_id_(std::move(navigation_id)),
       proxies_(proxies),
-      loader_factory_type_(loader_factory_type) {
+      loader_factory_type_(loader_factory_type),
+      ukm_source_id_(ukm_source_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // base::Unretained is safe here because the callback will be
   // canceled when |shutdown_notifier_| is destroyed, and |proxies_|
@@ -1130,14 +1146,16 @@ void WebRequestProxyingURLLoaderFactory::StartProxying(
     mojo::PendingReceiver<network::mojom::TrustedURLLoaderHeaderClient>
         header_client_receiver,
     WebRequestAPI::ProxySet* proxies,
-    content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type) {
+    content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type,
+    ukm::SourceId ukm_source_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   auto proxy = std::make_unique<WebRequestProxyingURLLoaderFactory>(
       browser_context, render_process_id, request_id_generator,
       std::move(navigation_ui_data), std::move(navigation_id),
       std::move(loader_receiver), std::move(target_factory_remote),
-      std::move(header_client_receiver), proxies, loader_factory_type);
+      std::move(header_client_receiver), proxies, loader_factory_type,
+      ukm_source_id);
 
   proxies->AddProxy(std::move(proxy));
 }
@@ -1178,10 +1196,10 @@ void WebRequestProxyingURLLoaderFactory::CreateLoaderAndStart(
   }
 
   auto result = requests_.emplace(
-      web_request_id,
-      std::make_unique<InProgressRequest>(
-          this, web_request_id, request_id, routing_id, options, request,
-          traffic_annotation, std::move(loader_receiver), std::move(client)));
+      web_request_id, std::make_unique<InProgressRequest>(
+                          this, web_request_id, request_id, routing_id, options,
+                          ukm_source_id_, request, traffic_annotation,
+                          std::move(loader_receiver), std::move(client)));
   result.first->second->Restart();
 }
 

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h

@@ -25,6 +25,7 @@
 #include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/completion_once_callback.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/metrics/public/cpp/ukm_source_id.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
@@ -54,6 +55,7 @@ class WebRequestProxyingURLLoaderFactory
         int32_t routing_id,
         int32_t network_service_request_id,
         uint32_t options,
+        ukm::SourceId ukm_source_id,
         const network::ResourceRequest& request,
         const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
         mojo::PendingReceiver<network::mojom::URLLoader> loader_receiver,
@@ -171,6 +173,7 @@ class WebRequestProxyingURLLoaderFactory
     const int32_t network_service_request_id_ = 0;
     const int32_t routing_id_ = 0;
     const uint32_t options_ = 0;
+    const ukm::SourceId ukm_source_id_;
     const net::MutableNetworkTrafficAnnotationTag traffic_annotation_;
     mojo::Receiver<network::mojom::URLLoader> proxied_loader_receiver_;
     mojo::Remote<network::mojom::URLLoaderClient> target_client_;
@@ -244,7 +247,8 @@ class WebRequestProxyingURLLoaderFactory
       mojo::PendingReceiver<network::mojom::TrustedURLLoaderHeaderClient>
           header_client_receiver,
       WebRequestAPI::ProxySet* proxies,
-      content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type);
+      content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type,
+      ukm::SourceId ukm_source_id);
 
   ~WebRequestProxyingURLLoaderFactory() override;
 
@@ -260,7 +264,8 @@ class WebRequestProxyingURLLoaderFactory
       mojo::PendingReceiver<network::mojom::TrustedURLLoaderHeaderClient>
           header_client_receiver,
       WebRequestAPI::ProxySet* proxies,
-      content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type);
+      content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type,
+      ukm::SourceId ukm_source_id);
 
   // network::mojom::URLLoaderFactory:
   void CreateLoaderAndStart(
@@ -320,6 +325,9 @@ class WebRequestProxyingURLLoaderFactory
 
   const content::ContentBrowserClient::URLLoaderFactoryType
       loader_factory_type_;
+  // A UKM source ID associated with the content::WebContents of the initiator
+  // frame.
+  const ukm::SourceId ukm_source_id_;
 
   // Mapping from our own internally generated request ID to an
   // InProgressRequest instance.

extensions/common/extension_features.cc

@@ -43,4 +43,8 @@ const base::Feature kAllowWithholdingExtensionPermissionsOnInstall{
     "AllowWithholdingExtensionPermissionsOnInstall",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Reports Extensions.WebRequest.KeepaliveRequestFinished when enabled.
+const base::Feature kReportKeepaliveUkm{"ReportKeepaliveUkm",
+                                        base::FEATURE_ENABLED_BY_DEFAULT};
+
 }  // namespace extensions_features

extensions/common/extension_features.h

@@ -25,6 +25,8 @@ extern const base::Feature kForceWebRequestProxyForTest;
 
 extern const base::Feature kAllowWithholdingExtensionPermissionsOnInstall;
 
+extern const base::Feature kReportKeepaliveUkm;
+
 }  // namespace extensions_features
 
 #endif  // EXTENSIONS_COMMON_EXTENSION_FEATURES_H_

tools/metrics/ukm/ukm.xml

@@ -3812,6 +3812,20 @@ be describing additional metrics about the same event.
   </metric>
 </event>
 
+<event name="Extensions.WebRequest.KeepaliveRequestFinished">
+  <owner>yhirano@chromium.org</owner>
+  <owner>kinuko@chromium.org</owner>
+  <summary>
+    Whether and how keepalive requests are blocked. Recorded when a keepalive
+    request finishes.
+  </summary>
+  <metric name="State" enum="ExtensionInProgressRequestState">
+    <summary>
+      The final state of the request.
+    </summary>
+  </metric>
+</event>
+
 <event name="FontMatchAttempts">
   <owner>caraitto@chromium.org</owner>
   <owner>pauljensen@chromium.org</owner>