Don't stop the documentLoader on navigations.

Source/core/loader/FrameLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ skipping 131 matching lines @@
 FrameLoader::FrameLoader(LocalFrame* frame, FrameLoaderClient* client)
     : m_frame(frame)
     , m_client(client)
     , m_mixedContentChecker(frame)
     , m_progressTracker(FrameProgressTracker::create(m_frame))
     , m_state(FrameStateProvisional)
     , m_loadType(FrameLoadTypeStandard)
     , m_fetchContext(FrameFetchContext::create(frame))
     , m_inStopAllLoaders(false)
     , m_isComplete(false)
+    , m_currentLoaderNeedsStop(false)
     , m_checkTimer(this, &FrameLoader::checkTimerFired)
     , m_shouldCallCheckCompleted(false)
     , m_didAccessInitialDocument(false)
     , m_didAccessInitialDocumentTimer(this, &FrameLoader::didAccessInitialDocumentTimerFired)
     , m_forcedSandboxFlags(SandboxNone)
 {
 }
 
 FrameLoader::~FrameLoader()
 {
@@ skipping 446 matching lines @@
     if (m_frame->view())
         m_frame->view()->maintainScrollPositionAtAnchor(0);
 }
 
 void FrameLoader::started()
 {
     for (LocalFrame* frame = m_frame; frame; frame = frame->tree().parent())
         frame->loader().m_isComplete = false;
 }
 
+void FrameLoader::onResponseReceived()
+{
+    checkCurrentDocumentLoaderNeedsStop();
+}
+
+void FrameLoader::checkCurrentDocumentLoaderNeedsStop()

[Nate Chapin, 2014/03/10 16:12:44]

If this function is only called once, let's merge it into responseReceived()
above.

[mkosiba (inactive), 2014/03/11 23:24:08]

sure, I was keeping it separate because I was experimenting on calling it from
other places too.

