Changeset 115549 in webkit

Timestamp:

Apr 27, 2012 11:28:37 PM (12 years ago)

Author:

jochen@chromium.org

Message:

Ensure that there's always a provisional document loader if the frame loader is in provisional state
​https://bugs.webkit.org/show_bug.cgi?id=83894

Reviewed by Nate Chapin.

We're still seeing crashes in the FrameLoader where the FrameLoader's
state is "provisional" but there is no provisional document loader. I
added code to update the FrameLoader's state everytime the provisional
document loader is cleared, and added checks that the FrameLoader's
state can't be set to provisional without a provisional loader.

If the crashes go away, or the newly added checks reveal the culprit,
we should relex the checks to use ASSERT() instead of CRASH().

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::init):
(WebCore::FrameLoader::setupForReplace):
(WebCore::FrameLoader::stopAllLoaders):
(WebCore::FrameLoader::clearProvisionalLoad):
(WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy):
(WebCore::FrameLoader::continueLoadAfterNavigationPolicy):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/FrameLoader.cpp (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-04-27  Jochen Eisinger  <jochen@chromium.org>
+
+        Ensure that there's always a provisional document loader if the frame loader is in provisional state
+        https://bugs.webkit.org/show_bug.cgi?id=83894
+
+        Reviewed by Nate Chapin.
+
+        We're still seeing crashes in the FrameLoader where the FrameLoader's
+        state is "provisional" but there is no provisional document loader. I
+        added code to update the FrameLoader's state everytime the provisional
+        document loader is cleared, and added checks that the FrameLoader's
+        state can't be set to provisional without a provisional loader.
+
+        If the crashes go away, or the newly added checks reveal the culprit,
+        we should relex the checks to use ASSERT() instead of CRASH().
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::init):
+        (WebCore::FrameLoader::setupForReplace):
+        (WebCore::FrameLoader::stopAllLoaders):
+        (WebCore::FrameLoader::clearProvisionalLoad):
+        (WebCore::FrameLoader::continueFragmentScrollAfterNavigationPolicy):
+        (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+
 2012-04-27  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -218 +218 @@
     m_stateMachine.advanceTo(FrameLoaderStateMachine::CreatingInitialEmptyDocument);
     setPolicyDocumentLoader(m_client->createDocumentLoader(ResourceRequest(KURL(ParsedURLString, emptyString())), SubstituteData()).get());
+    // FIXME: Make this an ASSERT() instead once we figured out what's going wrong.
+    if (!m_policyDocumentLoader.get())
+        CRASH();
     setProvisionalDocumentLoader(m_policyDocumentLoader.get());
     setState(FrameStateProvisional);
@@ -1116 +1119 @@
 void FrameLoader::setupForReplace()
 {
+    // FIXME: Make this an ASSERT() instead once we figured out what's going wrong.
+    if (!m_documentLoader.get())
+        CRASH();
     setState(FrameStateProvisional);
     m_provisionalDocumentLoader = m_documentLoader;
@@ -1524 +1530 @@
 
     setProvisionalDocumentLoader(0);
+    if (m_state == FrameStateProvisional)
+        setState(FrameStateComplete);
 
     m_checkTimer.stop();
@@ -1638 +1646 @@
 void FrameLoader::clearProvisionalLoad()
 {
-    setProvisionalDocumentLoader(0);
     if (Page* page = m_frame->page())
         page->progress()->progressCompleted(m_frame);
+    setProvisionalDocumentLoader(0);
     setState(FrameStateComplete);
 }
@@ -2596 +2604 @@
         m_provisionalDocumentLoader->stopLoading();
         setProvisionalDocumentLoader(0);
+        // FIXME: Make this an ASSERT() instead once we figured out what's going wrong.
+        if (m_state == FrameStateProvisional)
+            CRASH();
     }
 
@@ -2758 +2769 @@
 #endif
 
+    // FIXME: Make this an ASSERT() instead once we figured out what's going wrong.
+    if (!m_policyDocumentLoader.get())
+        CRASH();
     setProvisionalDocumentLoader(m_policyDocumentLoader.get());
     m_loadType = type;