FrameHost::SubresourceResponseStarted: s/ url / final_response_url /.

This CL changes the following method of content::mojom::FrameHost from:

  SubresourceResponseStarted(url.mojom.Url url, uint32 cert_status);

to:

  SubresourceResponseStarted(
      url.mojom.SchemeHostPort final_response_url, uint32 cert_status);

because it turns out that implementations of that method only ever
depend on the SchemeHostPort of the final URL (and not on other parts of
the full URL like path or query).

Note that this is a revival the following CL using SchemeHostPort
instead of Origin:
https://chromium-review.googlesource.com/c/chromium/src/+/1899739
Since Origin normalizes blob and filesystem, we cannot use it as
a simple replacement of GURL.  It caused crbug.com/1049625.

Bug: 973885
Change-Id: I3542be935522bc2abc6f2259d56dd175eabbb3ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3637099
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002483}

components/page_load_metrics/browser/page_load_metrics_observer_interface.h

@@ -64,7 +64,8 @@ struct ExtraRequestCompleteInfo {
   // The origin of the final URL for the request (final = after redirects).
   //
   // The full URL is not available, because in some cases the path and query
-  // be sanitized away - see https://crbug.com/973885.
+  // may be sanitized away - see https://crbug.com/973885.
+  // TODO(crbug.com/973885): use url::SchemeHostPort if applicable.
   const url::Origin origin_of_final_url;
 
   // The host (IP address) and port for the request.

components/page_load_metrics/renderer/metrics_render_frame_observer.cc

@@ -157,7 +157,7 @@ void MetricsRenderFrameObserver::DidObserveLayoutNg(uint32_t all_block_count,
 }
 
 void MetricsRenderFrameObserver::DidStartResponse(
-    const GURL& response_url,
+    const url::SchemeHostPort& final_response_url,
     int request_id,
     const network::mojom::URLResponseHead& response_head,
     network::mojom::RequestDestination request_destination) {
@@ -168,10 +168,10 @@ void MetricsRenderFrameObserver::DidStartResponse(
     // case. There should be a guarantee that DidStartProvisionalLoad be called
     // before DidStartResponse for the frame request.
     provisional_frame_resource_data_use_->DidStartResponse(
-        response_url, request_id, response_head, request_destination);
+        final_response_url, request_id, response_head, request_destination);
   } else if (page_timing_metrics_sender_) {
     page_timing_metrics_sender_->DidStartResponse(
-        response_url, request_id, response_head, request_destination);
+        final_response_url, request_id, response_head, request_destination);
     UpdateResourceMetadata(request_id);
   }
 }

components/page_load_metrics/renderer/metrics_render_frame_observer.h

@@ -63,7 +63,7 @@ class MetricsRenderFrameObserver
                           uint32_t all_call_count,
                           uint32_t ng_call_count) override;
   void DidStartResponse(
-      const GURL& response_url,
+      const url::SchemeHostPort& final_response_url,
       int request_id,
       const network::mojom::URLResponseHead& response_head,
       network::mojom::RequestDestination request_destination) override;

components/page_load_metrics/renderer/page_resource_data_use.cc

@@ -8,6 +8,7 @@
 #include "services/network/public/mojom/url_response_head.mojom.h"
 #include "third_party/blink/public/common/loader/resource_type_util.h"
 #include "url/gurl.h"
