Improve Top 10 bookmarks list maintenance, ensuring that we look at both visit count and last visit date to keep the list stable in the face of changing visit counts, and optimize list building slightly.

Add methods to allow bookmark sorting, currently only used by the top 10 list.

Remove a nasty hack that set the parent of a bookmark item to an NSNumber for Rendezvous bookmarks, replacing it with a RendezvousBookmark Bookmark subclass.

Fix a bug in the Bookmark enumerator, that could cause it to propagate above its root folder, and hang in some cases.

Optimize the notification handlers for various bookmark changes (notably visit count changes) to reduce the amount of work done when resetting all visit counts.

Optimize the building of the dictionary that maps from bookmarks to their favicon urls.

Add a stack-based C++ class that can be used to time an event.

camino/src/bookmarks/Bookmark.h

@@ -77,3 +77,20 @@
 - (id)savedFaviconURL;
 
 @end
+
+
+@interface RendezvousBookmark : Bookmark
+{
+  int       mServiceID;
+  BOOL      mResolved;
+}
+
+- (id)initWithServiceID:(int)inServiceID;
+- (void)setServiceID:(int)inServiceID;
+- (int)serviceID;
+
+- (BOOL)resolved;
+- (void)setResolved:(BOOL)inResolved;
+
+@end
+

camino/src/bookmarks/Bookmark.mm

@@ -475,4 +475,121 @@ NSString* const URLLoadSuccessKey     = @"url_bool";
     return nil;
 }
 
+#pragma mark -
+
+// sorting
+
+- (NSComparisonResult)compareURL:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  NSComparisonResult result;
+  // sort folders before sites
+  if ([aItem isKindOfClass:[BookmarkFolder class]])
+    result = NSOrderedDescending;
+  else
+    result = [[self url] compare:[(Bookmark*)aItem url] options:NSCaseInsensitiveSearch];
+
+  return [inDescending boolValue] ? (NSComparisonResult)(-1 * (int)result) : result;
+}
+
+// base class does the title compare
+
+- (NSComparisonResult)compareType:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  NSComparisonResult result;
+  // sort folders before other stuff, and separators before bookmarks
+  if ([aItem isKindOfClass:[BookmarkFolder class]])
+    result = NSOrderedDescending;
+  else
+    result = (NSComparisonResult)((int)[self isSeparator] - (int)[(Bookmark*)aItem isSeparator]);
+
+  return [inDescending boolValue] ? (NSComparisonResult)(-1 * (int)result) : result;
+}
+
+- (NSComparisonResult)compareVisitCount:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  NSComparisonResult result;
+  // sort folders before other stuff
+  if ([aItem isKindOfClass:[BookmarkFolder class]])
+    result = NSOrderedDescending;
+  else
+  {
+    int myVisits    = [self numberOfVisits];
+    int otherVisits = [(Bookmark*)aItem numberOfVisits];
+    if (myVisits == otherVisits)
+      result = NSOrderedSame;
+    else
+      result = (otherVisits > myVisits) ? NSOrderedAscending : NSOrderedDescending;
+  }
+  
+  return [inDescending boolValue] ? (NSComparisonResult)(-1 * (int)result) : result;
+}
+
+- (NSComparisonResult)compareLastVisitDate:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  NSComparisonResult result;
+  // sort categories before sites
+  if ([aItem isKindOfClass:[BookmarkFolder class]])
+    result = NSOrderedDescending;
+  else
+    result = [mLastVisit compare:[(Bookmark*)aItem lastVisit]];
+
+  return [inDescending boolValue] ? (NSComparisonResult)(-1 * (int)result) : result;
+}
+
+- (NSComparisonResult)compareForTop10:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  NSComparisonResult result;
+  // sort folders before other stuff
+  if ([aItem isKindOfClass:[BookmarkFolder class]])
+    result = NSOrderedDescending;
+  else
+  {
+    int myVisits    = [self numberOfVisits];
+    int otherVisits = [(Bookmark*)aItem numberOfVisits];
+    if (myVisits == otherVisits)
+      result = [mLastVisit compare:[(Bookmark*)aItem lastVisit]];
+    else
+      result = (otherVisits > myVisits) ? NSOrderedAscending : NSOrderedDescending;
+  }
+  
+  return [inDescending boolValue] ? (NSComparisonResult)(-1 * (int)result) : result;
+}
+
 @end 
+
+#pragma mark -
+
+@implementation RendezvousBookmark
+
+- (id)initWithServiceID:(int)inServiceID
+{
+  if ((self = [super init]))
+  {
+    mServiceID = inServiceID;
+    mResolved = NO;
+  }
+  return self;
+}
+
+- (void)setServiceID:(int)inServiceID
+{
+  mServiceID = inServiceID;
+}
+
+- (int)serviceID
+{
+  return mServiceID;
+}
+
+- (BOOL)resolved
+{
+  return mResolved;
+}
+
+- (void)setResolved:(BOOL)inResolved
+{
+  mResolved = inResolved;
+}
+
+@end
+

camino/src/bookmarks/BookmarkFolder.h

