cleanup problems with bookmark toolbar and bookmark manager when launched

from odoc or gurl appleEvents. lazily setup bookmark manager to improve
new window open times. (bug 232092)

camino/src/bookmarks/BookmarkManager.h

@@ -69,6 +69,7 @@
 // Class Methods & shutdown stuff
 + (void)startBookmarksManager:(RunLoopMessenger *)mainThreadRunLoopMessenger;
 + (BookmarkManager*)sharedBookmarkManager;
++ (NSString*)managerStartedNotification;
 - (void)shutdown;
 
 // Getters/Setters

camino/src/bookmarks/BookmarkManager.mm

@@ -103,6 +103,11 @@ static unsigned gFirstUserCollection = 0;
   return theManager;
 }
 
++ (NSString*)managerStartedNotification
+{
+  return @"BookmarkManagerStartedNotification";
+}
+
 //
 // Init, dealloc - better get inited on background thread.
 //
@@ -110,7 +115,7 @@ static unsigned gFirstUserCollection = 0;
 {
   if ((self = [super init]))
   {
-    BookmarkFolder* root = [[BookmarkFolder alloc] init];
+BookmarkFolder* root = [[BookmarkFolder alloc] init];
     [root setParent:self];
     [root setIsRoot:YES];
     [root setTitle:NSLocalizedString(@"BookmarksRootName", @"")];
@@ -158,6 +163,7 @@ static unsigned gFirstUserCollection = 0;
     [nc addObserver:self selector:@selector(bookmarkChanged:) name:BookmarkItemChangedNotification object:nil];
     [nc addObserver:self selector:@selector(writeBookmarks:) name:WriteBookmarkNotification object:nil];
   }
+  
   return self;
 }
 
@@ -177,6 +183,11 @@ static unsigned gFirstUserCollection = 0;
   [super dealloc];
 }
 
+//
+// -delayedStartupItems
+//
+// Perform additional setup items on the main thread.
+//
 - (void)delayedStartupItems
 {
   [[NSApp delegate] setupBookmarkMenus:gBookmarksManager];
@@ -198,8 +209,9 @@ static unsigned gFirstUserCollection = 0;
       [[allBookmarks objectAtIndex:i] performSelector:@selector(refreshIcon) withObject:nil afterDelay:delay];
     }
   }
-  // make sure bookmark toolbar was built (only a concern on startup from apple event)
-  [[[[[NSApp delegate] getFrontmostBrowserWindow] windowController] bookmarkToolbar] buildButtonList];
+
+  // broadcast to everyone interested that we're loaded and ready for public consumption
+  [[NSNotificationCenter defaultCenter] postNotificationName:[BookmarkManager managerStartedNotification] object:nil];
 }
 
 - (void)shutdown;

camino/src/bookmarks/BookmarkToolbar.mm

@@ -60,7 +60,7 @@
 - (void)setButtonInsertionPoint:(id <NSDraggingInfo>)sender;
 - (NSRect)insertionRectForButton:(NSView*)aButton position:(int)aPosition;
 - (BookmarkButton*)makeNewButtonWithItem:(BookmarkItem*)aItem;
-
+- (void)managerStarted:(NSNotification*)inNotify;
 @end
 
 @implementation BookmarkToolbar
@@ -81,6 +81,12 @@
     [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];
+    
+    // register for notifications of when the BM manager starts up. Since it does it on a separate thread,
+    // it can be created after we are and if we don't update ourselves, the bar will be blank. This
+    // happens most notably when the app is launched with a 'odoc' or 'GURL' appleEvent.
+    [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(managerStarted:)
+        name:[BookmarkManager managerStartedNotification] object:nil];
   }
   return self;
 }
@@ -92,6 +98,16 @@
   [super dealloc];
 }
 
