[webkit-reviews] Fix for regression on apple dashboard download page

[Maciej Stachowiak]

http://bugzilla.opendarwin.org/show_bug.cgi?id=3445

Layout tests pass. I did not add a new one because I could not think  
of an obvious way to make a layout test do a download without using a  
remote resource.

-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebKit/ChangeLog,v
retrieving revision 1.3186
diff -u -p -r1.3186 ChangeLog
--- ChangeLog	11 Jun 2005 00:42:51 -0000	1.3186
+++ ChangeLog	12 Jun 2005 09:44:48 -0000
@@ -1,3 +1,22 @@
+2005-06-12  Maciej Stachowiak  <mjs at apple.com>
+
+        Reviewed by NOBODY (OOPS!).
+
+	- fixed <rdar://problem/4142247> REGRESSION: List to browse widgets at Apple website failed. Closing tab afterwards caused Safari crash
+	http://bugzilla.opendarwin.org/show_bug.cgi?id=3445
+	
+        * WebView.subproj/WebFrame.m:
+        (-[WebFrame _transitionToCommitted:]): Stop loading the non-provisional data
+	source before swapping in the provisional.
+        (-[WebFrame _continueLoadRequestAfterNavigationPolicy:formState:]): Stop only
+	the provisional load here, we would not want to stop loading if this navigation
+	later turns into a download or is cancelled before being committed.
+        (-[WebFrame stopLoading]): Factored a bit.
+        (-[WebFrame _cancelProvisionalLoad]): New method to stop only provisional load,
+	and cancel any pending policy deicions.
+        (-[WebFrame _stopNonProvisionalLoadOnly]): New mthod that stops only the main
+	load.
+
 2005-06-10  John Sullivan  <sullivan at apple.com>
 
         reviewed by Dave Harrison (first & second drafts) and Darin Adler (third draft)
Index: WebView.subproj/WebFrame.m
===================================================================
RCS file: /cvs/root/WebKit/WebView.subproj/WebFrame.m,v
retrieving revision 1.230
diff -u -p -r1.230 WebView.subproj/WebFrame.m
--- WebView.subproj/WebFrame.m	5 Jun 2005 17:54:47 -0000	1.230
+++ WebView.subproj/WebFrame.m	12 Jun 2005 09:44:48 -0000
@@ -183,6 +183,8 @@ NSString *WebPageCacheDocumentViewKey = 
 - (WebHistoryItem *)_createItem: (BOOL)useOriginal;
 - (WebHistoryItem *)_createItemTreeWithTargetFrame:(WebFrame *)targetFrame clippedAtTarget:(BOOL)doClip;
 - (WebHistoryItem *)_currentBackForwardListItemToResetTo;
+- (void)_cancelProvisionalLoad;
+- (void)_stopNonProvisionalLoadOnly;
 @end
 
 @implementation WebFramePrivate
@@ -765,6 +767,7 @@ NSString *WebPageCacheDocumentViewKey = 
             }
 
             // Set the committed data source on the frame.
+            [self _stopNonProvisionalLoadOnly];
             [self _setDataSource:_private->provisionalDataSource];
                 
             [self _setProvisionalDataSource: nil];
@@ -2325,7 +2328,7 @@ static CFAbsoluteTime _timeOfLastComplet
     WebFrameLoadType loadType = _private->policyLoadType;
     WebDataSource *dataSource = [_private->policyDataSource retain];
     
-    [self stopLoading];
+    [self _cancelProvisionalLoad];
     [self _setLoadType:loadType];
     [self _setProvisionalDataSource:dataSource];
     [dataSource release];
@@ -2855,6 +2858,13 @@ static CFAbsoluteTime _timeOfLastComplet
     }
 }
 
+- (void)_cancelProvisionalLoad
+{
+    [self _invalidatePendingPolicyDecisionCallingDefaultAction:YES];
+    [_private->provisionalDataSource _stopLoading];
+    [self _setProvisionalDataSource:nil];
+}
+
 - (void)stopLoading
 {
     // If this method is called from within this method, infinite recursion can occur (3442218). Avoid this.
@@ -2862,15 +2872,19 @@ static CFAbsoluteTime _timeOfLastComplet
         return;
     }
     _private->isStoppingLoad = YES;
-    
-    [self _invalidatePendingPolicyDecisionCallingDefaultAction:YES];
-
-    [_private->provisionalDataSource _stopLoading];
+    [self _cancelProvisionalLoad];
     [_private->dataSource _stopLoading];
+    _private->isStoppingLoad = NO;
+}
 
-    // Release the provisional data source because there's no point in keeping it around since it is unused in this case.
-    [self _setProvisionalDataSource:nil];
-    
+- (void)_stopNonProvisionalLoadOnly
+{
+    // If this method is called from within this method, infinite recursion can occur (3442218). Avoid this.
+    if (_private->isStoppingLoad) {
+        return;
+    }
+    _private->isStoppingLoad = YES;
+    [_private->dataSource _stopLoading];
     _private->isStoppingLoad = NO;
 }
 
-------------- next part --------------

I'm not sure this totally clears up the timing of when closeURL gets  
called, I think it still happens twice on a navigation, but this  
should keep you from ending up with a dead page when you click on a  
download link.

Regards,
Maciej