[devtools] Add request resource for issue reporting

This CL adds a request resource, identified by a devtools navigation
token, that is used to associate a cookie issue with a request.

This is crucial for the front-end, as some cookie issues are reported
during navigation, and without the associated devtools navigation token
the front-end cannot preserve these issues for after the navigation.
Such issues belong logically to the page that navigation navigates to.

Bug: chromium:1060628
Change-Id: If26dd6b78d409f0aba1f2badbefc6da76cb518ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2098632
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757382}

content/browser/devtools/devtools_instrumentation.cc

@@ -611,7 +611,7 @@ void OnCorsPreflightRequestCompleted(
 }
 
 namespace {
-blink::mojom::SameSiteCookieIssueDetailsPtr BuildSameSiteCookieIssueDetails(
+std::vector<blink::mojom::SameSiteCookieExclusionReason> BuildExclusionReasons(
     net::CanonicalCookie::CookieInclusionStatus status) {
   std::vector<blink::mojom::SameSiteCookieExclusionReason> exclusion_reasons;
   if (status.HasExclusionReason(
@@ -625,7 +625,11 @@ blink::mojom::SameSiteCookieIssueDetailsPtr BuildSameSiteCookieIssueDetails(
     exclusion_reasons.push_back(blink::mojom::SameSiteCookieExclusionReason::
                                     ExcludeSameSiteNoneInsecure);
   }
+  return exclusion_reasons;
+}
 
+std::vector<blink::mojom::SameSiteCookieWarningReason> BuildWarningReasons(
+    net::CanonicalCookie::CookieInclusionStatus status) {
   std::vector<blink::mojom::SameSiteCookieWarningReason> warning_reasons;
   if (status.HasWarningReason(
           net::CanonicalCookie::CookieInclusionStatus::
@@ -679,28 +683,43 @@ blink::mojom::SameSiteCookieIssueDetailsPtr BuildSameSiteCookieIssueDetails(
     warning_reasons.push_back(blink::mojom::SameSiteCookieWarningReason::
                                   WarnSameSiteCrossSchemeInsecureUrlStrict);
   }
-
-  return blink::mojom::SameSiteCookieIssueDetails::New(
-      std::move(exclusion_reasons), std::move(warning_reasons));
+  return warning_reasons;
 }
 }  // namespace
 
-void ReportSameSiteCookieIssue(RenderFrameHostImpl* render_frame_host_impl,
-                               const net::CookieWithStatus& excluded_cookie,
-                               const GURL& url,
-                               const GURL& site_for_cookies) {
+void ReportSameSiteCookieIssue(
+    RenderFrameHostImpl* render_frame_host_impl,
+    const net::CookieWithStatus& excluded_cookie,
+    const GURL& url,
+    const net::SiteForCookies& site_for_cookies,
+    blink::mojom::SameSiteCookieOperation operation,
+    const base::Optional<std::string>& devtools_request_id) {
+  blink::mojom::AffectedRequestPtr affected_request;
+  if (devtools_request_id) {
+    // We can report the url here, because if devtools_request_id is set, the
+    // url is the url of the request.
+    affected_request =
+        blink::mojom::AffectedRequest::New(*devtools_request_id, url.spec());
+  }
+  auto affected_cookie = blink::mojom::AffectedCookie::New(
+      excluded_cookie.cookie.Name(), excluded_cookie.cookie.Path(),
+      excluded_cookie.cookie.Domain());
   auto details = blink::mojom::InspectorIssueDetails::New();
+  base::Optional<GURL> optional_site_for_cookies_url;
+  if (!site_for_cookies.IsNull()) {
+    optional_site_for_cookies_url = site_for_cookies.RepresentativeUrl();
+  }
   details->sameSiteCookieIssueDetails =
-      BuildSameSiteCookieIssueDetails(excluded_cookie.status);
-  auto resources = blink::mojom::AffectedResources::New();
-  resources->cookies.push_back(blink::mojom::AffectedCookie::New(
-      excluded_cookie.cookie.Name(), excluded_cookie.cookie.Path(),
-      excluded_cookie.cookie.Domain(), site_for_cookies));
+      blink::mojom::SameSiteCookieIssueDetails::New(
+          std::move(affected_cookie),
+          BuildExclusionReasons(excluded_cookie.status),
+          BuildWarningReasons(excluded_cookie.status), operation,
+          optional_site_for_cookies_url, url, std::move(affected_request));
 
   render_frame_host_impl->AddInspectorIssue(
       blink::mojom::InspectorIssueInfo::New(
           blink::mojom::InspectorIssueCode::kSameSiteCookieIssue,
-          std::move(details), std::move(resources)));
+          std::move(details)));
 }
 
 }  // namespace devtools_instrumentation

content/browser/devtools/devtools_instrumentation.h

@@ -18,6 +18,7 @@
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "services/network/public/mojom/url_response_head.mojom-forward.h"
+#include "third_party/blink/public/mojom/devtools/inspector_issue.mojom-forward.h"
 
 class GURL;
 
@@ -158,10 +159,13 @@ void PortalAttached(RenderFrameHostImpl* render_frame_host_impl);
 void PortalDetached(RenderFrameHostImpl* render_frame_host_impl);
 void PortalActivated(RenderFrameHostImpl* render_frame_host_impl);
 
-void ReportSameSiteCookieIssue(RenderFrameHostImpl* render_frame_host_impl,
-                               const net::CookieWithStatus& excluded_cookie,
-                               const GURL& url,
-                               const GURL& site_for_cookies);
+void ReportSameSiteCookieIssue(
+    RenderFrameHostImpl* render_frame_host_impl,
+    const net::CookieWithStatus& excluded_cookie,
+    const GURL& url,
+    const net::SiteForCookies& site_for_cookies,
+    blink::mojom::SameSiteCookieOperation operation,
+    const base::Optional<std::string>& devtools_request_id);
 }  // namespace devtools_instrumentation
 
 }  // namespace content

content/browser/network_context_client_base_impl.cc

@@ -158,15 +158,17 @@ void NetworkContextClientBase::OnCookiesChanged(
     int32_t routing_id,
     const GURL& url,
     const net::SiteForCookies& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {}
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {}
 
 void NetworkContextClientBase::OnCookiesRead(
     bool is_service_worker,
     int32_t process_id,
     int32_t routing_id,
     const GURL& url,
     const net::SiteForCookies& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {}
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {}
 
 #if defined(OS_ANDROID)
 void NetworkContextClientBase::OnGenerateHttpNegotiateAuthToken(

content/browser/storage_partition_impl.cc

@@ -423,11 +423,15 @@ BrowserContext* GetBrowserContextFromStoragePartition(
 }
 
 // TODO(crbug.com/977040): Remove when no longer needed.
-void DeprecateSameSiteCookies(int process_id,
-                              int routing_id,
-                              const net::CookieStatusList& cookie_list,
-                              const GURL& url,
-                              const GURL& site_for_cookies) {
+
+void DeprecateSameSiteCookies(
+    int process_id,
+    int routing_id,
+    const net::CookieStatusList& cookie_list,
+    const GURL& url,
+    const net::SiteForCookies& site_for_cookies,
+    blink::mojom::SameSiteCookieOperation operation,
+    const base::Optional<std::string>& devtools_request_id) {
   // Navigation requests start in the browser, before process_id is assigned, so
   // the id is set to network::mojom::kBrowserProcessId. In these situations,
   // the routing_id is the frame tree node id, and can be used directly.
@@ -500,7 +504,8 @@ void DeprecateSameSiteCookies(int process_id,
         samesite_none_insecure_cookies = true;
       }
       devtools_instrumentation::ReportSameSiteCookieIssue(
-          root_frame_host, excluded_cookie, url, site_for_cookies);
+          root_frame_host, excluded_cookie, url, site_for_cookies, operation,
+          devtools_request_id);
     }
     if (emit_messages) {
       root_frame_host->AddSameSiteCookieDeprecationMessage(
@@ -583,12 +588,14 @@ int64_t CrossSchemeWarningToContextInt64(
 void ReportCookiesChangedOnUI(
     std::vector<GlobalFrameRoutingId> destinations,
     const GURL& url,
-    const GURL& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {
+    const net::SiteForCookies& site_for_cookies,
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   for (const GlobalFrameRoutingId& id : destinations) {
-    DeprecateSameSiteCookies(id.child_id, id.frame_routing_id, cookie_list, url,
-                             site_for_cookies);
+    DeprecateSameSiteCookies(
+        id.child_id, id.frame_routing_id, cookie_list, url, site_for_cookies,
+        blink::mojom::SameSiteCookieOperation::SetCookie, devtools_request_id);
   }
 
   for (const auto& cookie_and_status : cookie_list) {
@@ -600,7 +607,7 @@ void ReportCookiesChangedOnUI(
             GetWebContentsForStoragePartition(id.child_id, id.frame_routing_id);
         if (!web_contents)
           continue;
-        web_contents->OnCookieChange(url, site_for_cookies,
+        web_contents->OnCookieChange(url, site_for_cookies.RepresentativeUrl(),
                                      cookie_and_status.cookie,
                                      /* blocked_by_policy =*/true);
       }
@@ -610,7 +617,7 @@ void ReportCookiesChangedOnUI(
             GetWebContentsForStoragePartition(id.child_id, id.frame_routing_id);
         if (!web_contents)
           continue;
-        web_contents->OnCookieChange(url, site_for_cookies,
+        web_contents->OnCookieChange(url, site_for_cookies.RepresentativeUrl(),
                                      cookie_and_status.cookie,
                                      /* blocked_by_policy =*/false);
 
@@ -637,13 +644,15 @@ void ReportCookiesChangedOnUI(
 void ReportCookiesReadOnUI(
     std::vector<GlobalFrameRoutingId> destinations,
     const GURL& url,
-    const GURL& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {
+    const net::SiteForCookies& site_for_cookies,
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   for (const GlobalFrameRoutingId& id : destinations) {
-    DeprecateSameSiteCookies(id.child_id, id.frame_routing_id, cookie_list, url,
-                             site_for_cookies);
+    DeprecateSameSiteCookies(
+        id.child_id, id.frame_routing_id, cookie_list, url, site_for_cookies,
+        blink::mojom::SameSiteCookieOperation::ReadCookie, devtools_request_id);
   }
 
   net::CookieList accepted, blocked;
@@ -665,7 +674,8 @@ void ReportCookiesReadOnUI(
           GetWebContentsForStoragePartition(id.child_id, id.frame_routing_id);
       if (!web_contents)
         continue;
-      web_contents->OnCookiesRead(url, site_for_cookies, accepted,
+      web_contents->OnCookiesRead(url, site_for_cookies.RepresentativeUrl(),
+                                  accepted,
                                   /* blocked_by_policy =*/false);
 
       // TODO(https://crbug.com/1046456): Remove after deprecated.
@@ -693,7 +703,8 @@ void ReportCookiesReadOnUI(
           GetWebContentsForStoragePartition(id.child_id, id.frame_routing_id);
       if (!web_contents)
         continue;
-      web_contents->OnCookiesRead(url, site_for_cookies, blocked,
+      web_contents->OnCookiesRead(url, site_for_cookies.RepresentativeUrl(),
+                                  blocked,
                                   /* blocked_by_policy =*/true);
     }
   }
@@ -702,8 +713,9 @@ void ReportCookiesReadOnUI(
 void OnServiceWorkerCookiesReadOnCoreThread(
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
     const GURL& url,
-    const GURL& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {
+    const net::SiteForCookies& site_for_cookies,
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   // Notify all the frames associated with this service worker of its cookie
   // activity.
@@ -713,15 +725,16 @@ void OnServiceWorkerCookiesReadOnCoreThread(
     RunOrPostTaskOnThread(
         FROM_HERE, BrowserThread::UI,
         base::BindOnce(ReportCookiesReadOnUI, *frame_routing_ids, url,
-                       site_for_cookies, cookie_list));
+                       site_for_cookies, cookie_list, devtools_request_id));
   }
 }
 
 void OnServiceWorkerCookiesChangedOnCoreThread(
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context,
     const GURL& url,
-    const GURL& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {
+    const net::SiteForCookies& site_for_cookies,
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   // Notify all the frames associated with this service worker of its cookie
   // activity.
@@ -731,7 +744,7 @@ void OnServiceWorkerCookiesChangedOnCoreThread(
     RunOrPostTaskOnThread(
         FROM_HERE, BrowserThread::UI,
         base::BindOnce(ReportCookiesChangedOnUI, *frame_routing_ids, url,
-                       site_for_cookies, cookie_list));
+                       site_for_cookies, cookie_list, devtools_request_id));
   }
 }
 
@@ -2039,20 +2052,21 @@ void StoragePartitionImpl::OnCookiesChanged(
     int32_t routing_id,
     const GURL& url,
     const net::SiteForCookies& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(initialized_);
   if (is_service_worker) {
     RunOrPostTaskOnThread(
         FROM_HERE, ServiceWorkerContext::GetCoreThreadId(),
-        base::BindOnce(
-            &OnServiceWorkerCookiesChangedOnCoreThread, service_worker_context_,
-            url, site_for_cookies.RepresentativeUrl(), std::move(cookie_list)));
+        base::BindOnce(&OnServiceWorkerCookiesChangedOnCoreThread,
+                       service_worker_context_, url, site_for_cookies,
+                       cookie_list, devtools_request_id));
   } else {
     std::vector<GlobalFrameRoutingId> destination;
     destination.emplace_back(process_id, routing_id);
-    ReportCookiesChangedOnUI(destination, url,
-                             site_for_cookies.RepresentativeUrl(), cookie_list);
+    ReportCookiesChangedOnUI(destination, url, site_for_cookies, cookie_list,
+                             devtools_request_id);
   }
 }
 
@@ -2062,20 +2076,21 @@ void StoragePartitionImpl::OnCookiesRead(
     int32_t routing_id,
     const GURL& url,
     const net::SiteForCookies& site_for_cookies,
-    const std::vector<net::CookieWithStatus>& cookie_list) {
+    const std::vector<net::CookieWithStatus>& cookie_list,
+    const base::Optional<std::string>& devtools_request_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(initialized_);
   if (is_service_worker) {
     RunOrPostTaskOnThread(
         FROM_HERE, ServiceWorkerContext::GetCoreThreadId(),
-        base::BindOnce(
-            &OnServiceWorkerCookiesReadOnCoreThread, service_worker_context_,
-            url, site_for_cookies.RepresentativeUrl(), std::move(cookie_list)));
+        base::BindOnce(&OnServiceWorkerCookiesReadOnCoreThread,
+                       service_worker_context_, url, site_for_cookies,
+                       std::move(cookie_list), devtools_request_id));
   } else {
     std::vector<GlobalFrameRoutingId> destination;
     destination.emplace_back(process_id, routing_id);
-    ReportCookiesReadOnUI(destination, url,
-                          site_for_cookies.RepresentativeUrl(), cookie_list);
+    ReportCookiesReadOnUI(destination, url, site_for_cookies, cookie_list,
+                          devtools_request_id);
   }
 }
 

content/browser/storage_partition_impl.h

@@ -261,14 +261,16 @@ class CONTENT_EXPORT StoragePartitionImpl
       int32_t routing_id,
       const GURL& url,
       const net::SiteForCookies& site_for_cookies,
-      const std::vector<net::CookieWithStatus>& cookie_list) override;
+      const std::vector<net::CookieWithStatus>& cookie_list,
+      const base::Optional<std::string>& devtools_request_id) override;
   void OnCookiesRead(
       bool is_service_worker,
       int32_t process_id,
       int32_t routing_id,
       const GURL& url,
       const net::SiteForCookies& site_for_cookies,
-      const std::vector<net::CookieWithStatus>& cookie_list) override;
+      const std::vector<net::CookieWithStatus>& cookie_list,
+      const base::Optional<std::string>& devtools_request_id) override;
 #if defined(OS_ANDROID)
   void OnGenerateHttpNegotiateAuthToken(
       const std::string& server_auth_token,

content/public/browser/network_context_client_base.h

@@ -70,14 +70,16 @@ class CONTENT_EXPORT NetworkContextClientBase
       int32_t routing_id,
       const GURL& url,
       const net::SiteForCookies& site_for_cookies,
-      const std::vector<net::CookieWithStatus>& cookie_list) override;
+      const std::vector<net::CookieWithStatus>& cookie_list,
+      const base::Optional<std::string>& devtools_request_id) override;
   void OnCookiesRead(
       bool is_service_worker,
       int32_t process_id,
       int32_t routing_id,
       const GURL& url,
       const net::SiteForCookies& site_for_cookies,
-      const std::vector<net::CookieWithStatus>& cookie_list) override;
+      const std::vector<net::CookieWithStatus>& cookie_list,
+      const base::Optional<std::string>& devtools_request_id) override;
 #if defined(OS_ANDROID)
   void OnGenerateHttpNegotiateAuthToken(
       const std::string& server_auth_token,

services/network/public/mojom/network_context.mojom

@@ -772,10 +772,14 @@ interface NetworkContextClient {
   //
   // If |is_service_worker| is false, |process_id| and |routing_id| identify
   // the relevant frame.
+  //
+  // |devtools_request_id| contains the DevTools request id of the request
+  // that triggered the cookie change, if the change was triggered by a request.
+  // May be set regardless of |is_service_worker|.
   OnCookiesChanged(
       bool is_service_worker, int32 process_id, int32 routing_id,
       url.mojom.Url url, SiteForCookies site_for_cookies,
-      array<CookieWithStatus> cookie_list);
+      array<CookieWithStatus> cookie_list, string? devtools_request_id);
 
   // Called when an attempt has been made to read the cookies in |cookie_list|,
   // with the status indicating whether the cookies were actually used or
@@ -794,10 +798,14 @@ interface NetworkContextClient {
   //
   // If |is_service_worker| is false, |process_id| and |routing_id| identify
   // the relevant frame.
+  //
+  // |devtools_request_id| contains the DevTools request id of the request
+  // that triggered the cookie change, if the read was triggered by a request.
+  // May be set regardless of |is_service_worker|.
   OnCookiesRead(
       bool is_service_worker, int32 process_id, int32 routing_id,
       url.mojom.Url url, SiteForCookies site_for_cookies,
-      array<CookieWithStatus> cookie_list);
+      array<CookieWithStatus> cookie_list, string? devtools_request_id);
 
   // Called to generate an auth token for SPNEGO authentication on Android.
   [EnableIf=is_android]