+//
+// - managerStarted:
+//
+// Notification callback from the bookmark manager. Build the button list.
+//
+- (void)managerStarted:(NSNotification*)inNotify
+{
+  [self buildButtonList];
+}
+
 - (void)drawRect:(NSRect)aRect
 {
   if (mDrawBorder)

camino/src/bookmarks/BookmarkViewController.h

@@ -69,6 +69,7 @@
     
   IBOutlet HistoryDataSource* mHistorySource; //can swap to this for history data
 
+  BOOL mSetupComplete;                      // have we been fully initialized?
   NSMutableDictionary *mExpandedStatus;
   NSString* mCachedHref;
   BookmarkFolder *mActiveRootCollection;
@@ -110,7 +111,7 @@
 
 -(void)deleteCollection:(id)aSender;
 -(void) focus;
--(void) windowDidLoad;
+-(void) completeSetup;
 -(void) ensureBookmarks;
 
 @end

camino/src/bookmarks/BookmarkViewController.mm

@@ -77,6 +77,21 @@ const long kMinSearchPaneHeight = 80;
 
 @implementation BookmarkViewController
 
+- (id)init
+{
+  if (self = [super init]) {
+    mCachedHref = nil;
+    mRootBookmarks = nil;
+    mActiveRootCollection = nil;
+    mExpandedStatus = nil;
+    mSearchResultArray = nil;
+    mOpenActionFlag = 0;
+    
+    // wait for |-completeSetup| to be called to lazily complete our setup
+    mSetupComplete = NO;
+  }
+}
+
 - (void)dealloc
 {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
@@ -88,12 +103,20 @@ const long kMinSearchPaneHeight = 80;
   [super dealloc];
 }
 
-
-- (void)windowDidLoad
+//
+// - managerStarted:
+//
+// Notification callback from the bookmark manager. Reload all the table data, but
+// only if we think we've fully initalized ourselves.
+//
+- (void)managerStarted:(NSNotification*)inNotify
+{
+  if (mSetupComplete)
+    [self ensureBookmarks];
+}
+
+- (void)completeSetup
 {
-  mCachedHref = nil;
-  mRootBookmarks = nil;
-  mOpenActionFlag = 0;
   [self setSearchResultArray:[NSMutableArray array]];
   // the standard table item doesn't handle text and icons. Replace it
   // with a custom cell that does.
@@ -131,6 +154,11 @@ const long kMinSearchPaneHeight = 80;
   [nc addObserver:self selector:@selector(bookmarkChanged:) name:BookmarkIconChangedNotification object:nil];
   [nc addObserver:self selector:@selector(serviceResolved:) name:NetworkServicesResolutionSuccess object:nil];
 
+  // register for notifications of when the BM manager starts up. Since it does it on a separate thread,
+  // it can be created after we are and if we don't update ourselves, the bar will be blank. This
+  // happens most notably when the app is launched with a 'odoc' or 'GURL' appleEvent.
+  [nc addObserver:self selector:@selector(managerStarted:) name:[BookmarkManager managerStartedNotification] object:nil];
+
   // register for dragged types
   [mContainerPane registerForDraggedTypes:[NSArray arrayWithObjects:@"MozBookmarkType",@"MozURLType", NSURLPboardType, NSStringPboardType, nil]];
 
@@ -145,12 +173,25 @@ const long kMinSearchPaneHeight = 80;
   [mItemPane setAutosaveTableColumns:YES];
   [mSearchPane setAutosaveTableColumns:YES];
   [mContainerPane setAutosaveTableColumns:YES];
+  
+  mSetupComplete = YES;
 }
 
+//
+// ensureBookmarks
+//
+// Setup the connections for the bookmark manager and tell the tables to reload their
+// data. This routine may be called more than once safely. Note that if the bookmark manager
+// has not yet been fully initialized by the time we get here, bail until we hear back later.
+//
 -(void)ensureBookmarks
 {
   if (!mRootBookmarks) {
-    mRootBookmarks = [[[BookmarkManager sharedBookmarkManager] rootBookmarks] retain];
+    BookmarkFolder* manager = [[BookmarkManager sharedBookmarkManager] rootBookmarks];
+    if (![manager count])     // not initialized yet, try again later (from start notifiation)
+      return;
+
+    mRootBookmarks = [manager retain];
     [mContainerPane setTarget:self];
     [mContainerPane setDeleteAction:@selector(deleteCollection:)];
     [mContainerPane reloadData];
@@ -161,9 +202,6 @@ const long kMinSearchPaneHeight = 80;
     [mSearchPane setTarget: self];
     [mSearchPane setDoubleAction: @selector(openBookmark:)];
     [self restoreFolderExpandedStates];
-    [mItemPane setAutosaveTableColumns:YES];
-    [mSearchPane setAutosaveTableColumns:YES];
-    [mContainerPane setAutosaveTableColumns:YES];
   }
 }
 

camino/src/browser/BrowserWindowController.h

@@ -139,6 +139,7 @@ typedef enum
   NSToolbarItem*                mBookmarkToolbarItem;
 
   BOOL mInitialized;
+  BOOL mBookmarkViewControllerInitialized;       // have we fully set up the bookmarks view, done lazily
   NSString* mPendingURL;
   NSString* mPendingReferrer;
   BOOL mPendingActivate;

camino/src/browser/BrowserWindowController.mm

@@ -270,6 +270,7 @@ static NSArray* sToolbarDefaults = nil;
     mBookmarkToolbarItem = nil;
     mSidebarToolbarItem = nil;
     mSavedTitle = nil;
+    mBookmarkViewControllerInitialized = NO;
   
     // register for services
     NSArray* sendTypes = [NSArray arrayWithObjects:NSStringPboardType, nil];
@@ -456,6 +457,12 @@ static NSArray* sToolbarDefaults = nil;
   [super dealloc];
 }
 
+//
+// windowDidLoad
+//
+// setup all the things we can't do in the nib. Note that we defer the setup of
+// the bookmarks view until the user actually displays it the first time.
+//
 - (void)windowDidLoad
 {
     [super windowDidLoad];
@@ -618,9 +625,6 @@ static NSArray* sToolbarDefaults = nil;
         [[self window] setFrameTopLeftPoint:topLeft];
       }
     }
-    
-    // let the in-window bookmark controller finish up some initialization
-    [mBookmarkViewController windowDidLoad];
 }
 
 - (NSSize)windowWillResize:(NSWindow *)sender toSize:(NSSize)proposedFrameSize
@@ -2659,6 +2663,12 @@ static NSArray* sToolbarDefaults = nil;
 //
 - (void)toggleBookmarkManager:(id)sender
 {
+  // lazily init the setup of the view's controller
+  if (!mBookmarkViewControllerInitialized) {
+    [mBookmarkViewController completeSetup];
+    mBookmarkViewControllerInitialized = YES;
+  }
+  
   // deactivate any gecko view that might think it has focus
   if ([self isResponderGeckoView:[[self window] firstResponder]]) {
     CHBrowserView* browserView = [mBrowserView getBrowserView];