+#include "url/scheme_host_port.h"
 
 namespace page_load_metrics {
 
@@ -30,7 +31,7 @@ PageResourceDataUse::PageResourceDataUse(const PageResourceDataUse& other) =
 PageResourceDataUse::~PageResourceDataUse() = default;
 
 void PageResourceDataUse::DidStartResponse(
-    const GURL& response_url,
+    const url::SchemeHostPort& final_response_url,
     int resource_id,
     const network::mojom::URLResponseHead& response_head,
     network::mojom::RequestDestination request_destination) {
@@ -40,7 +41,7 @@ void PageResourceDataUse::DidStartResponse(
   mime_type_ = response_head.mime_type;
   if (response_head.was_fetched_via_cache)
     cache_type_ = mojom::CacheType::kHttp;
-  is_secure_scheme_ = response_url.SchemeIsCryptographic();
+  is_secure_scheme_ = GURL::SchemeIsCryptographic(final_response_url.scheme());
   is_primary_frame_resource_ =
       blink::IsRequestDestinationFrame(request_destination);
 }

components/page_load_metrics/renderer/page_resource_data_use.h

@@ -15,6 +15,10 @@ namespace network {
 struct URLLoaderCompletionStatus;
 }  // namespace network
 
+namespace url {
+class SchemeHostPort;
+}  // namespace url
+
 namespace page_load_metrics {
 
 // PageResourceDataUse contains the data use information of one resource. Data
@@ -28,7 +32,7 @@ class PageResourceDataUse {
 
   ~PageResourceDataUse();
 
-  void DidStartResponse(const GURL& response_url,
+  void DidStartResponse(const url::SchemeHostPort& final_response_url,
                         int resource_id,
                         const network::mojom::URLResponseHead& response_head,
                         network::mojom::RequestDestination request_destination);

components/page_load_metrics/renderer/page_timing_metrics_sender.cc

@@ -125,7 +125,7 @@ void PageTimingMetricsSender::DidObserveMobileFriendlinessChanged(
 }
 
 void PageTimingMetricsSender::DidStartResponse(
-    const GURL& response_url,
+    const url::SchemeHostPort& final_response_url,
     int resource_id,
     const network::mojom::URLResponseHead& response_head,
     network::mojom::RequestDestination request_destination) {
@@ -135,7 +135,7 @@ void PageTimingMetricsSender::DidStartResponse(
       std::piecewise_construct, std::forward_as_tuple(resource_id),
       std::forward_as_tuple(std::make_unique<PageResourceDataUse>()));
   resource_it.first->second->DidStartResponse(
-      response_url, resource_id, response_head, request_destination);
+      final_response_url, resource_id, response_head, request_destination);
 }
 
 void PageTimingMetricsSender::DidReceiveTransferSizeUpdate(

components/page_load_metrics/renderer/page_timing_metrics_sender.h

@@ -58,7 +58,7 @@ class PageTimingMetricsSender {
                           uint32_t ng_call_count);
   void DidObserveMobileFriendlinessChanged(const blink::MobileFriendliness&);
 
-  void DidStartResponse(const GURL& response_url,
+  void DidStartResponse(const url::SchemeHostPort& final_response_url,
                         int resource_id,
                         const network::mojom::URLResponseHead& response_head,
                         network::mojom::RequestDestination request_destination);

content/browser/renderer_host/render_frame_host_impl.cc

@@ -7426,12 +7426,13 @@ void RenderFrameHostImpl::BeginNavigation(
 }
 
 void RenderFrameHostImpl::SubresourceResponseStarted(
-    const GURL& url,
+    const url::SchemeHostPort& final_response_url,
     net::CertStatus cert_status) {
-  OPTIONAL_TRACE_EVENT1(
-      "content", "RenderFrameHostImpl::SubresourceResponseStarted", "url", url);
+  OPTIONAL_TRACE_EVENT1("content",
+                        "RenderFrameHostImpl::SubresourceResponseStarted",
+                        "url", final_response_url.GetURL());
   frame_tree_->controller().ssl_manager()->DidStartResourceResponse(
-      url, cert_status);
+      final_response_url, cert_status);
 }
 
 void RenderFrameHostImpl::ResourceLoadComplete(

content/browser/renderer_host/render_frame_host_impl.h

@@ -2738,7 +2738,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
       mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
       mojo::PendingRemote<blink::mojom::PolicyContainerHostKeepAliveHandle>
           initiator_policy_container_host_keep_alive_handle) override;
-  void SubresourceResponseStarted(const GURL& url,
+  void SubresourceResponseStarted(const url::SchemeHostPort& final_response_url,
                                   net::CertStatus cert_status) override;
   void ResourceLoadComplete(
       blink::mojom::ResourceLoadInfoPtr resource_load_info) override;

content/browser/ssl/ssl_manager.cc

@@ -357,24 +357,28 @@ void SSLManager::OnCertError(std::unique_ptr<SSLErrorHandler> handler) {
   OnCertErrorInternal(std::move(handler));
 }
 
-void SSLManager::DidStartResourceResponse(const GURL& url,
-                                          bool has_certificate_errors) {
-  if (!url.SchemeIsCryptographic() || has_certificate_errors)
+void SSLManager::DidStartResourceResponse(
+    const url::SchemeHostPort& final_response_url,
+    bool has_certificate_errors) {
+  const std::string& scheme = final_response_url.scheme();
+  const std::string& host = final_response_url.host();
+
+  if (!GURL::SchemeIsCryptographic(scheme) || has_certificate_errors)
     return;
 
   // If the scheme is https: or wss and the cert did not have any errors, revoke
   // any previous decisions that have occurred.
   if (!ssl_host_state_delegate_ ||
       !ssl_host_state_delegate_->HasAllowException(
-          url.host(), controller_->DeprecatedGetWebContents())) {
+          host, controller_->DeprecatedGetWebContents())) {
     return;
   }
 
   // If there's no certificate error, a good certificate has been seen, so
   // clear out any exceptions that were made by the user for bad
   // certificates. This intentionally does not apply to cached resources
   // (see https://crbug.com/634553 for an explanation).
-  ssl_host_state_delegate_->RevokeUserAllowExceptions(url.host());
+  ssl_host_state_delegate_->RevokeUserAllowExceptions(host);
 }
 
 void SSLManager::OnCertErrorInternal(std::unique_ptr<SSLErrorHandler> handler) {

content/browser/ssl/ssl_manager.h

@@ -15,6 +15,7 @@
 #include "net/base/net_errors.h"
 #include "net/cert/cert_status_flags.h"
 #include "url/gurl.h"
+#include "url/scheme_host_port.h"
 
 namespace net {
 class SSLInfo;
@@ -67,7 +68,8 @@ class SSLManager {
   NavigationControllerImpl* controller() { return controller_; }
 
   void DidCommitProvisionalLoad(const LoadCommittedDetails& details);
-  void DidStartResourceResponse(const GURL& url, bool has_certificate_errors);
+  void DidStartResourceResponse(const url::SchemeHostPort& final_response_url,
+                                bool has_certificate_errors);
 
   // The following methods are called when a page includes insecure
   // content. These methods update the SSLStatus on the NavigationEntry

content/browser/web_contents/web_contents_impl.cc

@@ -5568,7 +5568,7 @@ void WebContentsImpl::ReadyToCommitNavigation(
         ->controller()
         .ssl_manager()
         ->DidStartResourceResponse(
-            navigation_handle->GetURL(),
+            url::SchemeHostPort(navigation_handle->GetURL()),
             navigation_handle->GetSSLInfo().has_value()
                 ? net::IsCertStatusError(
                       navigation_handle->GetSSLInfo()->cert_status)

content/common/frame.mojom

@@ -58,7 +58,7 @@ import "third_party/blink/public/mojom/window_features/window_features.mojom";
 import "ui/accessibility/mojom/ax_tree_update.mojom";
 import "ui/base/mojom/window_open_disposition.mojom";
 import "ui/gfx/geometry/mojom/geometry.mojom";
-import "url/mojom/origin.mojom";
+import "url/mojom/scheme_host_port.mojom";
 import "url/mojom/url.mojom";
 
 [Native]
@@ -780,9 +780,17 @@ interface FrameHost {
           initiator_policy_container_keep_alive_handle);
 
   // Sent when a subresource response has started.
+  //
+  // |final_response_url| is the final (after all redirects) URL of the
+  // subresource response.  The full URL is not available, because in some
+  // cases the path and query may be sanitized away
+  // - see https://crbug.com/973885.
+  //
   // |cert_status| is the bitmask of status info of the SSL certificate. (see
   // net/cert/cert_status_flags.h).
-  SubresourceResponseStarted(url.mojom.Url url, uint32 cert_status);
+  SubresourceResponseStarted(
+      url.mojom.SchemeHostPort final_response_url,
+      uint32 cert_status);
 
   // Sent when a resource load finished, successfully or not.
   ResourceLoadComplete(blink.mojom.ResourceLoadInfo url_load_info);

content/public/renderer/render_frame_observer.h

@@ -52,6 +52,10 @@ namespace network {
 struct URLLoaderCompletionStatus;
 }  // namespace network
 
+namespace url {
+class SchemeHostPort;
+}  // namespace url
+
 namespace content {
 
 class RendererPpapiHost;
@@ -238,7 +242,7 @@ class CONTENT_EXPORT RenderFrameObserver : public IPC::Listener,
   // Complete or Cancel is guaranteed to be called for a response that started.
   // |request_id| uniquely identifies the request within this render frame.
   virtual void DidStartResponse(
-      const GURL& response_url,
+      const url::SchemeHostPort& final_response_url,
       int request_id,
       const network::mojom::URLResponseHead& response_head,
       network::mojom::RequestDestination request_destination) {}

content/renderer/render_frame_impl.cc

@@ -2523,11 +2523,11 @@ void RenderFrameImpl::NotifyResourceResponseReceived(
     network::mojom::URLResponseHeadPtr response_head,
     network::mojom::RequestDestination request_destination) {
   if (!blink::IsRequestDestinationFrame(request_destination)) {
-    GetFrameHost()->SubresourceResponseStarted(response_url,
-                                               response_head->cert_status);
+    GetFrameHost()->SubresourceResponseStarted(
+        url::SchemeHostPort(response_url), response_head->cert_status);
   }
-  DidStartResponse(response_url, request_id, std::move(response_head),
-                   request_destination);
+  DidStartResponse(url::SchemeHostPort(response_url), request_id,
+                   std::move(response_head), request_destination);
 }
 
 void RenderFrameImpl::NotifyResourceTransferSizeUpdated(
@@ -4395,12 +4395,12 @@ void RenderFrameImpl::DidLoadResourceFromMemoryCache(
 }
 
 void RenderFrameImpl::DidStartResponse(
-    const GURL& response_url,
+    const url::SchemeHostPort& final_response_url,
     int request_id,
     network::mojom::URLResponseHeadPtr response_head,
     network::mojom::RequestDestination request_destination) {
   for (auto& observer : observers_) {
-    observer.DidStartResponse(response_url, request_id, *response_head,
+    observer.DidStartResponse(final_response_url, request_id, *response_head,
                               request_destination);
   }
 }

content/renderer/render_frame_impl.h

@@ -147,6 +147,7 @@ class MediaPermission;
 
 namespace url {
 class Origin;
+class SchemeHostPort;
 }
 
 namespace content {
@@ -736,7 +737,7 @@ class CONTENT_EXPORT RenderFrameImpl
   // browser.
   void OnDroppedNavigation();
 
-  void DidStartResponse(const GURL& response_url,
+  void DidStartResponse(const url::SchemeHostPort& final_response_url,
                         int request_id,
                         network::mojom::URLResponseHeadPtr response_head,
                         network::mojom::RequestDestination request_destination);

content/test/test_render_frame.cc

@@ -190,7 +190,7 @@ class MockFrameHost : public mojom::FrameHost {
       mojo::PendingRemote<blink::mojom::PolicyContainerHostKeepAliveHandle>)
       override {}
 
-  void SubresourceResponseStarted(const GURL& url,
+  void SubresourceResponseStarted(const url::SchemeHostPort& final_response_url,
                                   net::CertStatus cert_status) override {}
 
   void ResourceLoadComplete(

third_party/blink/public/mojom/loader/resource_load_info_notifier.mojom

@@ -19,6 +19,8 @@ interface ResourceLoadInfoNotifier {
                                  network.mojom.URLResponseHead redirect_response);
 
   // Called to notify the response has been received.
+  // TODO(crbug.com/973885): make |response_url| as url.mojom.SchemeHostPort
+  // instead of url.mojom.Url if applicable.
   NotifyResourceResponseReceived(int64 request_id,
                                  url.mojom.Url response_url,
                                  network.mojom.URLResponseHead head,