[go: nahoru, domu]

[gardener] Revert "Cleanup network::mojom::URLResponseHead"

This reverts commit 9c9653781023db9bbd86c2e873bffdcbaae25e28.

Reason for revert:
I think this is causing MSan failures, e.g.
https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/39949

->
    #0 0x55d134f77a97 in blink::ResourceFetcher::HandleLoaderFinish(blink::Resource*, base::TimeTicks, blink::ResourceFetcher::LoaderFinishType, unsigned int) ./../../third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc:2105:9

is accessing
    if (resource->GetResponse().ShouldPopulateResourceTiming())

which seems uninitialized since this CL.

Original change's description:
> Cleanup network::mojom::URLResponseHead
>
> While investigating https://crbug.com/1507770 I noticed two things:
>  1. ct_policy_compliance is not used any more.
>     - ct_policy_compliance was introduced by https://crrev.com/534288.
>     - It is not used after https://crrev.com/957351.
>  2. web_bundle_url is used only checking if the response is an inner
>     response of a WebBundle.
>
> So we can remove ct_policy_compliance param, and replace web_bundle_url
> param with a new bool param.
>
> The size of network::mojom::URLResponseHead decreases from 1072 bytes to
> 944 bytes.
>
> Bug: 1507770
> Change-Id: Iadf6ec88b04103141cb7f0c534af9b4ca6b2237c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5082541
> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1233197}

Bug: 1507770
Change-Id: I0f0e9d7cc049ed2ebfca1278ae0d4ebfa9b95848
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5087933
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Pavol Marko <pmarko@chromium.org>
Auto-Submit: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1233264}
diff --git a/services/network/public/mojom/url_response_head.mojom b/services/network/public/mojom/url_response_head.mojom
index 5803d57..8aff4a01 100644
--- a/services/network/public/mojom/url_response_head.mojom
+++ b/services/network/public/mojom/url_response_head.mojom
@@ -49,6 +49,9 @@
   // response's mime type.  This may be a derived value.
   string charset;
 
+  // The resource's compliance with the Certificate Transparency policy.
+  CTPolicyCompliance ct_policy_compliance;
+
   // Content length if available. -1 if not available
   int64 content_length = -1;
 
@@ -195,9 +198,6 @@
   // True if the response is an inner response of a signed exchange.
   bool is_signed_exchange_inner_response = false;
 
-  // True if the response is an inner response of a WebBundle.
-  bool is_web_bundle_inner_response = false;
-
   // True if this resource is served from the prefetch cache.
   bool was_in_prefetch_cache = false;
 
@@ -241,6 +241,10 @@
   // address used for the connection, in no particular order.
   array<string> dns_aliases;
 
+  // The URL of WebBundle this response was loaded from. This value is only
+  // populated for resources loaded from a WebBundle.
+  url.mojom.Url web_bundle_url;
+
   // True when there is an "authorization" header on the request and it is
   // covered by the wildcard in the preflight response.
   // TODO(crbug.com/1176753): Remove this once the investigation is done.
diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
index f91b4ac..5f99d15 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -1355,6 +1355,8 @@
     url_request_->GetLoadTimingInfo(&response->load_timing);
 
   if (url_request_->ssl_info().cert.get()) {
+    response->ct_policy_compliance =
+        url_request_->ssl_info().ct_policy_compliance;
     response->cert_status = url_request_->ssl_info().cert_status;
     if ((options_ & mojom::kURLLoadOptionSendSSLInfoWithResponse) ||
         (net::IsCertStatusError(url_request_->ssl_info().cert_status) &&
diff --git a/services/network/web_bundle/web_bundle_manager_unittest.cc b/services/network/web_bundle/web_bundle_manager_unittest.cc
index fa102d6..28e48ccd 100644
--- a/services/network/web_bundle/web_bundle_manager_unittest.cc
+++ b/services/network/web_bundle/web_bundle_manager_unittest.cc
@@ -336,7 +336,7 @@
     req.client->RunUntilComplete();
 
     EXPECT_EQ(net::OK, req.client->completion_status().error_code);
-    EXPECT_TRUE(req.client->response_head()->is_web_bundle_inner_response);
+    EXPECT_EQ(req.client->response_head()->web_bundle_url, GURL(kBundleUrl));
     std::string body;
     EXPECT_TRUE(
         mojo::BlockingCopyToString(req.client->response_body_release(), &body));
@@ -573,7 +573,7 @@
   // Confirm that the subresource is correctly loaded.
   client1_1->RunUntilComplete();
   EXPECT_EQ(net::OK, client1_1->completion_status().error_code);
-  EXPECT_TRUE(client1_1->response_head()->is_web_bundle_inner_response);
+  EXPECT_EQ(client1_1->response_head()->web_bundle_url, GURL(kBundleUrl));
   std::string body1_1;
   EXPECT_TRUE(
       mojo::BlockingCopyToString(client1_1->response_body_release(), &body1_1));
@@ -596,7 +596,7 @@
   // Confirm that the subresource is correctly loaded.
   client1_2->RunUntilComplete();
   EXPECT_EQ(net::OK, client1_2->completion_status().error_code);
-  EXPECT_TRUE(client1_2->response_head()->is_web_bundle_inner_response);
+  EXPECT_EQ(client1_2->response_head()->web_bundle_url, GURL(kBundleUrl));
   std::string body1_2;
   EXPECT_TRUE(
       mojo::BlockingCopyToString(client1_2->response_body_release(), &body1_2));
@@ -642,7 +642,7 @@
   // Confirm that the subresource is correctly loaded.
   client2->RunUntilComplete();
   EXPECT_EQ(net::OK, client2->completion_status().error_code);
-  EXPECT_TRUE(client2->response_head()->is_web_bundle_inner_response);
+  EXPECT_EQ(client2->response_head()->web_bundle_url, GURL(kBundleUrl));
   std::string body2;
   EXPECT_TRUE(
       mojo::BlockingCopyToString(client2->response_body_release(), &body2));
diff --git a/services/network/web_bundle/web_bundle_url_loader_factory.cc b/services/network/web_bundle/web_bundle_url_loader_factory.cc
index 70bdf18..af8ba7cc 100644
--- a/services/network/web_bundle/web_bundle_url_loader_factory.cc
+++ b/services/network/web_bundle/web_bundle_url_loader_factory.cc
@@ -878,7 +878,7 @@
     return;
   }
 
-  response_head->is_web_bundle_inner_response = true;
+  response_head->web_bundle_url = bundle_url_;
 
   response_head->load_timing = loader->load_timing();
   loader->SetBodyLength(payload_length);
diff --git a/services/network/web_bundle/web_bundle_url_loader_factory_unittest.cc b/services/network/web_bundle/web_bundle_url_loader_factory_unittest.cc
index e8b1534..6fee889 100644
--- a/services/network/web_bundle/web_bundle_url_loader_factory_unittest.cc
+++ b/services/network/web_bundle/web_bundle_url_loader_factory_unittest.cc
@@ -256,7 +256,7 @@
 
   EXPECT_EQ(net::OK, request.client->completion_status().error_code);
   EXPECT_FALSE(last_bundle_error().has_value());
-  EXPECT_TRUE(request.client->response_head()->is_web_bundle_inner_response);
+  EXPECT_EQ(request.client->response_head()->web_bundle_url, GURL(kBundleUrl));
   std::string body;
   EXPECT_TRUE(mojo::BlockingCopyToString(
       request.client->response_body_release(), &body));
diff --git a/third_party/blink/public/platform/web_url_response.h b/third_party/blink/public/platform/web_url_response.h
index dee1763..5cc66b9 100644
--- a/third_party/blink/public/platform/web_url_response.h
+++ b/third_party/blink/public/platform/web_url_response.h
@@ -272,7 +272,6 @@
   void SetEncodedBodyLength(uint64_t);
 
   void SetIsSignedExchangeInnerResponse(bool);
-  void SetIsWebBundleInnerResponse(bool);
   void SetWasInPrefetchCache(bool);
   void SetWasCookieInRequest(bool);
   void SetRecursivePrefetchToken(const absl::optional<base::UnguessableToken>&);
@@ -285,6 +284,9 @@
   // through to query name.
   void SetDnsAliases(const WebVector<WebString>&);
 
+  WebURL WebBundleURL() const;
+  void SetWebBundleURL(const WebURL&);
+
   void SetAuthChallengeInfo(const absl::optional<net::AuthChallengeInfo>&);
   const absl::optional<net::AuthChallengeInfo>& AuthChallengeInfo() const;
 
diff --git a/third_party/blink/renderer/platform/exported/web_url_response.cc b/third_party/blink/renderer/platform/exported/web_url_response.cc
index 04d456fe..2c28adb7 100644
--- a/third_party/blink/renderer/platform/exported/web_url_response.cc
+++ b/third_party/blink/renderer/platform/exported/web_url_response.cc
@@ -205,10 +205,10 @@
   response.SetRequestId(request_id);
   response.SetIsSignedExchangeInnerResponse(
       head.is_signed_exchange_inner_response);
-  response.SetIsWebBundleInnerResponse(head.is_web_bundle_inner_response);
   response.SetWasInPrefetchCache(head.was_in_prefetch_cache);
   response.SetWasCookieInRequest(head.was_cookie_in_request);
   response.SetRecursivePrefetchToken(head.recursive_prefetch_token);
+  response.SetWebBundleURL(KURL(head.web_bundle_url));
   response.SetTriggerVerifications(head.trigger_verifications);
 
   SetSecurityStyleAndDetails(GURL(KURL(url)), head, &response,
@@ -639,11 +639,6 @@
       is_signed_exchange_inner_response);
 }
 
-void WebURLResponse::SetIsWebBundleInnerResponse(
-    bool is_web_bundle_inner_response) {
-  resource_response_->SetIsWebBundleInnerResponse(is_web_bundle_inner_response);
-}
-
 void WebURLResponse::SetWasInPrefetchCache(bool was_in_prefetch_cache) {
   resource_response_->SetWasInPrefetchCache(was_in_prefetch_cache);
 }
@@ -725,6 +720,14 @@
   resource_response_->SetDnsAliases(std::move(dns_aliases));
 }
 