@@ -121,8 +121,15 @@ enum {
 
 // Smart Folder only methods
 -(void) insertIntoSmartFolderChild:(BookmarkItem *)aItem;
+-(void) insertIntoSmartFolderChild:(BookmarkItem *)aItem atIndex:(unsigned)inIndex;
 -(void) deleteFromSmartFolderChildAtIndex:(unsigned)index;
 
+// sorting
+- (void)sortChildrenUsingSelector:(SEL)inSelector reverseSort:(BOOL)inReverse sortDeep:(BOOL)inDeep;
+- (void)sortChildrenUsingPrimarySelector:(SEL)inSelector primaryReverseSort:(BOOL)inReverse
+                       secondarySelector:(SEL)inSecondarySelector secondaryReverseSort:(BOOL)inSecondaryReverse
+                                sortDeep:(BOOL)inDeep;
+
 // generation menus
 -(void) buildFlatFolderList:(NSMenu *)menu depth:(unsigned)pad;
 

camino/src/bookmarks/BookmarkFolder.mm

@@ -51,6 +51,28 @@ NSString* const BookmarkFolderChildIndexKey             = @"bf_ik";
 NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
 
 
+struct BookmarkSortData
+{
+  SEL       mSortSelector;
+  NSNumber* mReverseSort;
+
+  SEL       mSecondarySortSelector;
+  NSNumber* mSecondaryReverseSort;
+};
+
+
+static int BookmarkItemSort(id firstItem, id secondItem, void* context)
+{
+  BookmarkSortData* sortData = (BookmarkSortData*)context;
+  int comp = (int)[firstItem performSelector:sortData->mSortSelector withObject:secondItem withObject:sortData->mReverseSort];
+
+  if (comp == 0 && sortData->mSecondarySortSelector)
+    comp = (int)[firstItem performSelector:sortData->mSecondarySortSelector withObject:secondItem withObject:sortData->mSecondaryReverseSort];
+
+  return comp;
+}
+
+
 @interface BookmarksEnumerator : NSEnumerator
 {
   BookmarkFolder*   mRootFolder;
@@ -92,6 +114,7 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
 // notification methods
 -(void) itemAddedNote:(BookmarkItem *)theItem atIndex:(unsigned)anIndex;
 -(void) itemRemovedNote:(BookmarkItem *)theItem;
+-(void) itemChangedNote:(BookmarkItem *)theItem;
 -(void) dockMenuChanged:(NSNotification *)note;
 
 // aids in searching
@@ -153,23 +176,22 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
   // we're at the end of this folder.
   while (1)
   {
+    if (mCurFolder == mRootFolder)  // we're done
+    {
+      mCurFolder = nil;
+      break;
+    }
+
     // back up to parent folder, next index
     BookmarkFolder* newCurFolder = [mCurFolder parent];
     mCurChildIndex = [newCurFolder indexOfObject:mCurFolder] + 1;
     mCurFolder = newCurFolder;
-    
     if (mCurChildIndex < (int)[newCurFolder count])
     {
       BookmarkItem* nextItem = [mCurFolder objectAtIndex:mCurChildIndex];
       [self setupNextForItem:nextItem];
       return nextItem;
     }
-    
-    if (mCurFolder == mRootFolder)  // we finished
-    {
-      mCurFolder = nil;
-      break;
-    }
   }
   
   return nil;
@@ -323,9 +345,7 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
 
 -(BOOL) isSpecial
 {
-  if ([self isToolbar] || [self isRoot] || [self isSmartFolder] || [self isDockMenu])
-    return YES;
-  return NO;
+  return ([self isToolbar] || [self isRoot] || [self isSmartFolder] || [self isDockMenu]);
 }
 
 -(BOOL) isToolbar
@@ -487,14 +507,23 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
 //
 // Smart folder utilities - we don't set ourself as parent
 //
--(void) insertIntoSmartFolderChild:(BookmarkItem *)aItem
+- (void)insertIntoSmartFolderChild:(BookmarkItem *)aItem
 {
   if ([self isSmartFolder]) {
     [[self childArray] addObject:aItem];
-    [self itemAddedNote:aItem atIndex:([self count]-1)];
+    [self itemAddedNote:aItem atIndex:([self count] - 1)];
   }
 }
--(void) deleteFromSmartFolderChildAtIndex:(unsigned)index
+
+- (void)insertIntoSmartFolderChild:(BookmarkItem *)aItem atIndex:(unsigned)inIndex
+{
+  if ([self isSmartFolder]) {
+    [[self childArray] insertObject:aItem atIndex:inIndex];
+    [self itemAddedNote:aItem atIndex:inIndex];
+  }
+}
+
+- (void)deleteFromSmartFolderChildAtIndex:(unsigned)index
 {
   if ([self isSmartFolder]) {
     BookmarkItem *item = [[[self childArray] objectAtIndex:index] retain];
@@ -503,6 +532,54 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
   }   
 }
 
+- (void)sortChildrenUsingSelector:(SEL)inSelector reverseSort:(BOOL)inReverse sortDeep:(BOOL)inDeep
+{
+  BookmarkSortData sortData = { inSelector, [NSNumber numberWithBool:inReverse], NULL, nil };
+  [mChildArray sortUsingFunction:BookmarkItemSort context:&sortData];
+  
+  // notify
+  [self itemChangedNote:self];
+  
+  if (inDeep)
+  {
+    NSEnumerator *enumerator = [mChildArray objectEnumerator];
+    id childItem;
+    while ((childItem = [enumerator nextObject]))
+    {
+      if ([childItem isKindOfClass:[BookmarkFolder class]])
+        [childItem sortChildrenUsingSelector:inSelector reverseSort:inReverse sortDeep:inDeep];
+    }
+  }
+}
+
+- (void)sortChildrenUsingPrimarySelector:(SEL)inSelector primaryReverseSort:(BOOL)inReverse
+                       secondarySelector:(SEL)inSecondarySelector secondaryReverseSort:(BOOL)inSecondaryReverse
+                                sortDeep:(BOOL)inDeep
+{
+  BookmarkSortData sortData = { inSelector,           [NSNumber numberWithBool:inReverse],
+                                inSecondarySelector,  [NSNumber numberWithBool:inSecondaryReverse]
+                              };
+  [mChildArray sortUsingFunction:BookmarkItemSort context:&sortData];
+  
+  // notify
+  [self itemChangedNote:self];
+  
+  if (inDeep)
+  {
+    NSEnumerator *enumerator = [mChildArray objectEnumerator];
+    id childItem;
+    while ((childItem = [enumerator nextObject]))
+    {
+      if ([childItem isKindOfClass:[BookmarkFolder class]])
+        [childItem sortChildrenUsingPrimarySelector:inSelector
+                                 primaryReverseSort:inReverse
+                                  secondarySelector:inSecondarySelector
+                               secondaryReverseSort:inSecondaryReverse
+                                           sortDeep:inDeep];
+    }
+  }
+}
+
 //
 // Adding bookmarks
 //
@@ -970,6 +1047,11 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
   [[NSNotificationCenter defaultCenter] postNotification:note];
 }
 
+-(void) itemChangedNote:(BookmarkItem *)theItem
+{
+  [self itemUpdatedNote:kBookmarkItemChildrenChangedMask];
+}
+
 -(void) dockMenuChanged:(NSNotification *)note
 {
   if (([self isDockMenu]) && ([note object] != self))
@@ -977,10 +1059,11 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
 }
 
 
+#pragma mark -
+
 //
 // reading/writing from/to disk
 //
-#pragma mark -
 
 -(BOOL) readNativeDictionary:(NSDictionary *)aDict
 {
@@ -1332,6 +1415,7 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
   [self removeFromChildArrayAtIndex:aIndex];
   [self insertChild:aItem atIndex:aIndex isMove:NO];
 }
+
 //
 // child bookmark stuff
 //
@@ -1369,6 +1453,7 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
   [self removeFromChildBookmarksAtIndex:aIndex];
   [self insertInChildBookmarks:aItem atIndex:aIndex];
 }
