From 65a9812e8f82bd73e8da2c1865db8d1e81b5d9e7 Mon Sep 17 00:00:00 2001 From: japhet Date: Mon, 16 Nov 2015 10:12:09 -0800 Subject: [PATCH] Revert of Simplify starting a navigation (patchset #6 id:100001 of https://codereview.chromium.org/1418003006/ ) Reason for revert: This caused several regressions: https://code.google.com/p/chromium/issues/detail?id=555914 https://code.google.com/p/chromium/issues/detail?id=553078 https://code.google.com/p/chromium/issues/detail?id=553493 Original issue's description: > Simplify starting a navigation > > Remove a whole bunch of accumulated technical debt in > DocumentLoader and core/fetch/ related to starting a navigation > and handling redirects. > > BUG= > > Committed: https://crrev.com/83a52878976eaffc8753993a7689c9c178664fba > Cr-Commit-Position: refs/heads/master@{#358420} TBR=jochen@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/1444413002 Cr-Commit-Position: refs/heads/master@{#359864} --- chrome/browser/ssl/ssl_browser_tests.cc | 2 - .../307-after-303-after-post-expected.txt | 2 +- .../loading/redirect-methods-expected.txt | 8 +- .../WebKit/Source/core/fetch/FetchContext.cpp | 2 +- .../WebKit/Source/core/fetch/FetchContext.h | 2 +- .../WebKit/Source/core/fetch/RawResource.cpp | 8 ++ .../WebKit/Source/core/fetch/RawResource.h | 1 + .../WebKit/Source/core/fetch/Resource.cpp | 32 +++-- .../WebKit/Source/core/fetch/Resource.h | 3 + .../Source/core/fetch/ResourceFetcher.cpp | 4 +- .../Source/core/fetch/ResourceLoader.cpp | 15 ++- .../WebKit/Source/core/fetch/ResourceLoader.h | 7 +- .../Source/core/loader/DocumentLoader.cpp | 113 ++++++++++++------ .../Source/core/loader/DocumentLoader.h | 3 + .../Source/core/loader/FrameFetchContext.cpp | 9 +- .../Source/core/loader/FrameFetchContext.h | 2 +- .../core/loader/FrameFetchContextTest.cpp | 6 +- .../WebKit/Source/core/loader/FrameLoader.cpp | 4 - 18 files changed, 144 insertions(+), 79 deletions(-) diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc index 99999bf4b53b0..1d7e7c89fbc47 100644 --- a/chrome/browser/ssl/ssl_browser_tests.cc +++ b/chrome/browser/ssl/ssl_browser_tests.cc @@ -2573,8 +2573,6 @@ IN_PROC_BROWSER_TEST_F(CommonNameMismatchBrowserTest, CheckSecurityState(contents, CertError::NONE, content::SECURITY_STYLE_AUTHENTICATED, AuthState::NONE); replacements.SetHostStr("mail.example.com"); - // blink strips the ref from requests. - replacements.ClearRef(); GURL https_server_new_url = https_server_url.ReplaceComponents(replacements); // Verify that the current URL is the suggested URL. EXPECT_EQ(https_server_new_url.spec(), diff --git a/third_party/WebKit/LayoutTests/http/tests/loading/307-after-303-after-post-expected.txt b/third_party/WebKit/LayoutTests/http/tests/loading/307-after-303-after-post-expected.txt index 60b89d8d90d58..f386fc02b0fb0 100644 --- a/third_party/WebKit/LayoutTests/http/tests/loading/307-after-303-after-post-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/loading/307-after-303-after-post-expected.txt @@ -5,10 +5,10 @@ main frame - didHandleOnloadEventsForFrame main frame - didFinishLoadForFrame main frame - didStartProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/post-to-303-target.php - willSendRequest redirectResponse (null) +main frame - didReceiveServerRedirectForProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/post-to-303-target.php - willSendRequest redirectResponse main frame - didReceiveServerRedirectForProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/post-to-303-target.php - willSendRequest redirectResponse -main frame - didReceiveServerRedirectForProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/post-to-303-target.php - didReceiveResponse main frame - didCommitLoadForFrame http://127.0.0.1:8000/loading/resources/post-to-303-target.php - didFinishLoading diff --git a/third_party/WebKit/LayoutTests/http/tests/loading/redirect-methods-expected.txt b/third_party/WebKit/LayoutTests/http/tests/loading/redirect-methods-expected.txt index 28026fdf05dc3..f8610151ae686 100644 --- a/third_party/WebKit/LayoutTests/http/tests/loading/redirect-methods-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/loading/redirect-methods-expected.txt @@ -18,8 +18,8 @@ frame "0" - didHandleOnloadEventsForFrame frame "0" - didFinishLoadForFrame frame "0" - didStartProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse (null) -http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse frame "0" - didReceiveServerRedirectForProvisionalLoadForFrame +http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didReceiveResponse frame "0" - didCommitLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didFinishLoading @@ -41,8 +41,8 @@ frame "1" - didHandleOnloadEventsForFrame frame "1" - didFinishLoadForFrame frame "1" - didStartProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse (null) -http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse frame "1" - didReceiveServerRedirectForProvisionalLoadForFrame +http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didReceiveResponse frame "1" - didCommitLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didFinishLoading @@ -64,8 +64,8 @@ frame "2" - didHandleOnloadEventsForFrame frame "2" - didFinishLoadForFrame frame "2" - didStartProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse (null) -http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse frame "2" - didReceiveServerRedirectForProvisionalLoadForFrame +http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didReceiveResponse frame "2" - didCommitLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didFinishLoading @@ -87,8 +87,8 @@ frame "3" - didHandleOnloadEventsForFrame frame "3" - didFinishLoadForFrame frame "3" - didStartProvisionalLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse (null) -http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse frame "3" - didReceiveServerRedirectForProvisionalLoadForFrame +http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - willSendRequest redirectResponse http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didReceiveResponse frame "3" - didCommitLoadForFrame http://127.0.0.1:8000/loading/resources/redirect-methods-result.php - didFinishLoading diff --git a/third_party/WebKit/Source/core/fetch/FetchContext.cpp b/third_party/WebKit/Source/core/fetch/FetchContext.cpp index f7917ff8046bf..34ad58aa56051 100644 --- a/third_party/WebKit/Source/core/fetch/FetchContext.cpp +++ b/third_party/WebKit/Source/core/fetch/FetchContext.cpp @@ -90,7 +90,7 @@ void FetchContext::dispatchDidFail(unsigned long, const ResourceError&, bool) { } -void FetchContext::willStartLoadingResource(ResourceRequest&, FetchResourceType) +void FetchContext::willStartLoadingResource(ResourceRequest&) { } diff --git a/third_party/WebKit/Source/core/fetch/FetchContext.h b/third_party/WebKit/Source/core/fetch/FetchContext.h index 0dc486418a9e0..22869f183ccb4 100644 --- a/third_party/WebKit/Source/core/fetch/FetchContext.h +++ b/third_party/WebKit/Source/core/fetch/FetchContext.h @@ -83,7 +83,7 @@ class CORE_EXPORT FetchContext : public GarbageCollectedFinalized virtual void dispatchDidFail(unsigned long identifier, const ResourceError&, bool isInternalRequest); virtual bool shouldLoadNewResource(Resource::Type) const { return false; } - virtual void willStartLoadingResource(ResourceRequest&, FetchResourceType); + virtual void willStartLoadingResource(ResourceRequest&); virtual void didLoadResource(); virtual void addResourceTiming(const ResourceTimingInfo&); diff --git a/third_party/WebKit/Source/core/fetch/RawResource.cpp b/third_party/WebKit/Source/core/fetch/RawResource.cpp index f35f054380014..df654a13a01b7 100644 --- a/third_party/WebKit/Source/core/fetch/RawResource.cpp +++ b/third_party/WebKit/Source/core/fetch/RawResource.cpp @@ -134,6 +134,14 @@ void RawResource::willFollowRedirect(ResourceRequest& newRequest, const Resource Resource::willFollowRedirect(newRequest, redirectResponse); } +void RawResource::updateRequest(const ResourceRequest& request) +{ + ResourcePtr protect(this); + ResourceClientWalker w(m_clients); + while (RawResourceClient* c = w.next()) + c->updateRequest(this, request); +} + void RawResource::responseReceived(const ResourceResponse& response, PassOwnPtr handle) { InternalResourcePtr protect(this); diff --git a/third_party/WebKit/Source/core/fetch/RawResource.h b/third_party/WebKit/Source/core/fetch/RawResource.h index ad95b133ad7bb..bb3546dbb0535 100644 --- a/third_party/WebKit/Source/core/fetch/RawResource.h +++ b/third_party/WebKit/Source/core/fetch/RawResource.h @@ -74,6 +74,7 @@ class CORE_EXPORT RawResource final : public Resource { bool shouldIgnoreHTTPStatusCodeErrors() const override { return true; } void willFollowRedirect(ResourceRequest&, const ResourceResponse&) override; + void updateRequest(const ResourceRequest&) override; void responseReceived(const ResourceResponse&, PassOwnPtr) override; void setSerializedCachedMetadata(const char*, size_t) override; void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override; diff --git a/third_party/WebKit/Source/core/fetch/Resource.cpp b/third_party/WebKit/Source/core/fetch/Resource.cpp index f671d27431728..ffca9956ff1d7 100644 --- a/third_party/WebKit/Source/core/fetch/Resource.cpp +++ b/third_party/WebKit/Source/core/fetch/Resource.cpp @@ -176,8 +176,13 @@ Resource::Resource(const ResourceRequest& request, Type type) if (m_resourceRequest.url().protocolIsInHTTPFamily()) m_cacheHandler = CacheHandler::create(this); - if (!accept().isEmpty()) - m_resourceRequest.setHTTPAccept(accept()); + if (!m_resourceRequest.url().hasFragmentIdentifier()) + return; + KURL urlForCache = MemoryCache::removeFragmentIdentifierIfNeeded(m_resourceRequest.url()); + if (urlForCache.hasFragmentIdentifier()) + return; + m_fragmentIdentifierForRequest = m_resourceRequest.url().fragmentIdentifier(); + m_resourceRequest.setURL(urlForCache); } Resource::~Resource() @@ -209,23 +214,28 @@ void Resource::load(ResourceFetcher* fetcher, const ResourceLoaderOptions& optio { m_options = options; m_loading = true; - m_status = Pending; + ResourceRequest request(m_revalidatingRequest.isNull() ? m_resourceRequest : m_revalidatingRequest); + if (!accept().isEmpty()) + request.setHTTPAccept(accept()); + + // FIXME: It's unfortunate that the cache layer and below get to know anything about fragment identifiers. + // We should look into removing the expectation of that knowledge from the platform network stacks. + if (!m_fragmentIdentifierForRequest.isNull()) { + KURL url = request.url(); + url.setFragmentIdentifier(m_fragmentIdentifierForRequest); + request.setURL(url); + m_fragmentIdentifierForRequest = String(); + } + m_status = Pending; if (m_loader) { ASSERT(m_revalidatingRequest.isNull()); RELEASE_ASSERT(m_options.synchronousPolicy == RequestSynchronously); m_loader->changeToSynchronous(); return; } - - ResourceRequest& request(m_revalidatingRequest.isNull() ? m_resourceRequest : m_revalidatingRequest); - ResourceRequest requestCopy(m_revalidatingRequest.isNull() ? m_resourceRequest : m_revalidatingRequest); - m_loader = ResourceLoader::create(fetcher, this, requestCopy, options); + m_loader = ResourceLoader::create(fetcher, this, request, options); m_loader->start(); - - // Headers might have been modified during load start. Copy them over so long as the url didn't change. - if (request.url() == requestCopy.url()) - request = requestCopy; } void Resource::checkNotify() diff --git a/third_party/WebKit/Source/core/fetch/Resource.h b/third_party/WebKit/Source/core/fetch/Resource.h index 7e63e3d97a83a..9377f52c6490a 100644 --- a/third_party/WebKit/Source/core/fetch/Resource.h +++ b/third_party/WebKit/Source/core/fetch/Resource.h @@ -195,6 +195,7 @@ class CORE_EXPORT Resource : public NoBaseWillBeGarbageCollectedFinalized); void setResponse(const ResourceResponse& response) { m_response = response; } const ResourceResponse& response() const { return m_response; } @@ -379,6 +380,8 @@ class CORE_EXPORT Resource : public NoBaseWillBeGarbageCollectedFinalized m_cachedMetadata; OwnPtrWillBeMember m_cacheHandler; diff --git a/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp index 558b0f738b814..dc7a14c9c6ce3 100644 --- a/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp +++ b/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp @@ -489,8 +489,6 @@ void ResourceFetcher::initializeResourceRequest(ResourceRequest& request, Resour if (type == Resource::LinkPrefetch || type == Resource::LinkSubresource) request.setHTTPHeaderField("Purpose", "prefetch"); - request.setURL(MemoryCache::removeFragmentIdentifierIfNeeded(request.url())); - context().addAdditionalRequestHeaders(request, (type == Resource::MainResource) ? FetchMainResource : FetchSubresource); } @@ -997,7 +995,7 @@ void ResourceFetcher::willTerminateResourceLoader(ResourceLoader* loader) void ResourceFetcher::willStartLoadingResource(Resource* resource, ResourceRequest& request) { - context().willStartLoadingResource(request, resource->type() == Resource::MainResource ? FetchMainResource : FetchSubresource); + context().willStartLoadingResource(request); storeResourceTimingInitiatorInformation(resource); TRACE_EVENT_ASYNC_BEGIN2("blink.net", "Resource", resource, "url", resource->url().string().ascii(), "priority", resource->resourceRequest().priority()); } diff --git a/third_party/WebKit/Source/core/fetch/ResourceLoader.cpp b/third_party/WebKit/Source/core/fetch/ResourceLoader.cpp index a49167eb212d8..fdcbe349d0def 100644 --- a/third_party/WebKit/Source/core/fetch/ResourceLoader.cpp +++ b/third_party/WebKit/Source/core/fetch/ResourceLoader.cpp @@ -53,7 +53,7 @@ namespace blink { -ResourceLoader* ResourceLoader::create(ResourceFetcher* fetcher, Resource* resource, ResourceRequest& request, const ResourceLoaderOptions& options) +ResourceLoader* ResourceLoader::create(ResourceFetcher* fetcher, Resource* resource, const ResourceRequest& request, const ResourceLoaderOptions& options) { ResourceLoader* loader = new ResourceLoader(fetcher, resource, options); loader->init(request); @@ -105,13 +105,15 @@ void ResourceLoader::releaseResources() m_fetcher.clear(); } -void ResourceLoader::init(ResourceRequest& request) +void ResourceLoader::init(const ResourceRequest& passedRequest) { ASSERT(m_state != Terminated); + ResourceRequest request(passedRequest); m_fetcher->willSendRequest(m_resource->identifier(), request, ResourceResponse(), m_options.initiatorInfo); ASSERT(m_state != Terminated); ASSERT(!request.isNull()); - m_request = applyOptions(request); + m_originalRequest = m_request = applyOptions(request); + m_resource->updateRequest(request); ASSERT(m_state != Terminated); m_fetcher->didInitializeResourceLoader(this); } @@ -276,10 +278,15 @@ void ResourceLoader::willFollowRedirect(WebURLLoader*, WebURLRequest& passedNewR applyOptions(newRequest); // canAccessRedirect() can modify m_options so we should re-apply it. m_fetcher->redirectReceived(m_resource, redirectResponse); - m_fetcher->willSendRequest(m_resource->identifier(), newRequest, redirectResponse, m_options.initiatorInfo); + ASSERT(m_state != Terminated); m_resource->willFollowRedirect(newRequest, redirectResponse); if (newRequest.isNull() || m_state == Terminated) return; + + m_fetcher->willSendRequest(m_resource->identifier(), newRequest, redirectResponse, m_options.initiatorInfo); + ASSERT(m_state != Terminated); + ASSERT(!newRequest.isNull()); + m_resource->updateRequest(newRequest); m_request = newRequest; } diff --git a/third_party/WebKit/Source/core/fetch/ResourceLoader.h b/third_party/WebKit/Source/core/fetch/ResourceLoader.h index e0b42dbc20e56..5077de7a0e1b5 100644 --- a/third_party/WebKit/Source/core/fetch/ResourceLoader.h +++ b/third_party/WebKit/Source/core/fetch/ResourceLoader.h @@ -47,7 +47,7 @@ class ThreadedDataReceiver; class CORE_EXPORT ResourceLoader final : public GarbageCollectedFinalized, protected WebURLLoaderClient { public: - static ResourceLoader* create(ResourceFetcher*, Resource*, ResourceRequest&, const ResourceLoaderOptions&); + static ResourceLoader* create(ResourceFetcher*, Resource*, const ResourceRequest&, const ResourceLoaderOptions&); ~ResourceLoader() override; DECLARE_TRACE(); @@ -59,6 +59,7 @@ class CORE_EXPORT ResourceLoader final : public GarbageCollectedFinalized m_fetcher; ResourceRequest m_request; + ResourceRequest m_originalRequest; // Before redirects. bool m_notifiedLoadComplete; diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp index 2c84b0d65a9e0..01b2985374a57 100644 --- a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp +++ b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp @@ -95,11 +95,10 @@ DocumentLoader::DocumentLoader(LocalFrame* frame, const ResourceRequest& req, co , m_documentLoadTiming(*this) , m_timeOfLastDataReceived(0.0) , m_applicationCacheHost(ApplicationCacheHost::create(this)) - , m_state(Provisional) + , m_state(NotStarted) , m_inDataReceived(false) , m_dataBuffer(SharedBuffer::create()) { - timing().markNavigationStart(); } FrameLoader* DocumentLoader::frameLoader() const @@ -251,7 +250,7 @@ bool DocumentLoader::isLoading() const if (document() && document()->hasActiveParser()) return true; - return m_state < MainResourceDone || m_fetcher->isFetching(); + return (m_state > NotStarted && m_state < MainResourceDone) || m_fetcher->isFetching(); } void DocumentLoader::notifyFinished(Resource* resource) @@ -313,38 +312,60 @@ bool DocumentLoader::isRedirectAfterPost(const ResourceRequest& newRequest, cons void DocumentLoader::redirectReceived(Resource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse) { ASSERT_UNUSED(resource, resource == m_mainResource); + willSendRequest(request, redirectResponse); +} + +void DocumentLoader::updateRequest(Resource* resource, const ResourceRequest& request) +{ + ASSERT_UNUSED(resource, resource == m_mainResource); + m_request = request; +} + +static bool isFormSubmission(NavigationType type) +{ + return type == NavigationTypeFormSubmitted || type == NavigationTypeFormResubmitted; +} + +void DocumentLoader::willSendRequest(ResourceRequest& newRequest, const ResourceResponse& redirectResponse) +{ + // Note that there are no asserts here as there are for the other callbacks. This is due to the + // fact that this "callback" is sent when starting every load, and the state of callback + // deferrals plays less of a part in this function in preventing the bad behavior deferring + // callbacks is meant to prevent. + ASSERT(!newRequest.isNull()); + if (isFormSubmission(m_navigationType) && !m_frame->document()->contentSecurityPolicy()->allowFormAction(newRequest.url())) { + cancelMainResourceLoad(ResourceError::cancelledError(newRequest.url())); + return; + } + + ASSERT(timing().fetchStart()); + if (!redirectResponse.isNull()) { + // If the redirecting url is not allowed to display content from the target origin, + // then block the redirect. + RefPtr redirectingOrigin = SecurityOrigin::create(redirectResponse.url()); + if (!redirectingOrigin->canDisplay(newRequest.url())) { + FrameLoader::reportLocalLoadFailed(m_frame, newRequest.url().string()); + cancelMainResourceLoad(ResourceError::cancelledError(newRequest.url())); + return; + } + timing().addRedirect(redirectResponse.url(), newRequest.url()); + } // If we're fielding a redirect in response to a POST, force a load from origin, since // this is a common site technique to return to a page viewing some data that the POST // just modified. - if (request.cachePolicy() == UseProtocolCachePolicy && isRedirectAfterPost(request, redirectResponse)) - request.setCachePolicy(ReloadBypassingCache); + if (newRequest.cachePolicy() == UseProtocolCachePolicy && isRedirectAfterPost(newRequest, redirectResponse)) + newRequest.setCachePolicy(ReloadBypassingCache); - m_request = request; - - // Add the fragment to the new request url. Note that this is only done on m_request, - // not on the request that will be passed back out of blink. The network stack doesn't care - // about the fragment, and doesn't expect the URL to change unless it is invalidated. - KURL url = request.url(); - if (m_originalRequest.url().hasFragmentIdentifier()) { - url.setFragmentIdentifier(m_originalRequest.url().fragmentIdentifier()); - m_request.setURL(url); - } + m_request = newRequest; - // If the redirecting url is not allowed to display content from the target origin, - // then block the redirect. - RefPtr redirectingOrigin = SecurityOrigin::create(redirectResponse.url()); - if (!redirectingOrigin->canDisplay(url)) { - FrameLoader::reportLocalLoadFailed(m_frame, url.string()); - cancelMainResourceLoad(ResourceError::cancelledError(url)); + if (redirectResponse.isNull()) return; - } - timing().addRedirect(redirectResponse.url(), url); - appendRedirect(url); + appendRedirect(newRequest.url()); frameLoader()->receivedMainResourceRedirect(m_request.url()); - if (!frameLoader()->shouldContinueForNavigationPolicy(request, SubstituteData(), this, CheckContentSecurityPolicy, m_navigationType, NavigationPolicyCurrentTab, replacesCurrentHistoryItem())) - cancelMainResourceLoad(ResourceError::cancelledError(url)); + if (!frameLoader()->shouldContinueForNavigationPolicy(newRequest, SubstituteData(), this, CheckContentSecurityPolicy, m_navigationType, NavigationPolicyCurrentTab, replacesCurrentHistoryItem())) + cancelMainResourceLoad(ResourceError::cancelledError(m_request.url())); } static bool canShowMIMEType(const String& mimeType, Page* page) @@ -701,35 +722,53 @@ bool DocumentLoader::maybeLoadEmpty() void DocumentLoader::startLoadingMainResource() { RefPtrWillBeRawPtr protect(this); - ASSERT(m_state == Provisional); + m_mainDocumentError = ResourceError(); + timing().markNavigationStart(); + ASSERT(!m_mainResource); + ASSERT(m_state == NotStarted); + m_state = Provisional; if (maybeLoadEmpty()) return; + ASSERT(timing().navigationStart()); ASSERT(!timing().fetchStart()); timing().markFetchStart(); + willSendRequest(m_request, ResourceResponse()); + + // willSendRequest() may lead to our LocalFrame being detached or cancelling the load via nulling the ResourceRequest. + if (!m_frame || m_request.isNull()) + return; + + m_applicationCacheHost->willStartLoadingMainResource(m_request); prepareSubframeArchiveLoadIfNeeded(); + ResourceRequest request(m_request); DEFINE_STATIC_LOCAL(ResourceLoaderOptions, mainResourceLoadOptions, (DoNotBufferData, AllowStoredCredentials, ClientRequestedCredentials, CheckContentSecurityPolicy, DocumentContext)); - FetchRequest fetchRequest(m_request, FetchInitiatorTypeNames::document, mainResourceLoadOptions); - - m_mainResource = RawResource::fetchMainResource(fetchRequest, fetcher(), m_substituteData); + FetchRequest cachedResourceRequest(request, FetchInitiatorTypeNames::document, mainResourceLoadOptions); + m_mainResource = RawResource::fetchMainResource(cachedResourceRequest, fetcher(), m_substituteData); if (!m_mainResource) { m_request = ResourceRequest(); + // If the load was aborted by clearing m_request, it's possible the ApplicationCacheHost + // is now in a state where starting an empty load will be inconsistent. Replace it with + // a new ApplicationCacheHost. + if (m_applicationCacheHost) + m_applicationCacheHost->detachFromDocumentLoader(); + m_applicationCacheHost = ApplicationCacheHost::create(this); maybeLoadEmpty(); return; } - m_mainResource->addClient(this); - if (!m_mainResource) - return; // A bunch of headers are set when the underlying ResourceLoader is created, and m_request needs to include those. - // However, if there was a fragment identifier on m_request, the cache will have stripped it. m_request should include - // the fragment identifier, so reset the url. - m_request = m_mainResource->resourceRequest(); - m_request.setURL(m_originalRequest.url()); + if (mainResourceLoader()) + request = mainResourceLoader()->originalRequest(); + // If there was a fragment identifier on m_request, the cache will have stripped it. m_request should include + // the fragment identifier, so add that back in. + if (equalIgnoringFragmentIdentifier(m_request.url(), request.url())) + request.setURL(m_request.url()); + m_request = request; } void DocumentLoader::cancelMainResourceLoad(const ResourceError& resourceError) diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.h b/third_party/WebKit/Source/core/loader/DocumentLoader.h index a1089507e9473..ff2d37547b0fe 100644 --- a/third_party/WebKit/Source/core/loader/DocumentLoader.h +++ b/third_party/WebKit/Source/core/loader/DocumentLoader.h @@ -174,10 +174,12 @@ class CORE_EXPORT DocumentLoader : public RefCountedWillBeGarbageCollectedFinali void prepareSubframeArchiveLoadIfNeeded(); + void willSendRequest(ResourceRequest&, const ResourceResponse&); void finishedLoading(double finishTime); void mainReceivedError(const ResourceError&); void cancelLoadAfterXFrameOptionsOrCSPDenied(const ResourceResponse&); void redirectReceived(Resource*, ResourceRequest&, const ResourceResponse&) final; + void updateRequest(Resource*, const ResourceRequest&) final; void responseReceived(Resource*, const ResourceResponse&, PassOwnPtr) final; void dataReceived(Resource*, const char* data, unsigned length) final; void processData(const char* data, unsigned length); @@ -231,6 +233,7 @@ class CORE_EXPORT DocumentLoader : public RefCountedWillBeGarbageCollectedFinali InitialScrollState m_initialScrollState; enum State { + NotStarted, Provisional, Committed, DataReceived, diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp index 3dc981ca810b0..0fe7ebba61bff 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp +++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp @@ -301,13 +301,10 @@ bool FrameFetchContext::shouldLoadNewResource(Resource::Type type) const return m_documentLoader == frame()->loader().documentLoader(); } -void FrameFetchContext::willStartLoadingResource(ResourceRequest& request, FetchResourceType type) +void FrameFetchContext::willStartLoadingResource(ResourceRequest& request) { - if (!m_documentLoader) - return; - if (type == FetchMainResource) - m_documentLoader->applicationCacheHost()->willStartLoadingMainResource(request); - m_documentLoader->applicationCacheHost()->willStartLoadingResource(request); + if (m_documentLoader) + m_documentLoader->applicationCacheHost()->willStartLoadingResource(request); } void FrameFetchContext::didLoadResource() diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContext.h b/third_party/WebKit/Source/core/loader/FrameFetchContext.h index 3d8704043f2ac..c2cbc7501e40b 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContext.h +++ b/third_party/WebKit/Source/core/loader/FrameFetchContext.h @@ -80,7 +80,7 @@ class CORE_EXPORT FrameFetchContext final : public FetchContext { void dispatchDidFail(unsigned long identifier, const ResourceError&, bool isInternalRequest) override; bool shouldLoadNewResource(Resource::Type) const override; - void willStartLoadingResource(ResourceRequest&, FetchResourceType) override; + void willStartLoadingResource(ResourceRequest&) override; void didLoadResource() override; void addResourceTiming(const ResourceTimingInfo&) override; diff --git a/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp b/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp index bc5e3c2bbeaf4..9e12e0e2984f0 100644 --- a/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp +++ b/third_party/WebKit/Source/core/loader/FrameFetchContextTest.cpp @@ -96,8 +96,8 @@ class FrameFetchContextTest : public ::testing::Test { { dummyPageHolder = DummyPageHolder::create(IntSize(500, 500)); dummyPageHolder->page().setDeviceScaleFactor(1.0); + documentLoader = DocumentLoader::create(&dummyPageHolder->frame(), ResourceRequest("http://www.example.com"), SubstituteData()); document = toHTMLDocument(&dummyPageHolder->document()); - documentLoader = document->loader(); fetchContext = static_cast(&documentLoader->fetcher()->context()); owner = StubFrameOwner::create(); FrameFetchContext::provideDocumentToContext(*fetchContext, document.get()); @@ -105,9 +105,11 @@ class FrameFetchContextTest : public ::testing::Test { void TearDown() override { + documentLoader->detachFromFrame(); documentLoader.clear(); if (childFrame) { + childDocumentLoader->detachFromFrame(); childDocumentLoader.clear(); childFrame->detach(FrameDetachType::Remove); } @@ -119,8 +121,8 @@ class FrameFetchContextTest : public ::testing::Test { childFrame = LocalFrame::create(childClient.get(), document->frame()->host(), owner.get()); childFrame->setView(FrameView::create(childFrame.get(), IntSize(500, 500))); childFrame->init(); + childDocumentLoader = DocumentLoader::create(childFrame.get(), ResourceRequest("http://www.example.com"), SubstituteData()); childDocument = childFrame->document(); - childDocumentLoader = childDocument->loader(); FrameFetchContext* childFetchContext = static_cast(&childDocumentLoader->fetcher()->context()); FrameFetchContext::provideDocumentToContext(*childFetchContext, childDocument.get()); return childFetchContext; diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp index 28d99d964d16b..b6431fd77b190 100644 --- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp +++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp @@ -1323,10 +1323,6 @@ bool FrameLoader::shouldContinueForNavigationPolicy(const ResourceRequest& reque return false; } - bool isFormSubmission = type == NavigationTypeFormSubmitted || type == NavigationTypeFormResubmitted; - if (isFormSubmission && !m_frame->document()->contentSecurityPolicy()->allowFormAction(request.url())) - return false; - policy = client()->decidePolicyForNavigation(request, loader, type, policy, replacesCurrentHistoryItem); if (policy == NavigationPolicyCurrentTab) return true;