+WebURL WebURLResponse::WebBundleURL() const {
+  return resource_response_->WebBundleURL();
+}
+
+void WebURLResponse::SetWebBundleURL(const WebURL& url) {
+  resource_response_->SetWebBundleURL(url);
+}
+
 void WebURLResponse::SetAuthChallengeInfo(
     const absl::optional<net::AuthChallengeInfo>& auth_challenge_info) {
   resource_response_->SetAuthChallengeInfo(auth_challenge_info);
diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_response.cc b/third_party/blink/renderer/platform/loader/fetch/resource_response.cc
index 8360c9db..f4c88c7 100644
--- a/third_party/blink/renderer/platform/loader/fetch/resource_response.cc
+++ b/third_party/blink/renderer/platform/loader/fetch/resource_response.cc
@@ -104,7 +104,7 @@
 }
 
 bool ResourceResponse::ShouldPopulateResourceTiming() const {
-  return IsHTTP() || is_web_bundle_inner_response_;
+  return IsHTTP() || WebBundleURL().IsValid();
 }
 
 const KURL& ResourceResponse::CurrentRequestUrl() const {
diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_response.h b/third_party/blink/renderer/platform/loader/fetch/resource_response.h
index e6b8494e..1a67b0a 100644
--- a/third_party/blink/renderer/platform/loader/fetch/resource_response.h
+++ b/third_party/blink/renderer/platform/loader/fetch/resource_response.h
@@ -88,8 +88,7 @@
   // When serving resources from a WebBundle, we might have resources whose
   // source isn't a URL (like urn:uuid), but we still need to create and
   // populate ResourceTiming entries for them, so we need to check that either
-  // response has a proper request URL or whether the response is an inner
-  // response of a WebBundle.
+  // response has a proper request URL or a WebBundleURL.
   bool ShouldPopulateResourceTiming() const;
 
   // The current request URL for this resource (the URL after redirects).
@@ -211,6 +210,9 @@
   const absl::optional<net::SSLInfo>& GetSSLInfo() const { return ssl_info_; }
   void SetSSLInfo(const net::SSLInfo& ssl_info);
 
+  const KURL& WebBundleURL() const { return web_bundle_url_; }
+  void SetWebBundleURL(const KURL& url) { web_bundle_url_ = url; }
+
   bool EmittedExtraInfo() const { return emitted_extra_info_; }
   void SetEmittedExtraInfo(bool emitted_extra_info) {
     emitted_extra_info_ = emitted_extra_info;
@@ -423,10 +425,6 @@
     is_signed_exchange_inner_response_ = is_signed_exchange_inner_response;
   }
 
-  void SetIsWebBundleInnerResponse(bool is_web_bundle_inner_response) {
-    is_web_bundle_inner_response_ = is_web_bundle_inner_response;
-  }
-
   bool WasInPrefetchCache() const { return was_in_prefetch_cache_; }
 
   void SetWasInPrefetchCache(bool was_in_prefetch_cache) {
@@ -551,9 +549,6 @@
   // https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html
   bool is_signed_exchange_inner_response_ : 1;
 
-  // True if this resource is an inner response of a WebBundle.
-  bool is_web_bundle_inner_response_ : 1;
-
   // True if this resource is served from the prefetch cache.
   bool was_in_prefetch_cache_ : 1;
 
@@ -684,6 +679,10 @@
   // address used for the connection, in no particular order.
   Vector<String> dns_aliases_;
 
+  // The URL of WebBundle this response was loaded from. This value is only
+  // populated for resources loaded from a WebBundle.
+  KURL web_bundle_url_;
+
   absl::optional<net::AuthChallengeInfo> auth_challenge_info_;
 
   bool emitted_extra_info_ = false;