[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;