Reviewed by Tim Omernick.
authorsullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Apr 2006 04:17:43 +0000 (04:17 +0000)
committersullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Apr 2006 04:17:43 +0000 (04:17 +0000)
        - fixed <rdar://problem/4139799> Seed: Safari: Private Browsing leaves traces in Icon Cache

        * Misc/WebIconDatabasePrivate.h:
        new ivars: pageURLsBoundDuringPrivateBrowsing, iconURLsBoundDuringPrivateBrowsing, and privateBrowsingEnabled

        * Misc/WebIconDatabase.m:
        (-[NSMutableDictionary init]):
        initialize new ivars, and listen for notifications that WebPreferences changed so we can react to changes
        to private browsing.
        (-[NSMutableDictionary iconForURL:withSize:cache:]):
        Don't remove icon URL from extraRetain dictionary; that's now done in _forgetIconForIconURLString. (I left a
        comment here earlier about why I was worried about this change, but I convinced myself that it's fine.)
        (-[WebIconDatabase removeAllIcons]):
        Removed no-longer-true (and never very clear) comment, and braces. Also remove all objects from the two
        private-browsing-related dictionaries.
        (-[WebIconDatabase _setIcon:forIconURL:]):
        remember icon URL if private browsing is enabled
        (-[WebIconDatabase _setHaveNoIconForIconURL:]):
        remember icon URL if private browsing is enabled
        (-[WebIconDatabase _setIconURL:forURL:]):
        added an assert that helped me out at one point
        (-[WebIconDatabase _clearDictionaries]):
        clear the two new dictionaries too
        (-[WebIconDatabase _loadIconDictionaries]):
        made an existing ERROR not fire in the expected case where there are no icons at all on disk
        (-[WebIconDatabase _updateFileDatabase]):
        when saving the pageURLToIconURL dictionary to disk, first remove any values that were created during
        private browsing
        (-[WebIconDatabase _retainIconForIconURLString:]):
        skip the code that deals with saving changes to disk if private browsing is enabled
        (-[WebIconDatabase _forgetIconForIconURLString:]):
        Remove the icon URL from extraRetain dictionary here. We're forgetting everything about this icon URL
        so we should forget its former extraRetain count too.
        (-[WebIconDatabase _resetCachedWebPreferences:]):
        Cache the new value of private browsing. If it has now been turned off, forget everything we learned
        while it was on. This causes (e.g.) icons for bookmarks or pre-existing history items to be forgotten
        if the icon was only learned during private browsing.

        * History/WebHistoryItem.m:
        removed an unnecessary #import I happened to notice

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

WebKit/ChangeLog
WebKit/History/WebHistoryItem.m
WebKit/Misc/WebIconDatabase.m
WebKit/Misc/WebIconDatabasePrivate.h

index 8f26b2d1c1fd0e4c90a2859fa28b2eaea61a5e0d..a2fca52f1bba8e7f873e2e46e45bfca326d854bc 100644 (file)
@@ -1,3 +1,48 @@
+2006-04-10  John Sullivan  <sullivan@apple.com>
+
+        Reviewed by Tim Omernick.
+        
+        - fixed <rdar://problem/4139799> Seed: Safari: Private Browsing leaves traces in Icon Cache
+        
+        * Misc/WebIconDatabasePrivate.h:
+        new ivars: pageURLsBoundDuringPrivateBrowsing, iconURLsBoundDuringPrivateBrowsing, and privateBrowsingEnabled
+
+        * Misc/WebIconDatabase.m:
+        (-[NSMutableDictionary init]):
+        initialize new ivars, and listen for notifications that WebPreferences changed so we can react to changes
+        to private browsing.
+        (-[NSMutableDictionary iconForURL:withSize:cache:]):
+        Don't remove icon URL from extraRetain dictionary; that's now done in _forgetIconForIconURLString. (I left a
+        comment here earlier about why I was worried about this change, but I convinced myself that it's fine.)
+        (-[WebIconDatabase removeAllIcons]):
+        Removed no-longer-true (and never very clear) comment, and braces. Also remove all objects from the two
+        private-browsing-related dictionaries.
+        (-[WebIconDatabase _setIcon:forIconURL:]):
+        remember icon URL if private browsing is enabled
+        (-[WebIconDatabase _setHaveNoIconForIconURL:]):
+        remember icon URL if private browsing is enabled
+        (-[WebIconDatabase _setIconURL:forURL:]):
+        added an assert that helped me out at one point
+        (-[WebIconDatabase _clearDictionaries]):
+        clear the two new dictionaries too
+        (-[WebIconDatabase _loadIconDictionaries]):
+        made an existing ERROR not fire in the expected case where there are no icons at all on disk
+        (-[WebIconDatabase _updateFileDatabase]):
+        when saving the pageURLToIconURL dictionary to disk, first remove any values that were created during
+        private browsing
+        (-[WebIconDatabase _retainIconForIconURLString:]):
+        skip the code that deals with saving changes to disk if private browsing is enabled
+        (-[WebIconDatabase _forgetIconForIconURLString:]):
+        Remove the icon URL from extraRetain dictionary here. We're forgetting everything about this icon URL
+        so we should forget its former extraRetain count too.
+        (-[WebIconDatabase _resetCachedWebPreferences:]):
+        Cache the new value of private browsing. If it has now been turned off, forget everything we learned
+        while it was on. This causes (e.g.) icons for bookmarks or pre-existing history items to be forgotten
+        if the icon was only learned during private browsing.
+
+        * History/WebHistoryItem.m:
+        removed an unnecessary #import I happened to notice
+        
 2006-04-10  David Hyatt  <hyatt@apple.com>
 
         Make the broken CG focus ring painting work when WebCore sets a clip
index 67ffc8f6503d76666d780ed1a487447b29aa9649..d962cf31d06e673c371deb753501037817886729 100644 (file)
@@ -33,7 +33,6 @@
 #import <WebKit/WebFrameView.h>
 #import <WebKit/WebHTMLViewPrivate.h>
 #import <WebKit/WebIconDatabase.h>
-#import <WebKit/WebIconLoader.h>
 #import <WebKit/WebKitLogging.h>
 #import <WebKit/WebKitNSStringExtras.h>
 #import <WebKit/WebNSDictionaryExtras.h>
index 2179d6567c67d904be7b0174f04ec35cf3c56997..274d80f4b1419d086de49e93f43318f8636a63ad 100644 (file)
@@ -31,8 +31,9 @@
 #import <WebKit/WebIconDatabasePrivate.h>
 #import <WebKit/WebFileDatabase.h>
 #import <WebKit/WebKitLogging.h>
-#import <WebKit/WebNSURLExtras.h>
 #import <WebKit/WebKitNSStringExtras.h>
+#import <WebKit/WebNSURLExtras.h>
+#import <WebKit/WebPreferences.h>
 
 NSString * const WebIconDatabaseVersionKey =   @"WebIconDatabaseVersion";
 NSString * const WebURLToIconURLKey =          @"WebSiteURLToIconURLKey";
@@ -69,6 +70,7 @@ NSSize WebIconLargeSize = {128, 128};
 - (void)_releaseIconForIconURLString:(NSString *)iconURL;
 - (void)_retainOriginalIconsOnDisk;
 - (void)_releaseOriginalIconsOnDisk;
+- (void)_resetCachedWebPreferences:(NSNotification *)notification;
 - (void)_sendNotificationForURL:(NSString *)URL;
 - (int)_totalRetainCountForIconURLString:(NSString *)iconURLString;
 - (NSImage *)_largestIconFromDictionary:(NSMutableDictionary *)icons;
@@ -120,11 +122,18 @@ NSSize WebIconLargeSize = {128, 128};
     _private->iconsToEraseWithURLs = [[NSMutableSet alloc] init];
     _private->iconsToSaveWithURLs = [[NSMutableSet alloc] init];
     _private->iconURLsWithNoIcons = [[NSMutableSet alloc] init];
+    _private->iconURLsBoundDuringPrivateBrowsing = [[NSMutableSet alloc] init];
+    _private->pageURLsBoundDuringPrivateBrowsing = [[NSMutableSet alloc] init];
+    _private->privateBrowsingEnabled = [[WebPreferences standardPreferences] privateBrowsingEnabled];
     
     [[NSNotificationCenter defaultCenter] addObserver:self
                                              selector:@selector(_applicationWillTerminate:)
                                                  name:NSApplicationWillTerminateNotification
                                                object:NSApp];
+    [[NSNotificationCenter defaultCenter] 
+            addObserver:self selector:@selector(_resetCachedWebPreferences:) 
+                   name:WebPreferencesChangedNotification object:nil];
+    
 
     // Retain icons on disk then release them once clean-up has begun.
     // This gives the client the opportunity to retain them before they are erased.
@@ -160,9 +169,6 @@ NSSize WebIconLargeSize = {128, 128};
            // disk behind our back?). Forget that we ever had it so it will be re-fetched next time.
            ERROR("WebIconDatabase used to contain %@, but the icon file is missing. Now forgetting that we ever knew about this icon.", iconURLString);
             [self _forgetIconForIconURLString:iconURLString];
-            // _forgetIconForIconURLString doesn't modify the retain count, so we need to do so here. Perhaps it would be
-            // safe to do this in _forgetIconForIconURLString, but the comment in removeAllIcons implies otherwise.
-            CFDictionaryRemoveValue(_private->iconURLToExtraRetainCount, iconURLString);
        }
         return [self defaultIconWithSize:size];
     }        
@@ -311,13 +317,8 @@ NSSize WebIconLargeSize = {128, 128};
 {
     NSEnumerator *keyEnumerator = [(NSDictionary *)_private->iconURLToPageURLs keyEnumerator];
     NSString *iconURLString;
-    while ((iconURLString = [keyEnumerator nextObject]) != nil) {
-        // Note that _forgetIconForIconURLString does not affect retain counts, so the current clients
-        // need not do anything about retaining/releasing icons here. (However, the current clients should
-        // respond to WebIconDatabaseDidRemoveAllIconsNotification by refetching any icons that are 
-        // displayed in the UI.) 
+    while ((iconURLString = [keyEnumerator nextObject]) != nil)
         [self _forgetIconForIconURLString:iconURLString];
-    }
     
     // Delete entire file database immediately. This has at least three advantages over waiting for
     // _updateFileDatabase to execute:
@@ -328,6 +329,8 @@ NSSize WebIconLargeSize = {128, 128};
     [_private->fileDatabase removeAllObjects];
     [_private->iconsToEraseWithURLs removeAllObjects];
     [_private->iconsToSaveWithURLs removeAllObjects];
+    [_private->iconURLsBoundDuringPrivateBrowsing removeAllObjects];
+    [_private->pageURLsBoundDuringPrivateBrowsing removeAllObjects];
     [self _clearDictionaries];
     [[NSNotificationCenter defaultCenter] postNotificationName:WebIconDatabaseDidRemoveAllIconsNotification
                                                         object:self
@@ -356,6 +359,11 @@ NSSize WebIconLargeSize = {128, 128};
     
     [_private->iconURLToIcons setObject:icons forKey:iconURL];
     
+    // Don't update any icon information on disk during private browsing. Remember which icons have been
+    // affected during private browsing so we can forget this information when private browsing is turned off.
+    if (_private->privateBrowsingEnabled)
+        [_private->iconURLsBoundDuringPrivateBrowsing addObject:iconURL];
+
     [self _retainIconForIconURLString:iconURL];
     
     // Release the newly created icon much like an autorelease.
@@ -371,8 +379,13 @@ NSSize WebIconLargeSize = {128, 128};
 
     [_private->iconURLsWithNoIcons addObject:iconURL];
     
+    // Don't update any icon information on disk during private browsing. Remember which icons have been
+    // affected during private browsing so we can forget this information when private browsing is turned off.
+    if (_private->privateBrowsingEnabled)
+        [_private->iconURLsBoundDuringPrivateBrowsing addObject:iconURL];
+
     [self _retainIconForIconURLString:iconURL];
-    
+
     // Release the newly created icon much like an autorelease.
     // This gives the client enough time to retain it.
     // FIXME: Should use an actual autorelease here using a proxy object instead.
@@ -386,6 +399,7 @@ NSSize WebIconLargeSize = {128, 128};
     ASSERT(URL);
     ASSERT([self _isEnabled]);
     ASSERT([self _hasIconForIconURL:iconURL]);
+    ASSERT(_private->pageURLToIconURL);
 
     if ([[_private->pageURLToIconURL objectForKey:URL] isEqualToString:iconURL]) {
         // Don't do any work if the icon URL is already bound to the site URL
@@ -398,6 +412,11 @@ NSSize WebIconLargeSize = {128, 128};
         // a nonexistent icon
         return;
     }
+    
+    // Keep track of which entries in pageURLToIconURL were created during private browsing so that they can be skipped
+    // when saving to disk.
+    if (_private->privateBrowsingEnabled)
+        [_private->pageURLsBoundDuringPrivateBrowsing addObject:URL];
 
     [_private->pageURLToIconURL setObject:iconURL forKey:URL];
     [_private->iconURLToPageURLs _web_setObjectUsingSetIfNecessary:URL forKey:iconURL];
@@ -442,10 +461,14 @@ NSSize WebIconLargeSize = {128, 128};
     [_private->iconURLToPageURLs release];
     [_private->iconsOnDiskWithURLs release];
     [_private->originalIconsOnDiskWithURLs release];
+    [_private->iconURLsBoundDuringPrivateBrowsing release];
+    [_private->pageURLsBoundDuringPrivateBrowsing release];
     _private->pageURLToIconURL = [[NSMutableDictionary alloc] init];
     _private->iconURLToPageURLs = [[NSMutableDictionary alloc] init];
     _private->iconsOnDiskWithURLs = [[NSMutableSet alloc] init];
     _private->originalIconsOnDiskWithURLs = [[NSMutableSet alloc] init];
+    _private->iconURLsBoundDuringPrivateBrowsing = [[NSMutableSet alloc] init];
+    _private->pageURLsBoundDuringPrivateBrowsing = [[NSMutableSet alloc] init];
 }
 
 - (void)_loadIconDictionaries
@@ -479,8 +502,10 @@ NSSize WebIconLargeSize = {128, 128};
     }
     
     // Must double-check all values read from disk. If any are bogus, we just throw out the whole icon cache.
+    // We expect this to be nil if the icon cache has been cleared, so we shouldn't whine in that case.
     if (![pageURLToIconURL isKindOfClass:[NSMutableDictionary class]]) {
-        ERROR("Clearing icon cache because bad value %@ was found on disk, expected an NSMutableDictionary", pageURLToIconURL);
+        if (pageURLToIconURL)
+            ERROR("Clearing icon cache because bad value %@ was found on disk, expected an NSMutableDictionary", pageURLToIconURL);
         [self _clearDictionaries];
         return;
     }
@@ -567,9 +592,12 @@ NSSize WebIconLargeSize = {128, 128};
     [_private->iconsToEraseWithURLs removeAllObjects];
     [_private->iconsToSaveWithURLs removeAllObjects];
 
-    // Save the icon dictionaries to disk. Save them as mutable copies otherwise WebFileDatabase may access the 
-    // same dictionaries on a separate thread as it's being modified. We think this fixes 3566336.
+    // Save the icon dictionaries to disk, after removing any values created during private browsing.
+    // Even if we weren't modifying the dictionary we'd still want to use a copy so that WebFileDatabase
+    // doesn't look at the original from a different thread. (We used to think this would fix 3566336
+    // but that bug's progeny are still alive and kicking.)
     NSMutableDictionary *pageURLToIconURLCopy = [_private->pageURLToIconURL mutableCopy];
+    [pageURLToIconURLCopy removeObjectsForKeys:[_private->pageURLsBoundDuringPrivateBrowsing allObjects]];
     [fileDB setObject:pageURLToIconURLCopy forKey:WebURLToIconURLKey];
     [pageURLToIconURLCopy release];
 }
@@ -691,7 +719,7 @@ NSSize WebIconLargeSize = {128, 128};
 
     CFDictionarySetValue(_private->iconURLToExtraRetainCount, iconURLString, (void *)newRetainCount);
 
-    if (newRetainCount == 1) {
+    if (newRetainCount == 1 && !_private->privateBrowsingEnabled) {
 
         // Either we know nothing about this icon and need to save it to disk, or we were planning to remove it
         // from disk (as set up in _forgetIconForIconURLString:) and should stop that process.
@@ -712,7 +740,7 @@ NSSize WebIconLargeSize = {128, 128};
         [_private->iconsToEraseWithURLs addObject:iconURLString];
         [_private->iconsToSaveWithURLs removeObject:iconURLString];
     }
-    
+        
     // Remove the icon's images
     [_private->iconURLToIcons removeObjectForKey:iconURLString];
     
@@ -731,6 +759,7 @@ NSSize WebIconLargeSize = {128, 128};
         }
     }
     [_private->iconURLToPageURLs removeObjectForKey:iconURLString];
+    CFDictionaryRemoveValue(_private->iconURLToExtraRetainCount, iconURLString);
     [iconURLString release];
 }
 
@@ -784,6 +813,37 @@ NSSize WebIconLargeSize = {128, 128};
     _private->didCleanup = YES;
 }
 
+- (void)_resetCachedWebPreferences:(NSNotification *)notification
+{
+    BOOL privateBrowsingEnabledNow = [[WebPreferences standardPreferences] privateBrowsingEnabled];
+    if (privateBrowsingEnabledNow == _private->privateBrowsingEnabled)
+        return;
+    
+    _private->privateBrowsingEnabled = privateBrowsingEnabledNow;
+    
+    // When private browsing is turned off, forget everything we learned while it was on 
+    if (!_private->privateBrowsingEnabled) {
+        // Forget all of the icons whose existence we learned of during private browsing.
+        NSEnumerator *iconEnumerator = [_private->iconURLsBoundDuringPrivateBrowsing objectEnumerator];
+        NSString *iconURLString;
+        while ((iconURLString = [iconEnumerator nextObject]) != nil)
+            [self _forgetIconForIconURLString:iconURLString];
+
+        // Forget the relationships between page and icon that we learned during private browsing.
+        NSEnumerator *pageEnumerator = [_private->pageURLsBoundDuringPrivateBrowsing objectEnumerator];
+        NSString *pageURLString;
+        while ((pageURLString = [pageEnumerator nextObject]) != nil) {
+            [_private->pageURLToIconURL removeObjectForKey:pageURLString];
+            // Tell clients that these pages' icons have changed (to generic). The notification is named
+            // WebIconDatabaseDidAddIconNotification but it really means just "did change icon".
+            [self _sendNotificationForURL:pageURLString];
+        }
+        
+        [_private->iconURLsBoundDuringPrivateBrowsing removeAllObjects];
+        [_private->pageURLsBoundDuringPrivateBrowsing removeAllObjects];
+    }
+}
+
 - (void)_sendNotificationForURL:(NSString *)URL
 {
     ASSERT(URL);
index 6a24d8c2cf0b4ed8dc4810e7e8f5a5792b37ec02..95cbdcdf898737cd54b46a4daf55d4280bd9c9c7 100644 (file)
@@ -38,7 +38,7 @@
 
     NSMutableDictionary *iconURLToIcons;
     NSMutableDictionary *iconURLToPageURLs;
-    NSMutableDictionary *pageURLToIconURL;    
+    NSMutableDictionary *pageURLToIconURL;
     CFMutableDictionaryRef pageURLToRetainCount;
     CFMutableDictionaryRef iconURLToExtraRetainCount;
     
     NSMutableSet *iconsToSaveWithURLs;
     NSMutableSet *iconURLsWithNoIcons;
     NSMutableSet *originalIconsOnDiskWithURLs;
+    NSMutableSet *pageURLsBoundDuringPrivateBrowsing;
+    NSMutableSet *iconURLsBoundDuringPrivateBrowsing;
     
     int cleanupCount;
 
     BOOL didCleanup;
     BOOL waitingToCleanup;
+    BOOL privateBrowsingEnabled;
 
     NSMutableDictionary *htmlIcons;
     NSMutableDictionary *defaultIcons;