Reviewed by Maciej.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Oct 2006 13:53:55 +0000 (13:53 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 8 Oct 2006 13:53:55 +0000 (13:53 +0000)
        - fix two recently introduced leaks: one of an NSString, the other of a WebDataSource

        * Loader/WebDocumentLoadState.m: (-[WebDocumentLoadState setTitle:]):
        Rearranged code to avoid storage leak in case of identical title.

        * Loader/WebFrameLoader.h: Removed _setPolicyDocumentLoadState: method
        from the header.
        * Loader/WebFrameLoader.m:
        (-[WebFrameLoader _setPolicyDocumentLoadState:]): Added logic to call detachFromFrameLoader
        as needed if this load state is going away rather than moving on to become the provisional
        load state.
        (-[WebFrameLoader shouldReloadToHandleUnreachableURLFromRequest:]): Tweaked formatting.
        (-[WebFrameLoader _loadRequest:archive:]): Added an assertion.
        (-[WebFrameLoader _loadRequest:triggeringAction:loadType:formState:]): Added an assertion.
        (-[WebFrameLoader _reloadAllowingStaleDataWithOverrideEncoding:]): Added an assertion.
        (-[WebFrameLoader reload]): Added an assertion.
        (-[WebFrameLoader loadDataSource:withLoadType:formState:]): Added a local variable to avoid
        calling _documentLoadState over and over again.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@16887 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKit/ChangeLog
WebKit/Loader/WebDocumentLoadState.m
WebKit/Loader/WebFrameLoader.h
WebKit/Loader/WebFrameLoader.m

index 2e7b536079855ab5961156e40f2b9069f9ccb4b8..e06ba4d535f9b0f7bf4774de85dd1e21da029fad 100644 (file)
@@ -1,3 +1,26 @@
+2006-10-08  Darin Adler  <darin@apple.com>
+
+        Reviewed by Maciej.
+
+        - fix two recently introduced leaks: one of an NSString, the other of a WebDataSource
+
+        * Loader/WebDocumentLoadState.m: (-[WebDocumentLoadState setTitle:]):
+        Rearranged code to avoid storage leak in case of identical title.
+
+        * Loader/WebFrameLoader.h: Removed _setPolicyDocumentLoadState: method
+        from the header.
+        * Loader/WebFrameLoader.m:
+        (-[WebFrameLoader _setPolicyDocumentLoadState:]): Added logic to call detachFromFrameLoader
+        as needed if this load state is going away rather than moving on to become the provisional
+        load state.
+        (-[WebFrameLoader shouldReloadToHandleUnreachableURLFromRequest:]): Tweaked formatting.
+        (-[WebFrameLoader _loadRequest:archive:]): Added an assertion.
+        (-[WebFrameLoader _loadRequest:triggeringAction:loadType:formState:]): Added an assertion.
+        (-[WebFrameLoader _reloadAllowingStaleDataWithOverrideEncoding:]): Added an assertion.
+        (-[WebFrameLoader reload]): Added an assertion.
+        (-[WebFrameLoader loadDataSource:withLoadType:formState:]): Added a local variable to avoid
+        calling _documentLoadState over and over again.
+
 2006-10-07  Don Gibson  <dgibson77@gmail.com>
 
         Reviewed/landed by Adam.
index 0e0ba60004ad1550f83d0c4308c466fd1c180b47..d2fdc554a9c99d78cd01ebb33e1cd9c09b492011 100644 (file)
 
 - (void)setTitle:(NSString *)title
 {
-    NSString *trimmed = nil;
-    if (title) {
-        trimmed = [title mutableCopy];
-        CFStringTrimWhitespace((CFMutableStringRef)trimmed);
-        if ([trimmed length] == 0) {
-            [trimmed release];
-            trimmed = nil;
-        }
-    }
-
-    if (!trimmed)
-        return;
-
-    if ([pageTitle isEqualToString:trimmed])
+    if (!title)
         return;
 
-    [frameLoader willChangeTitleForDocumentLoadState:self];
+    NSString *trimmed = [title mutableCopy];
+    CFStringTrimWhitespace((CFMutableStringRef)trimmed);
 
-    [pageTitle release];
-    pageTitle = [trimmed copy];
+    if ([trimmed length] != 0 && ![pageTitle isEqualToString:trimmed]) {
+        [frameLoader willChangeTitleForDocumentLoadState:self];
+        [pageTitle release];
+        pageTitle = [trimmed copy];
+        [frameLoader didChangeTitleForDocumentLoadState:self];
+    }
 
     [trimmed release];
-
-    [frameLoader didChangeTitleForDocumentLoadState:self];
 }
 
 @end
index 3cc3b97b4d30e65ced5884884835ad81a8e2f3ec..34ffa81005568fc7eeb38d9dfead9c2f98887185 100644 (file)
@@ -123,7 +123,6 @@ typedef enum {
 - (WebDocumentLoadState *)activeDocumentLoadState;
 - (WebDocumentLoadState *)documentLoadState;
 - (WebDocumentLoadState *)provisionalDocumentLoadState;
-- (void)_setPolicyDocumentLoadState:(WebDocumentLoadState *)loadState;
 - (WebFrameState)state;
 - (void)clearDataSource;
 - (void)setupForReplace;
index 8064b1ce1abd8711e95f3a3b3d53c12bd221bb64..0ce96a42003c70457e6b7b1488a1cc8cd6a44514 100644 (file)
@@ -30,6 +30,7 @@
 
 #import "WebDataProtocol.h"
 #import "WebDataSourceInternal.h"
+#import "WebDocumentLoadStateMac.h"
 #import "WebDownloadInternal.h"
 #import "WebFrameBridge.h"
 #import "WebFrameInternal.h"
 
 - (void)_setPolicyDocumentLoadState:(WebDocumentLoadState *)loadState
 {
-    [loadState retain];
+    if (policyDocumentLoadState == loadState)
+        return;
+
+    if (policyDocumentLoadState != provisionalDocumentLoadState && policyDocumentLoadState != documentLoadState)
+        [policyDocumentLoadState detachFromFrameLoader];
+
     [policyDocumentLoadState release];
-    policyDocumentLoadState = loadState;
+    [loadState retain];
+    policyDocumentLoadState = nil;
 }
    
 - (void)clearDataSource
@@ -860,11 +867,10 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
     // case handles malformed URLs and unknown schemes. Loading alternate content
     // at other times behaves like a standard load.
     WebDataSource *compareDataSource = nil;
-    if (delegateIsDecidingNavigationPolicy || delegateIsHandlingUnimplementablePolicy) {
+    if (delegateIsDecidingNavigationPolicy || delegateIsHandlingUnimplementablePolicy)
         compareDataSource = [self policyDataSource];
-    } else if (delegateIsHandlingProvisionalLoadError) {
+    else if (delegateIsHandlingProvisionalLoadError)
         compareDataSource = [self provisionalDataSource];
-    }
     
     return compareDataSource != nil && [unreachableURL isEqual:[[compareDataSource request] URL]];
 }
@@ -873,6 +879,7 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
 {
     WebFrameLoadType type;
     
+    ASSERT(!policyDocumentLoadState);
     policyDocumentLoadState = [client _createDocumentLoadStateWithRequest:request];
     WebDataSource *newDataSource = [client _dataSourceForDocumentLoadState:policyDocumentLoadState];
 
@@ -900,11 +907,11 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
 
 - (void)_loadRequest:(NSURLRequest *)request triggeringAction:(NSDictionary *)action loadType:(WebFrameLoadType)type formState:(WebFormState *)formState
 {
+    ASSERT(!policyDocumentLoadState);
     policyDocumentLoadState = [client _createDocumentLoadStateWithRequest:request];
     WebDataSource *newDataSource = [client _dataSourceForDocumentLoadState:policyDocumentLoadState];
 
     [policyDocumentLoadState setTriggeringAction:action];
-
     [policyDocumentLoadState setOverrideEncoding:[[self documentLoadState] overrideEncoding]];
 
     [self loadDataSource:newDataSource withLoadType:type formState:formState];
@@ -922,6 +929,7 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
         [request setURL:unreachableURL];
 
     [request setCachePolicy:NSURLRequestReturnCacheDataElseLoad];
+    ASSERT(!policyDocumentLoadState);
     policyDocumentLoadState = [client _createDocumentLoadStateWithRequest:request];
     WebDataSource *newDataSource = [client _dataSourceForDocumentLoadState:policyDocumentLoadState];
     [request release];
@@ -949,6 +957,7 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
     if (unreachableURL != nil)
         initialRequest = [NSURLRequest requestWithURL:unreachableURL];
     
+    ASSERT(!policyDocumentLoadState);
     policyDocumentLoadState = [client _createDocumentLoadStateWithRequest:initialRequest];
     WebDataSource *newDataSource = [client _dataSourceForDocumentLoadState:policyDocumentLoadState];
     NSMutableURLRequest *request = [newDataSource request];
@@ -1125,7 +1134,7 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
     [decisionListener release];
 }
 
--(void)_continueAfterNewWindowPolicy:(WebPolicyAction)policy
+- (void)_continueAfterNewWindowPolicy:(WebPolicyAction)policy
 {
     NSURLRequest *request = [[policyRequest retain] autorelease];
     NSString *frameName = [[policyFrameName retain] autorelease];
@@ -1331,17 +1340,18 @@ static BOOL isCaseInsensitiveEqual(NSString *a, NSString *b)
 
     policyLoadType = type;
 
+    WebDocumentLoadStateMac *loadState = (WebDocumentLoadStateMac *)[newDataSource _documentLoadState];
+
     WebFrame *parentFrame = [client parentFrame];
     if (parentFrame)
-        [[newDataSource _documentLoadState] setOverrideEncoding:[[[parentFrame dataSource] _documentLoadState] overrideEncoding]];
+        [loadState setOverrideEncoding:[[[parentFrame dataSource] _documentLoadState] overrideEncoding]];
 
-    WebDocumentLoadStateMac *loadState = (WebDocumentLoadStateMac *)[newDataSource _documentLoadState];
     [loadState setFrameLoader:self];
     [loadState setDataSource:newDataSource];
 
     [self invalidatePendingPolicyDecisionCallingDefaultAction:YES];
 
-    [self _setPolicyDocumentLoadState:[newDataSource _documentLoadState]];
+    [self _setPolicyDocumentLoadState:loadState];
 
     [self checkNavigationPolicyForRequest:[newDataSource request]
                                dataSource:newDataSource