Changeset 19583 in webkit

Timestamp:

Feb 12, 2007, 3:01:01 PM (18 years ago)

Author:

kdecker

Message:

Reviewed by Anders.

Fixed: <rdar://problem/4874059> REGRESSION: Painter IX:register - Crash in WebCore

ResourceLoader::willSendRequest() What happened here was that a WebDataSource was being dealloced *while* a load for that resource is still in progress.

• loader/FrameLoader.cpp: (WebCore::FrameLoader::stopAllLoaders): Calling stopAllLoaders cancels loads and informs the frame load delegate accordingly. The delegate however may decide to kick off a new provisional load as the result of the cancel. Therefore a local variable for the provisional and main doucment loader is introduced, and we now only nil out the provisional loader if the local and member provisional variables represent the exact same load. (WebCore::FrameLoader::continueLoadAfterNavigationPolicy): Added the same check here, too.

Location:

trunk/WebCore

Files:

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

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-02-12  Kevin Decker <kdecker@apple.com>
+
+        Reviewed by Anders.
+
+        Fixed: <rdar://problem/4874059> REGRESSION: Painter IX:register - Crash in WebCore:: ResourceLoader::willSendRequest()
+        
+        What happened here was that a WebDataSource was being dealloced *while* a load for that resource is still in progress. 
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::stopAllLoaders): Calling stopAllLoaders cancels loads and informs the frame load delegate accordingly.
+        The delegate however may decide to kick off a new provisional load as the result of the cancel. Therefore a local variable 
+        for the provisional and main doucment loader is introduced, and we now only nil out the provisional loader if the local and
+        member provisional variables represent the exact same load.
+        (WebCore::FrameLoader::continueLoadAfterNavigationPolicy): Added the same check here, too.
+
 2007-02-12  Lars Knoll  <lars@trolltech.com>
 

trunk/WebCore/loader/FrameLoader.cpp

@@ -2102 +2102 @@
 
     stopLoadingSubframes();
-    if (m_provisionalDocumentLoader)
-        m_provisionalDocumentLoader->stopLoading();
-    if (m_documentLoader)
-        m_documentLoader->stopLoading();
-    setProvisionalDocumentLoader(0);
+    RefPtr<DocumentLoader> provisionalDocumentLoader = m_provisionalDocumentLoader;
+    RefPtr<DocumentLoader> documentLoader = m_documentLoader;
+    if (provisionalDocumentLoader)
+        provisionalDocumentLoader->stopLoading();
+
+    // Calling stopLoading() on the provisional loader results in a delegate callback, 
+    // which may kick off a brand new provisional load. So only nil out the provisional loader
+    // if we're dealing with the exact same loader. 
+    if (provisionalDocumentLoader == m_provisionalDocumentLoader)
+        setProvisionalDocumentLoader(0);
+
+    if (documentLoader)
+        documentLoader->stopLoading();
+    
     m_client->clearArchivedResources();
-
     m_inStopAllLoaders = false;    
 }
@@ -3362 +3370 @@
 
     FrameLoadType type = m_policyLoadType;
-    stopAllLoaders();
+    {
+        RefPtr<DocumentLoader> previousProvisionalDocumentLoader = m_provisionalDocumentLoader;
+        stopAllLoaders();
+        
+        // stopAllLoaders cancels loads and informs the frame load delegate accordingly. The delegate however,
+        // may decide to kick off a new provisional load as the result of the cancel, and if it does, we need
+        // to bail out now and avoid prematurely destroying the new provisional load in progress. 
+        if (m_provisionalDocumentLoader != previousProvisionalDocumentLoader)
+            return;
+    }
+    
     setProvisionalDocumentLoader(m_policyDocumentLoader.get());
     m_loadType = type;