+
 //
 // child bookmark folder stuff
 //
@@ -1434,11 +1519,14 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
     NSRelativePosition relPos = [relSpec relativePosition];
     if (baseSpec == nil)
       return nil;
+
     if ([kiddies count] == 0)
       return [NSArray array];
+
     if ([baseKey isEqualToString:@"childBookmarks"] ||
         [baseKey isEqualToString:@"childArray"] ||
-        [baseKey isEqualToString:@"childFolders"] ){
+        [baseKey isEqualToString:@"childFolders"] )
+    {
       unsigned baseIndex;
       id baseObject = [baseSpec objectsByEvaluatingWithContainers:self];
       if ([baseObject isKindOfClass:[NSArray class]]) {
@@ -1446,16 +1534,17 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
         if (baseCount == 0) 
           baseObject = nil;
         else {
-          if (relPos == NSRelativeBefore){
+          if (relPos == NSRelativeBefore)
             baseObject = [baseObject objectAtIndex:0];
-          } else {
+          else
             baseObject = [baseObject objectAtIndex:(baseCount-1)];
         }
       }
-      }
+
       if (!baseObject) 
         // Oops.  We could not find the base object.
         return nil;
+
       baseIndex = [kiddies indexOfObjectIdenticalTo:baseObject];
       if (baseIndex == NSNotFound) 
         // Oops.  We couldn't find the base object in the child array.  This should not happen.
@@ -1470,7 +1559,9 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
           baseIndex--;
       else 
           baseIndex++;
-      while ((baseIndex >= 0) && (baseIndex < kiddiesCount)) {
+
+      while ((baseIndex >= 0) && (baseIndex < kiddiesCount))
+      {
         if (keyIsArray) {
           [result addObject:[NSNumber numberWithInt:baseIndex]];
           break;
@@ -1482,6 +1573,7 @@ NSString* const BookmarkFolderDockMenuChangeNotificaton = @"bf_dmc";
             break;
           }
         }
+
         if (relPos == NSRelativeBefore)
           baseIndex--;
         else

camino/src/bookmarks/BookmarkItem.h

@@ -54,6 +54,9 @@ enum
   kBookmarkItemStatusChangedMask        = (1 << 7),   // really "flags", like separator vs. bookmark
   kBookmarkItemNumVisitsChangedMask     = (1 << 8),
 
+  // flags for bookmark folder changes
+  kBookmarkItemChildrenChangedMask      = (1 << 9),
+  
   // mask of flags that require a save of the bookmarks
   kBookmarkItemSignificantChangeFlagsMask = kBookmarkItemTitleChangedMask |
                                             kBookmarkItemDescriptionChangedMask |
@@ -78,6 +81,9 @@ enum
   unsigned int    mPendingChangeFlags;
 }
 
+// returns YES if any of the supplied flags are set in the userInfo
++ (BOOL)bookmarkChangedNotificationUserInfo:(NSDictionary*)inUserInfo containsFlags:(unsigned int)inFlags;
+
 // Setters/Getters
 -(id) parent;
 -(NSString *) title;
@@ -137,6 +143,16 @@ enum
 - (id)savedKeyword;
 - (id)savedUUID;    // does not generate a new UUID if UUID is not set
 
+// sorting
+
+// we put sort comparators on the base class for convenience
+- (NSComparisonResult)compareURL:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending;
+- (NSComparisonResult)compareTitle:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending;
+- (NSComparisonResult)compareType:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending;
+- (NSComparisonResult)compareVisitCount:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending;
+- (NSComparisonResult)compareLastVisitDate:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending;
+- (NSComparisonResult)compareForTop10:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending;
+
 @end
 
 // Bunch of Keys for reading/writing dictionaries.

camino/src/bookmarks/BookmarkItem.mm

@@ -93,6 +93,18 @@ NSString* const CaminoTrueKey = @"true";
 
 static BOOL gSuppressAllUpdates = NO;
 
+
++ (BOOL)bookmarkChangedNotificationUserInfo:(NSDictionary*)inUserInfo containsFlags:(unsigned int)inFlags
+{
+  unsigned int changeFlags = kBookmarkItemEverythingChangedMask;  // assume everything changed
+  NSNumber* noteChangeFlags = [inUserInfo objectForKey:BookmarkItemChangedFlagsKey];
+  if (noteChangeFlags)
+    changeFlags = [noteChangeFlags unsignedIntValue];
+
+  return ((changeFlags & inFlags) != 0);
+}
+
+
 //Initialization
 -(id) init
 {
@@ -132,6 +144,7 @@ static BOOL gSuppressAllUpdates = NO;
   [super dealloc];
 }
 
+@class BookmarkFolder;
 
 // Basic properties
 -(id) parent
@@ -405,6 +418,41 @@ static BOOL gSuppressAllUpdates = NO;
   return mUUID ? mUUID : @"";
 }
 
