Commit d08d9630 authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

Stop keeping request redirect body copies in Resource.

This CL changes Resource to have ResourceRequestHead instead of
ResourceRequest in redirect chain.
That's because we don't copy request body in Resource after
crrev.com/c/2059889.

Change-Id: Ib85c81d82b2318e8b2f25448f32f97d9182b05f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2072342Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746786}
parent abecb7d9
...@@ -173,7 +173,7 @@ void RawResource::DidAddClient(ResourceClient* c) { ...@@ -173,7 +173,7 @@ void RawResource::DidAddClient(ResourceClient* c) {
RevalidationStartForbiddenScope revalidation_start_forbidden_scope(this); RevalidationStartForbiddenScope revalidation_start_forbidden_scope(this);
RawResourceClient* client = static_cast<RawResourceClient*>(c); RawResourceClient* client = static_cast<RawResourceClient*>(c);
for (const auto& redirect : RedirectChain()) { for (const auto& redirect : RedirectChain()) {
client->RedirectReceived(this, redirect.request_, client->RedirectReceived(this, ResourceRequest(redirect.request_),
redirect.redirect_response_); redirect.redirect_response_);
if (!HasClient(c)) if (!HasClient(c))
return; return;
......
...@@ -472,7 +472,7 @@ static bool CanUseResponse(const ResourceResponse& response, ...@@ -472,7 +472,7 @@ static bool CanUseResponse(const ResourceResponse& response,
return CurrentAge(response, response_timestamp) <= max_life; return CurrentAge(response, response_timestamp) <= max_life;
} }
const ResourceRequest& Resource::LastResourceRequest() const { const ResourceRequestHead& Resource::LastResourceRequest() const {
if (!redirect_chain_.size()) if (!redirect_chain_.size())
return GetResourceRequest(); return GetResourceRequest();
return redirect_chain_.back().request_; return redirect_chain_.back().request_;
...@@ -1067,8 +1067,7 @@ bool Resource::MustRevalidateDueToCacheHeaders(bool allow_stale) const { ...@@ -1067,8 +1067,7 @@ bool Resource::MustRevalidateDueToCacheHeaders(bool allow_stale) const {
GetResourceRequest().CacheControlContainsNoStore(); GetResourceRequest().CacheControlContainsNoStore();
} }
static bool ShouldRevalidateStaleResponse(const ResourceRequest& request, static bool ShouldRevalidateStaleResponse(const ResourceResponse& response,
const ResourceResponse& response,
base::Time response_timestamp) { base::Time response_timestamp) {
base::TimeDelta staleness = response.CacheControlStaleWhileRevalidate(); base::TimeDelta staleness = response.CacheControlStaleWhileRevalidate();
if (staleness.is_zero()) if (staleness.is_zero())
...@@ -1082,15 +1081,14 @@ bool Resource::ShouldRevalidateStaleResponse() const { ...@@ -1082,15 +1081,14 @@ bool Resource::ShouldRevalidateStaleResponse() const {
for (auto& redirect : redirect_chain_) { for (auto& redirect : redirect_chain_) {
// Use |response_timestamp_| since we don't store the timestamp // Use |response_timestamp_| since we don't store the timestamp
// of each redirect response. // of each redirect response.
if (blink::ShouldRevalidateStaleResponse(redirect.request_, if (blink::ShouldRevalidateStaleResponse(redirect.redirect_response_,
redirect.redirect_response_,
response_timestamp_)) { response_timestamp_)) {
return true; return true;
} }
} }
return blink::ShouldRevalidateStaleResponse( return blink::ShouldRevalidateStaleResponse(GetResponse(),
GetResourceRequest(), GetResponse(), response_timestamp_); response_timestamp_);
} }
bool Resource::StaleRevalidationRequested() const { bool Resource::StaleRevalidationRequested() const {
......
...@@ -179,7 +179,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>, ...@@ -179,7 +179,7 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>,
const ResourceRequest& GetResourceRequest() const { const ResourceRequest& GetResourceRequest() const {
return resource_request_; return resource_request_;
} }
const ResourceRequest& LastResourceRequest() const; const ResourceRequestHead& LastResourceRequest() const;
const ResourceResponse* LastResourceResponse() const; const ResourceResponse* LastResourceResponse() const;
virtual void SetRevalidatingRequest(const ResourceRequest&); virtual void SetRevalidatingRequest(const ResourceRequest&);
...@@ -466,13 +466,11 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>, ...@@ -466,13 +466,11 @@ class PLATFORM_EXPORT Resource : public GarbageCollected<Resource>,
DISALLOW_NEW(); DISALLOW_NEW();
public: public:
explicit RedirectPair(const ResourceRequest& request, explicit RedirectPair(const ResourceRequestHead& request,
const ResourceResponse& redirect_response) const ResourceResponse& redirect_response)
: redirect_response_(redirect_response) { : request_(request), redirect_response_(redirect_response) {}
request_.CopyFrom(request);
}
ResourceRequest request_; ResourceRequestHead request_;
ResourceResponse redirect_response_; ResourceResponse redirect_response_;
}; };
const Vector<RedirectPair>& RedirectChain() const { return redirect_chain_; } const Vector<RedirectPair>& RedirectChain() const { return redirect_chain_; }
......
...@@ -2030,10 +2030,11 @@ void ResourceFetcher::EmulateLoadStartedForInspector( ...@@ -2030,10 +2030,11 @@ void ResourceFetcher::EmulateLoadStartedForInspector(
ResourceLoaderOptions options = resource->Options(); ResourceLoaderOptions options = resource->Options();
options.initiator_info.name = initiator_name; options.initiator_info.name = initiator_name;
FetchParameters params(std::move(resource_request), options); FetchParameters params(std::move(resource_request), options);
Context().CanRequest(resource->GetType(), resource->LastResourceRequest(), ResourceRequest last_resource_request(resource->LastResourceRequest());
resource->LastResourceRequest().Url(), params.Options(), Context().CanRequest(resource->GetType(), last_resource_request,
last_resource_request.Url(), params.Options(),
ReportingDisposition::kReport, ReportingDisposition::kReport,
resource->LastResourceRequest().GetRedirectStatus()); last_resource_request.GetRedirectStatus());
DidLoadResourceFromMemoryCache(resource->InspectorId(), resource, DidLoadResourceFromMemoryCache(resource->InspectorId(), resource,
params.GetResourceRequest(), params.GetResourceRequest(),
false /* is_static_data */); false /* is_static_data */);
......
...@@ -804,18 +804,19 @@ bool ResourceLoader::WillFollowRedirect( ...@@ -804,18 +804,19 @@ bool ResourceLoader::WillFollowRedirect(
redirect_response_with_type ? *redirect_response_with_type redirect_response_with_type ? *redirect_response_with_type
: redirect_response; : redirect_response;
// The following two calls may rewrite the new_request.Url() to // The following two calls may rewrite the new_request->Url() to
// something else not for rejecting redirect but for other reasons. // something else not for rejecting redirect but for other reasons.
// E.g. WebFrameTestClient::WillSendRequest() and // E.g. WebFrameTestClient::WillSendRequest() and
// RenderFrameImpl::WillSendRequest(). We should reflect the // RenderFrameImpl::WillSendRequest(). We should reflect the
// rewriting but currently we cannot. So, compare new_request.Url() and // rewriting but currently we cannot. So, compare new_request->Url() and
// new_url after calling them, and return false to make the redirect fail on // new_url after calling them, and return false to make the redirect fail on
// mismatch. // mismatch.
WebScopedVirtualTimePauser unused_virtual_time_pauser; WebScopedVirtualTimePauser unused_virtual_time_pauser;
// TODO(yoichio): Have PrepareRequest use ResourceRequestHead.
Context().PrepareRequest(*new_request, resource_->Options().initiator_info, Context().PrepareRequest(*new_request, resource_->Options().initiator_info,
unused_virtual_time_pauser, unused_virtual_time_pauser, resource_->GetType());
resource_->GetType()); DCHECK(!new_request->HttpBody());
if (auto* observer = fetcher_->GetResourceLoadObserver()) { if (auto* observer = fetcher_->GetResourceLoadObserver()) {
observer->WillSendRequest(resource_->InspectorId(), *new_request, observer->WillSendRequest(resource_->InspectorId(), *new_request,
redirect_response_to_pass, resource_->GetType(), redirect_response_to_pass, resource_->GetType(),
...@@ -957,8 +958,10 @@ void ResourceLoader::DidReceiveResponseInternal( ...@@ -957,8 +958,10 @@ void ResourceLoader::DidReceiveResponseInternal(
kEnableCorsHandlingByResourceFetcher && kEnableCorsHandlingByResourceFetcher &&
request_mode == network::mojom::RequestMode::kCors && request_mode == network::mojom::RequestMode::kCors &&
response.WasFallbackRequiredByServiceWorker()) { response.WasFallbackRequiredByServiceWorker()) {
DCHECK(resource_->RedirectChain().IsEmpty());
ResourceRequest last_request; ResourceRequest last_request;
last_request.CopyFrom(resource_->LastResourceRequest()); last_request.CopyHeadFrom(resource_->GetResourceRequest());
last_request.SetHttpBody(resource_->GetResourceRequest().HttpBody());
DCHECK(!last_request.GetSkipServiceWorker()); DCHECK(!last_request.GetSkipServiceWorker());
// This code handles the case when a controlling service worker doesn't // This code handles the case when a controlling service worker doesn't
// handle a cross origin request. // handle a cross origin request.
......
...@@ -117,7 +117,7 @@ void ResourceRequest::CopyHeadFrom(const ResourceRequestHead& src) { ...@@ -117,7 +117,7 @@ void ResourceRequest::CopyHeadFrom(const ResourceRequestHead& src) {
this->ResourceRequestHead::operator=(src); this->ResourceRequestHead::operator=(src);
} }
std::unique_ptr<ResourceRequest> ResourceRequest::CreateRedirectRequest( std::unique_ptr<ResourceRequest> ResourceRequestHead::CreateRedirectRequest(
const KURL& new_url, const KURL& new_url,
const AtomicString& new_method, const AtomicString& new_method,
const net::SiteForCookies& new_site_for_cookies, const net::SiteForCookies& new_site_for_cookies,
......
...@@ -67,13 +67,24 @@ class PLATFORM_EXPORT ResourceRequestHead { ...@@ -67,13 +67,24 @@ class PLATFORM_EXPORT ResourceRequestHead {
ResourceRequestHead(); ResourceRequestHead();
explicit ResourceRequestHead(const KURL&); explicit ResourceRequestHead(const KURL&);
explicit ResourceRequestHead(const ResourceRequestHead&); ResourceRequestHead(const ResourceRequestHead&);
ResourceRequestHead& operator=(const ResourceRequestHead&); ResourceRequestHead& operator=(const ResourceRequestHead&);
explicit ResourceRequestHead(ResourceRequestHead&&); ResourceRequestHead(ResourceRequestHead&&);
ResourceRequestHead& operator=(ResourceRequestHead&&); ResourceRequestHead& operator=(ResourceRequestHead&&);
~ResourceRequestHead(); ~ResourceRequestHead();
// Constructs a new ResourceRequest for a redirect from this instance.
// Since body for a redirect request is kept and handled in the network
// service, the returned instance here in blink side doesn't contain body.
std::unique_ptr<ResourceRequest> CreateRedirectRequest(
const KURL& new_url,
const AtomicString& new_method,
const net::SiteForCookies& new_site_for_cookies,
const String& new_referrer,
network::mojom::ReferrerPolicy new_referrer_policy,
bool skip_service_worker) const;
bool IsNull() const; bool IsNull() const;
const KURL& Url() const; const KURL& Url() const;
...@@ -559,15 +570,6 @@ class PLATFORM_EXPORT ResourceRequest final : public ResourceRequestHead { ...@@ -559,15 +570,6 @@ class PLATFORM_EXPORT ResourceRequest final : public ResourceRequestHead {
void CopyFrom(const ResourceRequest&); void CopyFrom(const ResourceRequest&);
void CopyHeadFrom(const ResourceRequestHead&); void CopyHeadFrom(const ResourceRequestHead&);
// Constructs a new ResourceRequest for a redirect from this instance.
std::unique_ptr<ResourceRequest> CreateRedirectRequest(
const KURL& new_url,
const AtomicString& new_method,
const net::SiteForCookies& new_site_for_cookies,
const String& new_referrer,
network::mojom::ReferrerPolicy new_referrer_policy,
bool skip_service_worker) const;
EncodedFormData* HttpBody() const; EncodedFormData* HttpBody() const;
void SetHttpBody(scoped_refptr<EncodedFormData>); void SetHttpBody(scoped_refptr<EncodedFormData>);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment