Reviewed by Tim Omernick.
authorsullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 May 2006 15:17:41 +0000 (15:17 +0000)
committersullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 May 2006 15:17:41 +0000 (15:17 +0000)
        - fixed <rdar://problem/3126419> history load enforces history limit, but deletes the newest instead of oldest items
        - added notification reporting items discarded during load because the age limit or item count limit is exceeded
        - a few other minor tweaks

        * History/WebHistory.h:
        fixed a typo and an incorrect method name

        * History/WebHistoryPrivate.h:
        Added declaration of WebHistoryItemsDiscardedWhileLoadingNotification. Also changed signature of
        WebHistoryPrivate method -loadFromURL:error: to have new collectDiscardedItemsInto: parameter.
        Also deleted declarations of two methods that didn't actually exist (loadHistory and initWithFile:),
        and added comments about which methods should become public API, WebKit-internal, or file-internal.

        * History/WebHistory.m:
        (-[WebHistoryPrivate arrayRepresentation]):
        This method, called only by _saveHistoryGuts:, used to deliberately leave out items that violated
        either the age limit or the item count limit. Now all the items are included (and thus saved), and
        all the pruning is done at load time, so clients can keep track of the pruned items by observing
        the new WebHistoryItemsDiscardedWhileLoadingNotification
        (-[WebHistoryPrivate _loadHistoryGutsFromURL:savedItemsCount:collectDiscardedItemsInto:error:]):
        Now keeps track of all the items that violated the age limit or item count limit in the new
        collectedDiscardedItemsInto: parameter. Also, now processes items in forward order rather than
        reverse order to fix 3126419. Now uses compare: rather than _webkit_compareDay: to check against
        age limit; this is faster and also more correct (most noticeable with small age limits).
        (-[WebHistoryPrivate loadFromURL:collectDiscardedItemsInto:error:]):
        new collectDiscardedItemsInto: parameter, passed into _loadHistoryGuts:...
        (-[WebHistory loadFromURL:error:]):
        Now sends new WebHistoryItemsDiscardedWhileLoadingNotification if any items were discarded due
        to age limit or item count limit.

        * WebKit.exp:
        exported symbol for WebHistoryItemsDiscardedWhileLoadingNotification

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

WebKit/ChangeLog
WebKit/History/WebHistory.h
WebKit/History/WebHistory.m
WebKit/History/WebHistoryPrivate.h

index a549e270bf70c5b5c046c0f0b448715e9f73aec6..4be7365c354955142946660a1a7c0bcf029aeb8d 100644 (file)
@@ -1,3 +1,40 @@
+2006-05-01  John Sullivan  <sullivan@apple.com>
+
+        Reviewed by Tim Omernick.
+        
+        - fixed <rdar://problem/3126419> history load enforces history limit, but deletes the newest instead of oldest items
+        - added notification reporting items discarded during load because the age limit or item count limit is exceeded
+        - a few other minor tweaks
+
+        * History/WebHistory.h:
+        fixed a typo and an incorrect method name
+        
+        * History/WebHistoryPrivate.h:
+        Added declaration of WebHistoryItemsDiscardedWhileLoadingNotification. Also changed signature of
+        WebHistoryPrivate method -loadFromURL:error: to have new collectDiscardedItemsInto: parameter.
+        Also deleted declarations of two methods that didn't actually exist (loadHistory and initWithFile:), 
+        and added comments about which methods should become public API, WebKit-internal, or file-internal.
+
+        * History/WebHistory.m:
+        (-[WebHistoryPrivate arrayRepresentation]):
+        This method, called only by _saveHistoryGuts:, used to deliberately leave out items that violated
+        either the age limit or the item count limit. Now all the items are included (and thus saved), and
+        all the pruning is done at load time, so clients can keep track of the pruned items by observing
+        the new WebHistoryItemsDiscardedWhileLoadingNotification
+        (-[WebHistoryPrivate _loadHistoryGutsFromURL:savedItemsCount:collectDiscardedItemsInto:error:]):
+        Now keeps track of all the items that violated the age limit or item count limit in the new
+        collectedDiscardedItemsInto: parameter. Also, now processes items in forward order rather than
+        reverse order to fix 3126419. Now uses compare: rather than _webkit_compareDay: to check against
+        age limit; this is faster and also more correct (most noticeable with small age limits).
+        (-[WebHistoryPrivate loadFromURL:collectDiscardedItemsInto:error:]):
+        new collectDiscardedItemsInto: parameter, passed into _loadHistoryGuts:...
+        (-[WebHistory loadFromURL:error:]):
+        Now sends new WebHistoryItemsDiscardedWhileLoadingNotification if any items were discarded due
+        to age limit or item count limit.
+        
+        * WebKit.exp:
+        exported symbol for WebHistoryItemsDiscardedWhileLoadingNotification
+
 2006-04-29  Timothy Hatcher  <timothy@apple.com>
 
         Reviewed by Maciej.
index b1ed573f59355f9263b4c95b8f5d430985a93218..e1ba947c306e75283f8113c867d66e6100cdc72b 100644 (file)
     @constant WebHistoryItemsAddedNotification Posted from addItems:.  This 
     notification comes with a userInfo dictionary that contains the array of
     items added.  The key for the array is WebHistoryItemsKey.
-    @constant WebHistoryItemsRemovedNotification Posted from and removeItems:.  
+    @constant WebHistoryItemsRemovedNotification Posted from removeItems:.  
     This notification comes with a userInfo dictionary that contains the array of
     items removed.  The key for the array is WebHistoryItemsKey.
     @constant WebHistoryAllItemsRemovedNotification Posted from removeAllItems
-    @constant WebHistoryLoadedNotification Posted from loadHistory.
+    @constant WebHistoryLoadedNotification Posted from loadFromURL:error:.
 */
 extern NSString *WebHistoryItemsAddedNotification;
 extern NSString *WebHistoryItemsRemovedNotification;
index 25dc9e5c7596b5b027bf109760f630359444f30f..b0b43822ecd7ba604bbfe2f36720c2e6beefdb2f 100644 (file)
@@ -43,6 +43,7 @@ NSString *WebHistoryItemsAddedNotification = @"WebHistoryItemsAddedNotification"
 NSString *WebHistoryItemsRemovedNotification = @"WebHistoryItemsRemovedNotification";
 NSString *WebHistoryAllItemsRemovedNotification = @"WebHistoryAllItemsRemovedNotification";
 NSString *WebHistoryLoadedNotification = @"WebHistoryLoadedNotification";
+NSString *WebHistoryItemsDiscardedWhileLoadingNotification = @"WebHistoryItemsDiscardedWhileLoadingNotification";
 NSString *WebHistorySavedNotification = @"WebHistorySavedNotification";
 NSString *WebHistoryItemsKey = @"WebHistoryItems";
 
@@ -366,55 +367,34 @@ NSString *DatesArrayKey = @"WebHistoryDates";
 }
 
 // Return a date that marks the age limit for history entries saved to or
-// loaded from disk. Any entry on this day or older should be rejected,
-// as tested with -[NSCalendarDate compareDay:]
+// loaded from disk. Any entry older than this item should be rejected.
 - (NSCalendarDate *)_ageLimitDate
 {
     return [[NSCalendarDate calendarDate] dateByAddingYears:0 months:0 days:-[self historyAgeInDaysLimit]
                                                       hours:0 minutes:0 seconds:0];
 }
 
-// Return a flat array of WebHistoryItems. Leaves out entries older than the age limit.
-// Stops filling array when item count limit is reached, even if there are currently
-// more entries than that.
+// Return a flat array of WebHistoryItems. Ignores the date and item count limits; these are
+// respected when loading instead of when saving, so that clients can learn of discarded items
+// by listening to WebHistoryItemsDiscardedWhileLoadingNotification.
 - (NSArray *)arrayRepresentation
 {
-    int dateCount, dateIndex;
-    int limit;
-    int totalSoFar;
-    NSMutableArray *arrayRep;
-    NSCalendarDate *ageLimitDate;
+    NSMutableArray *arrayRep = [NSMutableArray array];
 
-    arrayRep = [NSMutableArray array];
-
-    limit = [self historyItemLimit];
-    ageLimitDate = [self _ageLimitDate];
-    totalSoFar = 0;
-    
-    dateCount = [_entriesByDate count];
+    int dateCount = [_entriesByDate count];
+    int dateIndex;
     for (dateIndex = 0; dateIndex < dateCount; ++dateIndex) {
-        int entryCount, entryIndex;
-        NSArray *entries;
-
-        // skip remaining days if they are older than the age limit
-        if ([[_datesWithEntries objectAtIndex:dateIndex] _webkit_compareDay:ageLimitDate] != NSOrderedDescending) {
-            break;
-        }
-
-        entries = [_entriesByDate objectAtIndex:dateIndex];
-        entryCount = [entries count];
-        for (entryIndex = 0; entryIndex < entryCount; ++entryIndex) {
-            if (totalSoFar++ >= limit) {
-                break;
-            }
+        NSArray *entries = [_entriesByDate objectAtIndex:dateIndex];
+        int entryCount = [entries count];
+        int entryIndex;
+        for (entryIndex = 0; entryIndex < entryCount; ++entryIndex)
             [arrayRep addObject: [[entries objectAtIndex:entryIndex] dictionaryRepresentation]];
-        }
     }
 
     return arrayRep;
 }
 
-- (BOOL)_loadHistoryGuts: (int *)numberOfItemsLoaded URL:(NSURL *)URL error:(NSError **)error
+- (BOOL)_loadHistoryGutsFromURL:(NSURL *)URL savedItemsCount:(int *)numberOfItemsLoaded collectDiscardedItemsInto:(NSMutableArray *)discardedItems error:(NSError **)error
 {
     *numberOfItemsLoaded = 0;
 
@@ -461,51 +441,48 @@ NSString *DatesArrayKey = @"WebHistoryDates";
 
     NSArray *array = [fileAsDictionary objectForKey:DatesArrayKey];
         
-    int limit = [self historyItemLimit];
+    int itemCountLimit = [self historyItemLimit];
     NSCalendarDate *ageLimitDate = [self _ageLimitDate];
-    int index = 0;
-    // reverse dates so you're loading the oldest first, to minimize the number of comparisons
-    NSEnumerator *enumerator = [array reverseObjectEnumerator];
+    NSEnumerator *enumerator = [array objectEnumerator];
     BOOL ageLimitPassed = NO;
-
+    BOOL itemLimitPassed = NO;
+    ASSERT(*numberOfItemsLoaded == 0);
+    
     NSDictionary *itemAsDictionary;
     while ((itemAsDictionary = [enumerator nextObject]) != nil) {
-        WebHistoryItem *entry;
+        WebHistoryItem *item = [[WebHistoryItem alloc] initFromDictionaryRepresentation:itemAsDictionary];
 
-        entry = [[[WebHistoryItem alloc] initFromDictionaryRepresentation:itemAsDictionary] autorelease];
-
-        if ([entry URLString] == nil) {
-            // entry without URL is useless; data on disk must have been bad; ignore
-            continue;
-        }
-
-        // test against date limit
-        if (!ageLimitPassed) {
-            if ([[entry _lastVisitedDate] _webkit_compareDay:ageLimitDate] != NSOrderedDescending) {
-                continue;
-            } else {
+        // item without URL is useless; data on disk must have been bad; ignore
+        if ([item URLString]) {
+            // Test against date limit. Since the items are ordered newest to oldest, we can stop comparing
+            // once we've found the first item that's too old.
+            if (!ageLimitPassed && ([[item _lastVisitedDate] compare:ageLimitDate] != NSOrderedDescending))
                 ageLimitPassed = YES;
+            
+            if (ageLimitPassed || itemLimitPassed)
+                [discardedItems addObject:item];
+            else {
+                [self addItem:item];
+                ++(*numberOfItemsLoaded);
+                if (*numberOfItemsLoaded == itemCountLimit)
+                    itemLimitPassed = YES;
             }
         }
         
-        [self addItem: entry];
-        if (++index >= limit) {
-            break;
-        }
+        [item release];
     }
 
-    *numberOfItemsLoaded = MIN(index, limit);
     return YES;    
 }
 
-- (BOOL)loadFromURL:(NSURL *)URL error:(NSError **)error
+- (BOOL)loadFromURL:(NSURL *)URL collectDiscardedItemsInto:(NSMutableArray *)discardedItems error:(NSError **)error
 {
     int numberOfItems;
     double start, duration;
     BOOL result;
 
     start = CFAbsoluteTimeGetCurrent();
-    result = [self _loadHistoryGuts: &numberOfItems URL:URL error:error];
+    result = [self _loadHistoryGutsFromURL:URL savedItemsCount:&numberOfItems collectDiscardedItemsInto:discardedItems error:error];
 
     if (result) {
         duration = CFAbsoluteTimeGetCurrent() - start;
@@ -830,10 +807,16 @@ static inline bool matchUnicodeLetter(UniChar c, UniChar lowercaseLetter)
 
 - (BOOL)loadFromURL:(NSURL *)URL error:(NSError **)error
 {
-    if ([_historyPrivate loadFromURL:URL error:error]) {
+    NSMutableArray *discardedItems = [NSMutableArray array];
+    
+    if ([_historyPrivate loadFromURL:URL collectDiscardedItemsInto:discardedItems error:error]) {
         [[NSNotificationCenter defaultCenter]
-            postNotificationName: WebHistoryLoadedNotification
-                          object: self];
+            postNotificationName:WebHistoryLoadedNotification
+                          object:self];
+        
+        if ([discardedItems count] > 0)
+            [self _sendNotification:WebHistoryItemsDiscardedWhileLoadingNotification entries:discardedItems];
+        
         return YES;
     }
     return NO;
index 0e80a59f56bb9ca894e8c19b7dcd3f1dba6c5aed..3a4cf7b02bf3f8bd5f0a4329da60ca1f94beb11d 100644 (file)
 @class WebHistoryItem;
 @class NSError;
 
+/*
+    @constant WebHistoryItemsDiscardedWhileLoadingNotification Posted from loadFromURL:error:.  
+    This notification comes with a userInfo dictionary that contains the array of
+    items discarded due to the date limit or item limit.  The key for the array is WebHistoryItemsKey.
+*/
+// FIXME: This notification should become API in WebHistory.h
+extern NSString *WebHistoryItemsDiscardedWhileLoadingNotification;
+
+// FIXME: The WebHistoryPrivate interface should be in WebHistoryInternal.h or inside WebHistory.m
 @interface WebHistoryPrivate : NSObject {
 @private
     NSMutableDictionary *_entriesByURL;
@@ -58,7 +67,7 @@
 - (WebHistoryItem *)itemForURL:(NSURL *)URL;
 - (WebHistoryItem *)itemForURLString:(NSString *)URLString;
 
-- (BOOL)loadFromURL:(NSURL *)URL error:(NSError **)error;
+- (BOOL)loadFromURL:(NSURL *)URL collectDiscardedItemsInto:(NSMutableArray *)discardedItems error:(NSError **)error;
 - (BOOL)saveToURL:(NSURL *)URL error:(NSError **)error;
 
 - (NSCalendarDate*)_ageLimitDate;
 @end
 
 @interface WebHistory (WebPrivate)
+
+// FIXME: The following SPI is used by Safari. Should it be made into public API?
+- (WebHistoryItem *)_itemForURLString:(NSString *)URLString;
+
+// FIXME: Safari doesn't use the following SPI, and it's used in WebKit only inside WebHistory.m.
+// Should we move it into a FileInternal category inside WebHistory.m, or do we need it for other
+// clients?
 - (void)removeItem:(WebHistoryItem *)entry;
 - (void)addItem:(WebHistoryItem *)entry;
+- (BOOL)containsItemForURLString:(NSString *)URLString;
+
+// FIXME: Safari doesn't use the following SPI, but other WebKit classes do. Should we move it into
+// a WebHistoryInternal.h, or do we need it for other clients?
+- (WebHistoryItem *)addItemForURL:(NSURL *)URL;
 // Change date on existing item
 - (void)setLastVisitedTimeInterval:(NSTimeInterval)time forItem:(WebHistoryItem *)item;
 
-- (BOOL)loadHistory;
-- initWithFile:(NSString *)file;
-- (WebHistoryItem *)addItemForURL:(NSURL *)URL;
-- (BOOL)containsItemForURLString:(NSString *)URLString;
-- (WebHistoryItem *)_itemForURLString:(NSString *)URLString;
+// FIXME: neither Safari nor WebKit use the following SPI -- do we still need it?
 - (NSCalendarDate*)ageLimitDate;
 
 @end