+#pragma mark -
+
+// sorting
+
+- (NSComparisonResult)compareURL:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  return NSOrderedSame;
+}
+
+- (NSComparisonResult)compareTitle:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  NSComparisonResult result = [[self title] compare:[aItem title] options:NSCaseInsensitiveSearch];
+  return [inDescending boolValue] ? (NSComparisonResult)(-1 * (int)result) : result;
+}
+
+- (NSComparisonResult)compareType:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  return NSOrderedSame;
+}
+
+- (NSComparisonResult)compareVisitCount:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  return NSOrderedSame;
+}
+
+- (NSComparisonResult)compareLastVisitDate:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  return NSOrderedSame;
+}
+
+- (NSComparisonResult)compareForTop10:(BookmarkItem *)aItem sortDescending:(NSNumber*)inDescending
+{
+  return NSOrderedSame;
+}
+
 @end
 
 

camino/src/bookmarks/BookmarkManager.mm

@@ -279,6 +279,7 @@ static BookmarkManager* gBookmarkManager = nil;
   // All interested parties haven't been init'd yet, and/or will receive the
   // managerStartedNotification when setup is actually complete.
   [BookmarkItem setSuppressAllUpdateNotifications:YES];
+
   BOOL bookmarksReadOK = [self readBookmarks];
   if (!bookmarksReadOK)
   {
@@ -790,8 +791,9 @@ static BookmarkManager* gBookmarkManager = nil;
   NSMutableSet* urlSet = [urlMap objectForKey:inURL];
   if (!urlSet)
   {
-    urlSet = [NSMutableSet setWithCapacity:1];
+    urlSet = [[NSMutableSet alloc] initWithCapacity:1];
     [urlMap setObject:urlSet forKey:inURL];
+    [urlSet release];
   }
   [urlSet addObject:inBookmark];
 }
@@ -843,7 +845,9 @@ static BookmarkManager* gBookmarkManager = nil;
   [BookmarkManager addItem:inBookmark toURLMap:mBookmarkURLMap usingURL:bookmarkURL];
 
   // and add it to the site icon map
-  [BookmarkManager addItem:inBookmark toURLMap:mBookmarkFaviconURLMap usingURL:[BookmarkManager faviconURLForBookmark:inBookmark]];
+  NSString* faviconURL = [BookmarkManager faviconURLForBookmark:inBookmark];
+  if ([faviconURL length] > 0)
+    [BookmarkManager addItem:inBookmark toURLMap:mBookmarkFaviconURLMap usingURL:faviconURL];
 }
 
 - (void)unregisterBookmarkForLoads:(Bookmark*)inBookmark ignoringURL:(BOOL)inIgnoreURL
@@ -851,7 +855,9 @@ static BookmarkManager* gBookmarkManager = nil;
   NSString* bookmarkURL = inIgnoreURL ? nil : [BookmarkManager canonicalBookmarkURL:[inBookmark url]];
   [BookmarkManager removeItem:inBookmark fromURLMap:mBookmarkURLMap usingURL:bookmarkURL];
   
-  [BookmarkManager removeItem:inBookmark fromURLMap:mBookmarkFaviconURLMap usingURL:[BookmarkManager faviconURLForBookmark:inBookmark]];
+  NSString* faviconURL = [BookmarkManager faviconURLForBookmark:inBookmark];
+  if ([faviconURL length] > 0)
+    [BookmarkManager removeItem:inBookmark fromURLMap:mBookmarkFaviconURLMap usingURL:faviconURL];
 }
 
 

camino/src/bookmarks/BookmarkViewController.mm

