Simplify starting a navigation

third_party/WebKit/Source/core/loader/DocumentLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 77 matching lines @@
     , m_fetcher(FrameFetchContext::createContextAndFetcher(this))
     , m_originalRequest(req)
     , m_substituteData(substituteData)
     , m_request(req)
     , m_isClientRedirect(false)
     , m_replacesCurrentHistoryItem(false)
     , m_navigationType(NavigationTypeOther)
     , m_documentLoadTiming(*this)
     , m_timeOfLastDataReceived(0.0)
     , m_applicationCacheHost(ApplicationCacheHost::create(this))
-    , m_state(NotStarted)
+    , m_state(Provisional)
     , m_inDataReceived(false)
     , m_dataBuffer(SharedBuffer::create())
 {
+    timing().markNavigationStart();

[Nate Chapin, 2015/11/04 22:13:09]

As of
https://chromium.googlesource.com/chromium/src/+/b0bae9d815880b722d6fc3833932...,
creating a DocumentLoader means a navigation *will* start, so
markNavigationStart() in the constructor.

 }
 
 FrameLoader* DocumentLoader::frameLoader() const
 {
     if (!m_frame)
         return nullptr;
     return &m_frame->loader();
 }
 
 ResourceLoader* DocumentLoader::mainResourceLoader() const
@@ skipping 131 matching lines @@
         m_state = Committed;
         frameLoader()->commitProvisionalLoad();
     }
 }
 
 bool DocumentLoader::isLoading() const
 {
     if (document() && document()->hasActiveParser())
         return true;
 
-    return (m_state > NotStarted && m_state < MainResourceDone) || m_fetcher->isFetching();
+    return m_state < MainResourceDone || m_fetcher->isFetching();
 }
 
 void DocumentLoader::notifyFinished(Resource* resource)
 {
     ASSERT_UNUSED(resource, m_mainResource == resource);
     ASSERT(m_mainResource);
 
     RefPtrWillBeRawPtr<DocumentLoader> protect(this);
 
     if (!m_mainResource->errorOccurred() && !m_mainResource->wasCanceled()) {
@@ skipping 41 matching lines @@
     if (((status >= 301 && status <= 303) || status == 307)
         && m_originalRequest.httpMethod() == "POST")
         return true;
 
     return false;
 }
 
 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())) {

[Nate Chapin, 2015/11/04 22:13:09]

This is the only clause in willSendRequest() that matters for non-redirects, so
move it to FrameLoader::shouldContinueForNavigationPolicy() and don't call
willSendRequest() for non-redirects.

shouldContinueForNavigationPolicy() now has multiple CSP checks in it, so maybe
it should be renamed...

-        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<SecurityOrigin> 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 (newRequest.cachePolicy() == UseProtocolCachePolicy && isRedirectAfterPost(newRequest, redirectResponse))
-        newRequest.setCachePolicy(ReloadBypassingCache);
+    if (request.cachePolicy() == UseProtocolCachePolicy && isRedirectAfterPost(request, redirectResponse))
+        request.setCachePolicy(ReloadBypassingCache);
 
-    m_request = newRequest;
+    m_request = request;
 
-    if (redirectResponse.isNull())
+    // Add the fragment to the new request url. Note that this is only done on m_request,

[Nate Chapin, 2015/11/04 22:13:09]

Did you know reference fragments are propagated through redirects? I didn't
until debugging why RedirectTest.ServerReference was failing without this block.

+    // 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);
+    }
+
+    // If the redirecting url is not allowed to display content from the target origin,
+    // then block the redirect.
+    RefPtr<SecurityOrigin> redirectingOrigin = SecurityOrigin::create(redirectResponse.url());
+    if (!redirectingOrigin->canDisplay(url)) {
+        FrameLoader::reportLocalLoadFailed(m_frame, url.string());
+        cancelMainResourceLoad(ResourceError::cancelledError(url));
         return;
+    }
 
