Don't stop the documentLoader on navigations.

Source/core/loader/FrameLoader.cpp

Index: Source/core/loader/FrameLoader.cpp
diff --git a/Source/core/loader/FrameLoader.cpp b/Source/core/loader/FrameLoader.cpp
index f5fb5b154a7023794214f4516d996c25b347ad88..3caaa717e05c1ab5559bf8ecf510b2f5a463c4aa 100644
--- a/Source/core/loader/FrameLoader.cpp
+++ b/Source/core/loader/FrameLoader.cpp
@@ -1288,13 +1288,13 @@ void FrameLoader::loadWithNavigationAction(const NavigationAction& action, Frame
         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)

[Nate Chapin, 2014/05/20 17:47:09]

I don't think that removing this early return is safe.
DocumentLoader::stopLoading() doesn't do as much work as
FrameLoader::stopAllLoaders(), but I think if this is an iframe it can still
call into FrameLoader::checkCompleted(), which can call the parent's
checkCompleted(), which can fire the parent's load event and do work to trigger
the cases in this if().

[mkosiba (inactive), 2014/05/20 19:06:10]

I will leave the check in. If I wanted to hit this scenario manually what would
be a good test case? An iframe that navigates the parent?

Also, when you fiddle with it, how do you test the code? I ended up running
browser_tests and content_browsertests but those are incredibly slooow.

[Nate Chapin, 2014/05/20 19:16:06]

In terms of repro, I have never actually seen a reliable repro for either of
those cases. The m_frame->page() check has existed from time immemorial, and the
m_policyDocumentLoader check was in response to a top crasher I couldn't repro,
but fixed based on crash stacks. :(

In terms of testing, I usually run layout tests and webkit_unit_tests locally,
then send try jobs for everything else, and approximate "good enough to request
review" when the trybots have passed at least one bot.

-        return;
+    if (m_provisionalDocumentLoader) {
+        m_provisionalDocumentLoader->stopLoading();
+        if (m_provisionalDocumentLoader)
+            m_provisionalDocumentLoader->detachFromFrame();
+        m_provisionalDocumentLoader = nullptr;
+    }
+    m_checkTimer.stop();
 
     if (isLoadingMainFrame())
         m_frame->page()->inspectorController().resume();
@@ -1308,12 +1308,14 @@ void FrameLoader::loadWithNavigationAction(const NavigationAction& action, Frame
         m_client->dispatchWillSubmitForm(formState->form());
 
     m_progressTracker->progressStarted();
+

[Nate Chapin, 2014/05/20 17:47:09]

Any reason for the extra newlines here and below?

[mkosiba (inactive), 2014/05/20 19:06:10]

Done.

     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)