@@ -481,10 +481,9 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
   while ((curItem = [itemsEnum nextObject]))
   {
     // see if it's a rendezvous item
-    id parent = [curItem parent];
-    if (![parent isKindOfClass:[BookmarkItem class]])
+    if ([curItem isKindOfClass:[RendezvousBookmark class]] && ![curItem resolved])
     {
-      [[NetworkServices sharedNetworkServices] attemptResolveService:[parent intValue] forSender:curItem];
+      [[NetworkServices sharedNetworkServices] attemptResolveService:[(RendezvousBookmark*)curItem serviceID] forSender:curItem];
       mOpenActionFlag = kOpenBookmarkAction;
     }    
     else if ([curItem isKindOfClass:[BookmarkFolder class]])
@@ -518,10 +517,9 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
   while ((curItem = [itemsEnum nextObject]))
   {
     // see if it's a rendezvous item
-    id parent = [curItem parent];
-    if (![parent isKindOfClass:[BookmarkItem class]])
+    if ([curItem isKindOfClass:[RendezvousBookmark class]] && ![curItem resolved])
     {
-      [[NetworkServices sharedNetworkServices] attemptResolveService:[parent intValue] forSender:curItem];
+      [[NetworkServices sharedNetworkServices] attemptResolveService:[(RendezvousBookmark*)curItem serviceID] forSender:curItem];
       mOpenActionFlag = kOpenInNewTabAction;
     }
     else
@@ -548,10 +546,9 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
   while ((curItem = [itemsEnum nextObject]))
   {
     // see if it's a rendezvous item (this won't open in the new window, because we suck)
-    id parent = [curItem parent];
-    if (![parent isKindOfClass:[BookmarkItem class]])
+    if ([curItem isKindOfClass:[RendezvousBookmark class]] && ![curItem resolved])
     {
-      [[NetworkServices sharedNetworkServices] attemptResolveService:[parent intValue] forSender:curItem];
+      [[NetworkServices sharedNetworkServices] attemptResolveService:[(RendezvousBookmark*)curItem serviceID] forSender:curItem];
       mOpenActionFlag = kOpenInNewTabAction;
     }
     else
@@ -586,10 +583,9 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
   while ((curItem = [itemsEnum nextObject]))
   {
     // see if it's a rendezvous item
-    id parent = [curItem parent];
-    if (![parent isKindOfClass:[BookmarkItem class]])
+    if ([curItem isKindOfClass:[RendezvousBookmark class]] && ![curItem resolved])
     {
-      [[NetworkServices sharedNetworkServices] attemptResolveService:[parent intValue] forSender:curItem];
+      [[NetworkServices sharedNetworkServices] attemptResolveService:[(RendezvousBookmark*)curItem serviceID] forSender:curItem];
       mOpenActionFlag = kOpenInNewWindowAction;
     }
     else
@@ -1511,10 +1507,10 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
   if (mBookmarkUpdatesDisabled)
     return;
 
-  if (!item)
+  if (!item || (item == mActiveRootCollection))
     [mBookmarksOutlineView reloadData];
   else
-    [mBookmarksOutlineView reloadItem: item reloadChildren: aReloadChildren];
+    [mBookmarksOutlineView reloadItem:item reloadChildren:aReloadChildren];
 }
 
 - (int)numberOfSelectedRows
@@ -1785,17 +1781,22 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
     return;
   NSDictionary *dict = [note userInfo];
   id aClient = [dict objectForKey:NetworkServicesClientKey];
-  if ([aClient isKindOfClass:[Bookmark class]]) {
-    switch (mOpenActionFlag) {
+  if ([aClient isKindOfClass:[Bookmark class]])
+  {
+    switch (mOpenActionFlag)
+    {
       case (kOpenBookmarkAction):
-        [[NSRunLoop currentRunLoop] performSelector:@selector(openBookmark:) target:self argument:aClient order:10 modes:[NSArray arrayWithObject:NSDefaultRunLoopMode]];
+        [self performSelector:@selector(openBookmark:) withObject:aClient afterDelay:0];
         break;
+
       case (kOpenInNewTabAction):
-        [[NSRunLoop currentRunLoop] performSelector:@selector(openBookmarkInNewTab:) target:self argument:aClient order:10 modes:[NSArray arrayWithObject:NSDefaultRunLoopMode]];
+        [self performSelector:@selector(openBookmarkInNewTab:) withObject:aClient afterDelay:0];
         break;
+
       case (kOpenInNewWindowAction):
-        [[NSRunLoop currentRunLoop] performSelector:@selector(openBookmarkInNewWindow:) target:self argument:aClient order:10 modes:[NSArray arrayWithObject:NSDefaultRunLoopMode]];
+        [self performSelector:@selector(openBookmarkInNewWindow:) withObject:aClient afterDelay:0];
         break;
+
       default:
         break;
     }
@@ -1853,8 +1854,18 @@ static const int kDisabledQuicksearchPopupItemTag = 9999;
 
 - (void)bookmarkChanged:(NSNotification *)note
 {
-  // XXX look at change flags
-  [self reloadDataForItem:[note object] reloadChildren:NO];
+  const unsigned int kVisibleAttributeChangedFlags = (kBookmarkItemTitleChangedMask |
+                                                      kBookmarkItemURLChangedMask |
+                                                      kBookmarkItemKeywordChangedMask |
+                                                      kBookmarkItemDescriptionChangedMask |
+                                                      kBookmarkItemLastVisitChangedMask | 
+                                                      kBookmarkItemStatusChangedMask);
+
+  BOOL reloadItem     = [BookmarkItem bookmarkChangedNotificationUserInfo:[note userInfo] containsFlags:kVisibleAttributeChangedFlags];
+  BOOL reloadChildren = [BookmarkItem bookmarkChangedNotificationUserInfo:[note userInfo] containsFlags:kBookmarkItemChildrenChangedMask];
+
+  if (reloadItem || reloadChildren)
+    [self reloadDataForItem:[note object] reloadChildren:reloadChildren];
 }
 
 - (void)bookmarksViewDidMoveToWindow:(NSWindow*)inWindow

camino/src/bookmarks/KindaSmartFolderManager.h

@@ -54,7 +54,6 @@
   BookmarkFolder*       mAddressBookFolder;
   BookmarkFolder*       mRendezvousFolder;
   AddressBookManager*   mAddressBookManager;
-  unsigned              mFewestVisits;
 }
 
 -(id)initWithBookmarkManager:(BookmarkManager *)manager;

camino/src/bookmarks/KindaSmartFolderManager.mm

@@ -56,6 +56,8 @@
 -(void)rebuildTop10List;
 @end
 
+const unsigned kNumTop10Items = 10;   // well, 10, duh!
+
 @implementation KindaSmartFolderManager
 
 -(id)initWithBookmarkManager:(BookmarkManager *)manager
@@ -65,12 +67,11 @@
     mTop10Folder = [[manager top10Folder] retain];
     mAddressBookFolder = [[manager addressBookFolder] retain];
     mRendezvousFolder = [[manager rendezvousFolder] retain];
-    mFewestVisits = 0;
+
     // client notifications
     NSNotificationCenter *nc = [NSNotificationCenter defaultCenter];
     [nc addObserver:self selector:@selector(bookmarkRemoved:) name:BookmarkFolderDeletionNotification object:nil];
     [nc addObserver:self selector:@selector(bookmarkChanged:) name:BookmarkItemChangedNotification object:nil];
-    [nc addObserver:self selector:@selector(bookmarkChanged:) name:BookmarkIconChangedNotification object:nil];
   }
   return self;
 }
@@ -94,15 +95,11 @@
     [nc addObserver:self selector:@selector(availableServicesChanged:) name:NetworkServicesAvailableServicesChanged object:nil];
     [nc addObserver:self selector:@selector(serviceResolved:) name:NetworkServicesResolutionSuccess object:nil];
   }
+
   if (mAddressBookFolder)
     [self setupAddressBook];
-  // get top 10 list started
-  NSArray *bookmarkArray = [[manager rootBookmarks] allChildBookmarks];
-  unsigned i, j = [bookmarkArray count];
-  for (i=0; i < j; i++) {
-    Bookmark *aBookmark = [bookmarkArray objectAtIndex:i];
-    [self checkForNewTop10:aBookmark];
-  }
+
+  [self rebuildTop10List];
 }
 
 -(void) setupAddressBook
@@ -118,56 +115,110 @@
 -(void)rebuildTop10List
 {
   unsigned i, count = [mTop10Folder count];
-  for (i=0;i < count;i++)
+  for (i = 0; i < count; i++)
     [mTop10Folder deleteFromSmartFolderChildAtIndex:0];
-  mFewestVisits = 0;
-  // get top 10 list started
-  BookmarkManager *manager = [BookmarkManager sharedBookmarkManager];
-  NSArray *bookmarkArray = [[manager rootBookmarks] allChildBookmarks];
-  count = [bookmarkArray count];
-  for (i=0; i < count; i++) {
-    Bookmark *aBookmark = [bookmarkArray objectAtIndex:i];
-    [self checkForNewTop10:aBookmark];
-  }
-}
 
+  BookmarkManager *manager = [BookmarkManager sharedBookmarkManager];
+  BookmarkItem* curItem;
+
+  // We don't use rootBookmarks, because that will include the top10 folder,
+  // so we do the menu and toolbar folders manually. This also allows us to
+  // skip Rendezvous and Address Book folders, for which we don't store
+  // visit counts anyway. However, we will skip any other custom bookmark
+  // container folders that the user has created.
+  NSEnumerator* bookmarksEnum = [[manager bookmarkMenuFolder] objectEnumerator];
+  while ((curItem = [bookmarksEnum nextObject]))
+  {
+    if ([curItem isKindOfClass:[Bookmark class]])
+      [self checkForNewTop10:curItem];
+  }
+
+  bookmarksEnum = [[manager toolbarFolder] objectEnumerator];
+  while ((curItem = [bookmarksEnum nextObject]))
+  {
+    if ([curItem isKindOfClass:[Bookmark class]])
+      [self checkForNewTop10:curItem];
+  }
+  
+  // This is somewhat broken. -checkForNewTop10 doesn't insert based on visit count,
+  // it just assumes that later added bookmarks should replace earlier ones (with
+  // the same visit count).
+  [mTop10Folder sortChildrenUsingPrimarySelector:@selector(compareVisitCount:sortDescending:)
+                              primaryReverseSort:YES
+                               secondarySelector:@selector(compareLastVisitDate:sortDescending:)
+                            secondaryReverseSort:YES
+                                        sortDeep:NO];
+}
 
 -(void)checkForNewTop10:(Bookmark *)aBookmark
 {
-  unsigned smallVisit = [aBookmark numberOfVisits];
-  if (smallVisit == 0) {
-    if ([mTop10Folder indexOfObjectIdenticalTo:aBookmark] != NSNotFound)
-      //whoops.  we just cleared the visits on a top 10 item.  rebuild from scratch.
-      [self rebuildTop10List];
+  NSMutableArray* top10ItemsArray = [mTop10Folder childArray];
+
+//  NSLog(@"checkForNewTop10 %@ (%d items)", aBookmark, [top10ItemsArray count]);
+
+  // if it's already in the list
+  unsigned curIndex   = [top10ItemsArray indexOfObjectIdenticalTo:aBookmark];
+  unsigned visitCount = [aBookmark numberOfVisits];
+
+  // if it's not in the list, and the visit count is zero, nothing to do
+  if (curIndex == NSNotFound && visitCount == 0)
     return;
+
+  // we assume the list is sorted here
+  unsigned currentMinVisits = 1;
+  if ([top10ItemsArray count] == kNumTop10Items)
+    currentMinVisits = [[top10ItemsArray lastObject] numberOfVisits];
+
+  if (curIndex != NSNotFound) // it's already in the list
+  {
+    if (visitCount < currentMinVisits)
+    {
+      // the item dropped off the list. rather than grovel for the next highest item, just rebuild the list
+      // (this could be optimized)
+      [self rebuildTop10List]; // XXX potential recursion!
+    }
+    else
+    {
+      // just resort
+      [mTop10Folder sortChildrenUsingPrimarySelector:@selector(compareVisitCount:sortDescending:)
+                                  primaryReverseSort:YES
+                                   secondarySelector:@selector(compareLastVisitDate:sortDescending:)
+                                secondaryReverseSort:YES
+                                            sortDeep:NO];
+    }
+  }
+  else if (visitCount >= currentMinVisits)
+  {
+    // enter it into the list using insertion sort. it will go before other items with the same visit
+    // count (thus maintaining the visit count/last visit sort).
+    unsigned numItems = [top10ItemsArray count];
+    int insertionIndex = -1;
+      
+    NSString* newItemURL = [aBookmark url];
+    NSNumber* reverseSort = [NSNumber numberWithBool:YES];
+    
+    // we check the entire list to look for items with a duplicate url
+    for (unsigned i = 0; i < numItems; i ++)
+    {
+      Bookmark* curChild = [top10ItemsArray objectAtIndex:i];
+      if ([newItemURL isEqualToString:[curChild url]])
+        return;
+
+      // add before the first item with the same or lower visit count
+      if (([aBookmark compareForTop10:curChild sortDescending:reverseSort] != NSOrderedDescending) && insertionIndex == -1)
+        insertionIndex = i;
+    }
+    
+    if (insertionIndex == -1 && [top10ItemsArray count] < kNumTop10Items)
+      insertionIndex = [top10ItemsArray count];
+
+    if (insertionIndex != -1)
+    {
+      [mTop10Folder insertIntoSmartFolderChild:aBookmark atIndex:insertionIndex];
+      if ([top10ItemsArray count] > kNumTop10Items)
+        [mTop10Folder deleteFromSmartFolderChildAtIndex:[top10ItemsArray count] - 1];
     }
-  if (([mTop10Folder indexOfObjectIdenticalTo:aBookmark] != NSNotFound))
-    return;
-  // cycle through list of children
-  // find item with fewest visits, mark it for destruction
-  // if the URL from the new bookmark is already present in the
-  // list, we bail out
-  NSMutableArray *childArray = [mTop10Folder childArray];
-  unsigned i, kidVisit, count = [childArray count]; //j should be 10
-  if ((count >=10) && (smallVisit < mFewestVisits))
-    return;
-  Bookmark *aKid = nil;
-  NSString *newURL = [aBookmark url];
-  int doomedKidIndex = -1;
-  for (i=0; i< count; i++) {
-    aKid = [childArray objectAtIndex:i];
-    if ([newURL isEqualToString:[aKid url]])
-      return;
-    kidVisit = [aKid numberOfVisits];
-    if (kidVisit == mFewestVisits)
-      doomedKidIndex = i;
-    else if (smallVisit > kidVisit)
-      smallVisit = kidVisit;
   }
-  if ((doomedKidIndex != -1) && (count >= 10))
-    [mTop10Folder deleteFromSmartFolderChildAtIndex:doomedKidIndex];
-  [mTop10Folder insertIntoSmartFolderChild:aBookmark];
-  mFewestVisits = smallVisit;
 }
 
 
@@ -181,6 +232,7 @@
   if (index == NSNotFound)
     [aFolder insertIntoSmartFolderChild:aBookmark];
 }
+
 //
 // if we have this item, remove it
 // 
@@ -209,11 +261,13 @@ static int SortByProtocolAndName(NSDictionary* item1, NSDictionary* item2, void
   unsigned int i, count = [mRendezvousFolder count];
   for (i = 0; i < count; i++)
     [mRendezvousFolder deleteChild:[mRendezvousFolder objectAtIndex:0]];
+
   NetworkServices *netserv = [note object];
   NSEnumerator* keysEnumerator = [netserv serviceEnumerator];
   NSMutableArray* servicesArray = [[NSMutableArray alloc] initWithCapacity:10];
   id key;
-  while ((key = [keysEnumerator nextObject])) {
+  while ((key = [keysEnumerator nextObject]))
+  {
     NSDictionary* serviceDict = [NSDictionary dictionaryWithObjectsAndKeys:
                                                     key, @"id",
                    [netserv serviceName:[key intValue]], @"name",
@@ -221,17 +275,23 @@ static int SortByProtocolAndName(NSDictionary* item1, NSDictionary* item2, void
                                                          nil];
     [servicesArray addObject:serviceDict];
   }
-  if ([servicesArray count] != 0) {
+
+  if ([servicesArray count] != 0)
+  {
     // sort on protocol, then name
     [servicesArray sortUsingFunction:SortByProtocolAndName context:NULL];
+    
+    // make bookmarks
     unsigned int numServices = [servicesArray count];
     for (i = 0; i < numServices; i ++)
     {
       NSDictionary* serviceDict = [servicesArray objectAtIndex:i];
       NSString* itemName = [[serviceDict objectForKey:@"name"] stringByAppendingString:NSLocalizedString([serviceDict objectForKey:@"protocol"], @"")];
-      Bookmark *aBookmark = [mRendezvousFolder addBookmark];
-      [aBookmark setTitle:itemName];
-      [aBookmark setParent:[serviceDict objectForKey:@"id"]];
+      
+      RendezvousBookmark* serviceBookmark = [[RendezvousBookmark alloc] initWithServiceID:[[serviceDict objectForKey:@"id"] intValue]];
+      [serviceBookmark setTitle:itemName];
+      [mRendezvousFolder appendChild:serviceBookmark];
+      [serviceBookmark release];
     }
   }
   [servicesArray release];
@@ -240,20 +300,25 @@ static int SortByProtocolAndName(NSDictionary* item1, NSDictionary* item2, void
 - (void)serviceResolved:(NSNotification *)note
 {
   NSDictionary *dict = [note userInfo];
-  id aClient = [dict objectForKey:NetworkServicesClientKey];
-  if ([aClient isKindOfClass:[Bookmark class]]) {
-    Bookmark *aKid;
-    NSEnumerator* enumerator = [[mRendezvousFolder childArray] objectEnumerator];
-    while ((aKid = [enumerator nextObject])) {
-      if (aKid == aClient)
+  id client = [dict objectForKey:NetworkServicesClientKey];
+  if ([client isKindOfClass:[Bookmark class]])
   {
-        [aClient setUrl:[dict objectForKey:NetworkServicesResolvedURLKey]];
-        [aClient setParent:mRendezvousFolder];
+    // I'm not sure why we have to check to see that the client is a child
+    // of the rendezvous folder. Maybe just see if it's a RendezvousBookmark?
+    NSEnumerator* enumerator = [[mRendezvousFolder childArray] objectEnumerator];
+    Bookmark *curChild;
+    while ((curChild = [enumerator nextObject]))
+    {
+      if (curChild == client)
+      {
+        [client setUrl:[dict objectForKey:NetworkServicesResolvedURLKey]];
+        [client setResolved:YES];
       }
     }
   }
   return;
 }
+
 - (void)serviceResolutionFailed:(NSNotification *)note
 {
   return;
@@ -261,6 +326,7 @@ static int SortByProtocolAndName(NSDictionary* item1, NSDictionary* item2, void
 
 
 #pragma mark -
+
 //
 // BookmarkClient protocol
 //
@@ -281,9 +347,11 @@ static int SortByProtocolAndName(NSDictionary* item1, NSDictionary* item2, void
 
 - (void)bookmarkChanged:(NSNotification *)note
 {
-  // XXX look at change flags
+  BOOL visitCountChanged = [BookmarkItem bookmarkChangedNotificationUserInfo:[note userInfo] containsFlags:kBookmarkItemNumVisitsChangedMask];
+  if (visitCountChanged)
+  {
     BookmarkItem *anItem = [note object];
-  if ([anItem isKindOfClass:[Bookmark class]]) {
+    if ([anItem isKindOfClass:[Bookmark class]])
       [self checkForNewTop10:anItem];
   }
 }

camino/src/browser/SiteIconProvider.mm

@@ -265,32 +265,6 @@ NeckoCacheHelper::ClearCache()
 
 #pragma mark -
 
-static nsresult
-MakeFaviconURIFromURI(const nsAString& inURIString, nsAString& outFaviconURI)
-{
-  outFaviconURI.Truncate(0);
-  
-  nsCOMPtr<nsIURI> uri;
-  nsresult rv = NS_NewURI(getter_AddRefs(uri), inURIString);
-  if (NS_FAILED(rv))
-    return rv;
-
-  // check for http/https
-  PRBool isHTTP = PR_FALSE, isHTTPS = PR_FALSE;
-  uri->SchemeIs("http", &isHTTP);
-  uri->SchemeIs("https", &isHTTPS);
-  if (!isHTTP && !isHTTPS)
-    return NS_OK;
-
-  nsCAutoString faviconURI;
-  uri->GetPrePath(faviconURI);
-  faviconURI.Append("/favicon.ico");
-
-  outFaviconURI.Assign(NS_ConvertUTF8toUCS2(faviconURI));
-  return NS_OK;
-}
-
-
 @interface SiteIconProvider(Private)
 
 - (void)addToMissedIconsCache:(NSString*)inURI withExpirationSeconds:(unsigned int)inExpSeconds;
@@ -567,7 +541,6 @@ MakeFaviconURIFromURI(const nsAString& inURIString, nsAString& outFaviconURI)
   return sIconProvider;
 }
 
-
 + (NSString*)defaultFaviconLocationStringFromURI:(NSString*)inURI
 {
   // about: urls are special
@@ -579,12 +552,26 @@ MakeFaviconURIFromURI(const nsAString& inURIString, nsAString& outFaviconURI)
   if ([inURI compare:@"file://" options:NSCaseInsensitiveSearch range:NSMakeRange(0, 7)] == NSOrderedSame)
     return @"about:local_file";
 
-  nsAutoString uriString;
-  [inURI assignTo_nsAString:uriString];
+  // we use nsIURI here, rather than NSURL, because the former does
+  // a better job with suspect urls (e.g. those containing |), and 
+  // allows us go keep the port
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), [inURI UTF8String]);
+  if (NS_FAILED(rv))
+    return @"";
 
-  nsAutoString faviconURIString;
-  MakeFaviconURIFromURI(uriString, faviconURIString);
-  return [NSString stringWith_nsAString:faviconURIString];
+  // check for http/https
+  PRBool isHTTP = PR_FALSE, isHTTPS = PR_FALSE;
+  uri->SchemeIs("http", &isHTTP);
+  uri->SchemeIs("https", &isHTTPS);
+  if (!isHTTP && !isHTTPS)
+    return @"";
+
+  nsCAutoString faviconURI;
+  uri->GetPrePath(faviconURI);
+  faviconURI.Append("/favicon.ico");
+
+  return [NSString stringWith_nsACString:faviconURI];
 }
 
 @end

camino/src/includes/ChimeraUtils.h

@@ -56,3 +56,28 @@ protected:
   NSAutoreleasePool*    mPool;
 };
 
+
+// handy utility class for timing actions
+class StActionTimer
+{
+public:
+
+  StActionTimer(NSString* inActionName)
+  : mActionName(inActionName)
+  {
+    Microseconds(&mStartTime);
+  }
+
+  ~StActionTimer()
+  {
+    UnsignedWide endTime;
+    Microseconds(&endTime);
+
+    long long actionTime = UnsignedWideToUInt64(endTime) - UnsignedWideToUInt64(mStartTime);
+    NSLog(@"%@ took %qi us", mActionName, actionTime); 
+  }
+
+protected:
+  UnsignedWide    mStartTime;
+  NSString*       mActionName;
+};