+{
+    if (!m_currentLoaderNeedsStop)
+        return;
+
+    for (RefPtr<LocalFrame> child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling())
+        child->loader().stopAllLoaders();
+    if (m_documentLoader)
+        m_documentLoader->stopLoading();
+
+    m_currentLoaderNeedsStop = false;
+
+    if (m_state == FrameStateProvisional)
+        m_progressTracker->progressStarted();

[Nate Chapin, 2014/03/10 16:12:44]

It doesn't seem right to wait to send a progress tracker start notification
until a response is received.

[mkosiba (inactive), 2014/03/11 23:24:08]

this is the meat of the change. The problem is that when you call
loadWithNavigationAction that causes a stopLoading (if a load was in progress)
and an immediate startLoading after that. I'm trying to postpone the stop/start
till we know whether the new loader will not immediately error out.

The alternative to this was adding a new callback (as here:
https://codereview.chromium.org/131363006/).

A third option is to rework the rest of chrome to support overlapping start/stop
notifications, but that's more work so I was hoping to avoid it if possible.

[Nate Chapin, 2014/03/12 19:55:45]

I talked with darin, and our thoughts were:

1. Let's start by seeing how much of a perf impact it would be to just let both
the XHR and the main resource run in parallel. Maybe run some perf measurements
or see what the network stack folks think?

2. If that's going to hurt perf too badly, we could consider using the load
deferral concept in blink. Blink has a way to defer all loading in a page, but
we could use most of that logic to defer just FrameLoader::m_documentLoader
while letting FrameLoader::m_provisionalDocumentLoader proceed normally.

3. If that still causes perf problems, we could look at making load deferral
more aggressively throttle in-progress loads.

Does that seem reasonable?

+}
+
 void FrameLoader::setReferrerForFrameRequest(ResourceRequest& request, ShouldSendReferrer shouldSendReferrer, Document* originDocument)
 {
     if (shouldSendReferrer == NeverSendReferrer) {
         request.clearHTTPReferrer();
         return;
     }
 
     // Always use the initiating document to generate the referrer.
     // We need to generateReferrerHeader(), because we might not have enforced ReferrerPolicy or https->http
     // referrer suppression yet.
@@ skipping 302 matching lines @@
 
 FrameLoadType FrameLoader::loadType() const
 {
     return m_loadType;
 }
 
 void FrameLoader::checkLoadCompleteForThisFrame()
 {
     ASSERT(m_client->hasWebView());
 
+    if (m_currentLoaderNeedsStop)

[Nate Chapin, 2014/03/10 16:12:44]

Why is this needed?

[mkosiba (inactive), 2014/03/11 23:24:08]

because otherwise we'll call
    m_progressTracker->progressCompleted();
before the delayed
     m_progressTracker->progressStarted();
is called.

+        return;
+
     if (m_state == FrameStateProvisional && m_provisionalDocumentLoader) {
         const ResourceError& error = m_provisionalDocumentLoader->mainDocumentError();
         if (error.isNull())
             return;
         RefPtr<DocumentLoader> loader = m_provisionalDocumentLoader;
         m_client->dispatchDidFailProvisionalLoad(error);
         if (loader != m_provisionalDocumentLoader)
             return;
         m_provisionalDocumentLoader->detachFromFrame();
         m_provisionalDocumentLoader = nullptr;
@@ skipping 331 matching lines @@
     m_policyDocumentLoader->setReplacesCurrentHistoryItem(replacesCurrentHistoryItem);
     m_policyDocumentLoader->setIsClientRedirect(clientRedirect == ClientRedirect);
 
     if (LocalFrame* parent = m_frame->tree().parent())
         m_policyDocumentLoader->setOverrideEncoding(parent->loader().documentLoader()->overrideEncoding());
     else if (!overrideEncoding.isEmpty())
         m_policyDocumentLoader->setOverrideEncoding(overrideEncoding);
     else if (m_documentLoader)
         m_policyDocumentLoader->setOverrideEncoding(m_documentLoader->overrideEncoding());
 
-    // stopAllLoaders can detach the LocalFrame, so protect it.
+    // Stopping the provisional loader can detach the Frame, so protect it.
     RefPtr<LocalFrame> protect(m_frame);
     if ((!m_policyDocumentLoader->shouldContinueForNavigationPolicy(request) || !shouldClose()) && m_policyDocumentLoader) {
         m_policyDocumentLoader->detachFromFrame();
         m_policyDocumentLoader = nullptr;
         return;
     }
 
-    // A new navigation is in progress, so don't clear the history's provisional item.
-    stopAllLoaders();
-
-    // <rdar://problem/6250856> - In certain circumstances on pages with multiple frames, stopAllLoaders()
-    // might detach the current FrameLoader, in which case we should bail on this newly defunct load.
-    if (!m_frame->page() || !m_policyDocumentLoader)
-        return;
-
     if (isLoadingMainFrame())
         m_frame->page()->inspectorController().resume();
     m_frame->navigationScheduler().cancel();
 
+    if (m_provisionalDocumentLoader)
+        m_provisionalDocumentLoader->stopLoading();
+    if (m_provisionalDocumentLoader)

[Nate Chapin, 2014/03/10 16:12:44]

Merge these 2 ifs? Or can stopLoading() null out the provisional document
loader?

[mkosiba (inactive), 2014/03/11 23:24:08]

merge - sure. call stopLoading() - no, since that stops the documentLoader which
is what I'm trying to prevent.

+        m_provisionalDocumentLoader->detachFromFrame();
+    m_checkTimer.stop();
+
     m_provisionalDocumentLoader = m_policyDocumentLoader.release();
     m_loadType = type;
     m_state = FrameStateProvisional;
+    m_currentLoaderNeedsStop = true;
 
     if (formState)
         m_client->dispatchWillSubmitForm(formState);
 
-    m_progressTracker->progressStarted();
     if (m_provisionalDocumentLoader->isClientRedirect())
         m_provisionalDocumentLoader->appendRedirect(m_frame->document()->url());
     m_provisionalDocumentLoader->appendRedirect(m_provisionalDocumentLoader->request().url());
     m_client->dispatchDidStartProvisionalLoad();
     ASSERT(m_provisionalDocumentLoader);
     m_provisionalDocumentLoader->startLoadingMainResource();
 }
 
 void FrameLoader::applyUserAgent(ResourceRequest& request)
 {
@@ skipping 106 matching lines @@
 {
     SandboxFlags flags = m_forcedSandboxFlags;
     if (LocalFrame* parentFrame = m_frame->tree().parent())
         flags |= parentFrame->document()->sandboxFlags();
     if (HTMLFrameOwnerElement* ownerElement = m_frame->ownerElement())
         flags |= ownerElement->sandboxFlags();
     return flags;
 }
 
 } // namespace WebCore