-    appendRedirect(newRequest.url());
+    timing().addRedirect(redirectResponse.url(), url);
+    appendRedirect(url);
     frameLoader()->receivedMainResourceRedirect(m_request.url());
-    if (!frameLoader()->shouldContinueForNavigationPolicy(newRequest, SubstituteData(), this, CheckContentSecurityPolicy, m_navigationType, NavigationPolicyCurrentTab, replacesCurrentHistoryItem()))
-        cancelMainResourceLoad(ResourceError::cancelledError(m_request.url()));
+    if (!frameLoader()->shouldContinueForNavigationPolicy(request, SubstituteData(), this, CheckContentSecurityPolicy, m_navigationType, NavigationPolicyCurrentTab, replacesCurrentHistoryItem()))
+        cancelMainResourceLoad(ResourceError::cancelledError(url));
 }
 
 static bool canShowMIMEType(const String& mimeType, Page* page)
 {
     if (Platform::current()->mimeRegistry()->supportsMIMEType(mimeType) == WebMimeRegistry::IsSupported)
         return true;
     PluginData* pluginData = page->pluginData();
     return !mimeType.isEmpty() && pluginData && pluginData->supportsMimeType(mimeType);
 }
 
@@ skipping 336 matching lines @@
     if (m_request.url().isEmpty() && !frameLoader()->stateMachine()->creatingInitialEmptyDocument())
         m_request.setURL(blankURL());
     m_response = ResourceResponse(m_request.url(), "text/html", 0, nullAtom, String());
     finishedLoading(monotonicallyIncreasingTime());
     return true;
 }
 
 void DocumentLoader::startLoadingMainResource()
 {
     RefPtrWillBeRawPtr<DocumentLoader> protect(this);
-    m_mainDocumentError = ResourceError();

[Nate Chapin, 2015/11/04 22:13:09]

I have no idea when this line was relevant. Maybe when we supported
multipart/x-mixed-replace main resources?

-    timing().markNavigationStart();
-    ASSERT(!m_mainResource);
-    ASSERT(m_state == NotStarted);
-    m_state = Provisional;
+    ASSERT(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 cachedResourceRequest(request, FetchInitiatorTypeNames::document, mainResourceLoadOptions);
-    m_mainResource = RawResource::fetchMainResource(cachedResourceRequest, fetcher(), m_substituteData);
+    FetchRequest fetchRequest(m_request, FetchInitiatorTypeNames::document, mainResourceLoadOptions);
+
+    m_mainResource = RawResource::fetchMainResource(fetchRequest, 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);

[Nate Chapin, 2015/11/04 22:13:09]

Moved the m_applicationCacheHost->willStartLoadingMainResource(m_request); call
above to FrameFetchContext::willStartLoadingResource(), which makes this
nonsense obsolete.

         maybeLoadEmpty();
         return;
     }
+
     m_mainResource->addClient(this);
+    if (!m_mainResource)

[Nate Chapin, 2015/11/04 22:13:09]

Pre-cached main resources (i.e., data: urls or SubstituteData) can finish
synchronously inside addClient().

+        return;
 
     // A bunch of headers are set when the underlying ResourceLoader is created, and m_request needs to include those.
-    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;
+    // 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());
 }
 
 void DocumentLoader::cancelMainResourceLoad(const ResourceError& resourceError)
 {
     RefPtrWillBeRawPtr<DocumentLoader> protect(this);
     ResourceError error = resourceError.isNull() ? ResourceError::cancelledError(m_request.url()) : resourceError;
 
     if (mainResourceLoader())
         mainResourceLoader()->cancel(error);
 
@@ skipping 51 matching lines @@
 {
     m_writer = createWriterFor(ownerDocument, init, mimeType(), m_writer ? m_writer->encoding() : emptyAtom, true, ForceSynchronousParsing);
     if (!source.isNull())
         m_writer->appendReplacingData(source);
     endWriting(m_writer.get());
 }
 
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader);
 
 } // namespace blink