Commit 6e8fd3ec authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot

Revert "Make DocumentThreadableLoader a ResourceOwner"

This reverts commit ae15c135.

Reason for revert:
https://bugs.chromium.org/p/chromium/issues/detail?id=791291
https://bugs.chromium.org/p/chromium/issues/detail?id=791316
https://bugs.chromium.org/p/chromium/issues/detail?id=791288
https://bugs.chromium.org/p/chromium/issues/detail?id=791348
https://bugs.chromium.org/p/chromium/issues/detail?id=791347
https://bugs.chromium.org/p/chromium/issues/detail?id=791450
https://bugs.chromium.org/p/chromium/issues/detail?id=791300
https://bugs.chromium.org/p/chromium/issues/detail?id=791522

Original change's description:
> Make DocumentThreadableLoader a ResourceOwner
> 
> It had a custom implementation to support use of RawResourceClientStateChecker.
> Separate these.
> 
> Bug: 790778, 640291
> Change-Id: Id8a29a5ad916429969b47a0258675437de284b55
> Reviewed-on: https://chromium-review.googlesource.com/802094
> Commit-Queue: Nate Chapin <japhet@chromium.org>
> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521177}

TBR=hiroshige@chromium.org,japhet@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 790778, 640291
Change-Id: I15938754b57b8142f8d7e9c94f8aff9bdd8329ed
Reviewed-on: https://chromium-review.googlesource.com/806794Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521448}
parent 2e77c18e
...@@ -53,7 +53,6 @@ ...@@ -53,7 +53,6 @@
#include "platform/loader/fetch/FetchUtils.h" #include "platform/loader/fetch/FetchUtils.h"
#include "platform/loader/fetch/Resource.h" #include "platform/loader/fetch/Resource.h"
#include "platform/loader/fetch/ResourceFetcher.h" #include "platform/loader/fetch/ResourceFetcher.h"
#include "platform/loader/fetch/ResourceLoader.h"
#include "platform/loader/fetch/ResourceLoaderOptions.h" #include "platform/loader/fetch/ResourceLoaderOptions.h"
#include "platform/loader/fetch/ResourceRequest.h" #include "platform/loader/fetch/ResourceRequest.h"
#include "platform/weborigin/SchemeRegistry.h" #include "platform/weborigin/SchemeRegistry.h"
...@@ -537,7 +536,7 @@ void DocumentThreadableLoader::MakeCrossOriginAccessRequestBlinkCORS( ...@@ -537,7 +536,7 @@ void DocumentThreadableLoader::MakeCrossOriginAccessRequestBlinkCORS(
DocumentThreadableLoader::~DocumentThreadableLoader() { DocumentThreadableLoader::~DocumentThreadableLoader() {
CHECK(!client_); CHECK(!client_);
DCHECK(!GetResource()); DCHECK(!resource_);
} }
void DocumentThreadableLoader::OverrideTimeout( void DocumentThreadableLoader::OverrideTimeout(
...@@ -586,16 +585,14 @@ void DocumentThreadableLoader::Detach() { ...@@ -586,16 +585,14 @@ void DocumentThreadableLoader::Detach() {
} }
void DocumentThreadableLoader::SetDefersLoading(bool value) { void DocumentThreadableLoader::SetDefersLoading(bool value) {
if (GetResource() && GetResource()->Loader()) if (GetResource())
GetResource()->Loader()->SetDefersLoading(value); GetResource()->SetDefersLoading(value);
} }
void DocumentThreadableLoader::Clear() { void DocumentThreadableLoader::Clear() {
client_ = nullptr; client_ = nullptr;
timeout_timer_.Stop(); timeout_timer_.Stop();
request_started_seconds_ = 0.0; request_started_seconds_ = 0.0;
if (GetResource())
checker_.WillRemoveClient();
ClearResource(); ClearResource();
} }
...@@ -737,8 +734,6 @@ bool DocumentThreadableLoader::RedirectReceivedBlinkCORS( ...@@ -737,8 +734,6 @@ bool DocumentThreadableLoader::RedirectReceivedBlinkCORS(
// FIXME: consider combining this with CORS redirect handling performed by // FIXME: consider combining this with CORS redirect handling performed by
// CrossOriginAccessControl::handleRedirect(). // CrossOriginAccessControl::handleRedirect().
if (GetResource())
checker_.WillRemoveClient();
ClearResource(); ClearResource();
// If // If
...@@ -1106,8 +1101,6 @@ void DocumentThreadableLoader::DidTimeout(TimerBase* timer) { ...@@ -1106,8 +1101,6 @@ void DocumentThreadableLoader::DidTimeout(TimerBase* timer) {
} }
void DocumentThreadableLoader::LoadFallbackRequestForServiceWorker() { void DocumentThreadableLoader::LoadFallbackRequestForServiceWorker() {
if (GetResource())
checker_.WillRemoveClient();
ClearResource(); ClearResource();
ResourceRequest fallback_request(fallback_request_for_service_worker_); ResourceRequest fallback_request(fallback_request_for_service_worker_);
fallback_request_for_service_worker_ = ResourceRequest(); fallback_request_for_service_worker_ = ResourceRequest();
...@@ -1123,8 +1116,6 @@ void DocumentThreadableLoader::LoadActualRequest() { ...@@ -1123,8 +1116,6 @@ void DocumentThreadableLoader::LoadActualRequest() {
actual_request_ = ResourceRequest(); actual_request_ = ResourceRequest();
actual_options_ = ResourceLoaderOptions(); actual_options_ = ResourceLoaderOptions();
if (GetResource())
checker_.WillRemoveClient();
ClearResource(); ClearResource();
PrepareCrossOriginRequest(actual_request); PrepareCrossOriginRequest(actual_request);
...@@ -1209,8 +1200,6 @@ void DocumentThreadableLoader::LoadRequestAsync( ...@@ -1209,8 +1200,6 @@ void DocumentThreadableLoader::LoadRequestAsync(
} else { } else {
SetResource(RawResource::Fetch(new_params, fetcher)); SetResource(RawResource::Fetch(new_params, fetcher));
} }
if (GetResource())
checker_.WillAddClient();
if (!GetResource()) { if (!GetResource()) {
probe::documentThreadableLoaderFailedToStartLoadingForClient( probe::documentThreadableLoaderFailedToStartLoadingForClient(
...@@ -1376,6 +1365,7 @@ ExecutionContext* DocumentThreadableLoader::GetExecutionContext() const { ...@@ -1376,6 +1365,7 @@ ExecutionContext* DocumentThreadableLoader::GetExecutionContext() const {
} }
void DocumentThreadableLoader::Trace(blink::Visitor* visitor) { void DocumentThreadableLoader::Trace(blink::Visitor* visitor) {
visitor->Trace(resource_);
visitor->Trace(loading_context_); visitor->Trace(loading_context_);
ThreadableLoader::Trace(visitor); ThreadableLoader::Trace(visitor);
RawResourceClient::Trace(visitor); RawResourceClient::Trace(visitor);
......
...@@ -57,9 +57,8 @@ class ThreadableLoadingContext; ...@@ -57,9 +57,8 @@ class ThreadableLoadingContext;
// TODO(horo): We are using this class not only in documents, but also in // TODO(horo): We are using this class not only in documents, but also in
// workers. We should change the name to ThreadableLoaderImpl. // workers. We should change the name to ThreadableLoaderImpl.
class CORE_EXPORT DocumentThreadableLoader final class CORE_EXPORT DocumentThreadableLoader final : public ThreadableLoader,
: public ThreadableLoader, private RawResourceClient {
private ResourceOwner<RawResource> {
USING_GARBAGE_COLLECTED_MIXIN(DocumentThreadableLoader); USING_GARBAGE_COLLECTED_MIXIN(DocumentThreadableLoader);
public: public:
...@@ -204,6 +203,32 @@ class CORE_EXPORT DocumentThreadableLoader final ...@@ -204,6 +203,32 @@ class CORE_EXPORT DocumentThreadableLoader final
void LoadRequest(ResourceRequest&, ResourceLoaderOptions); void LoadRequest(ResourceRequest&, ResourceLoaderOptions);
bool IsAllowedRedirect(network::mojom::FetchRequestMode, const KURL&) const; bool IsAllowedRedirect(network::mojom::FetchRequestMode, const KURL&) const;
// TODO(hiroshige): After crbug.com/633696 is fixed,
// - Remove RawResourceClientStateChecker logic,
// - Make DocumentThreadableLoader to be a ResourceOwner and remove this
// re-implementation of ResourceOwner, and
// - Consider re-applying RawResourceClientStateChecker in a more
// general fashion (crbug.com/640291).
RawResource* GetResource() const { return resource_.Get(); }
void ClearResource() { SetResource(nullptr); }
void SetResource(RawResource* new_resource) {
if (new_resource == resource_)
return;
if (RawResource* old_resource = resource_.Release()) {
checker_.WillRemoveClient();
old_resource->RemoveClient(this);
}
if (new_resource) {
resource_ = new_resource;
checker_.WillAddClient();
resource_->AddClient(this);
}
}
Member<RawResource> resource_;
// End of ResourceOwner re-implementation, see above.
SecurityOrigin* GetSecurityOrigin() const; SecurityOrigin* GetSecurityOrigin() const;
// Returns null if the loader is not associated with Document. // Returns null if the loader is not associated with Document.
......
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include "platform/loader/fetch/MemoryCache.h" #include "platform/loader/fetch/MemoryCache.h"
#include "platform/loader/fetch/ResourceClientWalker.h" #include "platform/loader/fetch/ResourceClientWalker.h"
#include "platform/loader/fetch/ResourceFetcher.h" #include "platform/loader/fetch/ResourceFetcher.h"
#include "platform/loader/fetch/ResourceLoader.h"
#include "platform/network/http_names.h" #include "platform/network/http_names.h"
#include "platform/scheduler/child/web_scheduler.h" #include "platform/scheduler/child/web_scheduler.h"
#include "public/platform/Platform.h" #include "public/platform/Platform.h"
...@@ -295,6 +296,11 @@ void RawResource::NotifyFinished() { ...@@ -295,6 +296,11 @@ void RawResource::NotifyFinished() {
Resource::NotifyFinished(); Resource::NotifyFinished();
} }
void RawResource::SetDefersLoading(bool defers) {
if (Loader())
Loader()->SetDefersLoading(defers);
}
static bool ShouldIgnoreHeaderForCacheReuse(AtomicString header_name) { static bool ShouldIgnoreHeaderForCacheReuse(AtomicString header_name) {
// FIXME: This list of headers that don't affect cache policy almost certainly // FIXME: This list of headers that don't affect cache policy almost certainly
// isn't complete. // isn't complete.
......
...@@ -66,6 +66,12 @@ class PLATFORM_EXPORT RawResource final : public Resource { ...@@ -66,6 +66,12 @@ class PLATFORM_EXPORT RawResource final : public Resource {
return CreateForTest(KURL(url), type); return CreateForTest(KURL(url), type);
} }
// FIXME: AssociatedURLLoader shouldn't be a DocumentThreadableLoader and
// therefore shouldn't use RawResource. However, it is, and it needs to be
// able to defer loading. This can be fixed by splitting CORS preflighting out
// of DocumentThreadableLoader.
void SetDefersLoading(bool);
// Resource implementation // Resource implementation
bool CanReuse(const FetchParameters&) const override; bool CanReuse(const FetchParameters&) const override;
bool WillFollowRedirect(const ResourceRequest&, bool WillFollowRedirect(const ResourceRequest&,
......
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