Fix multi-window bookmark bubble problem.

Also fix a "flash" where the bookmark bubble opened in the wrong spot
before jumping to the correct spot (hard to see).

BookmarkBubble.xib change: NOT initially visible.

BUG=http://crbug.com/27545

TEST=see bug (it's clear).

Review URL: http://codereview.chromium.org/391057

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32030 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 36 insertions, 18 deletions

diff --git a/chrome/browser/cocoa/bookmark_bubble_controller.h b/chrome/browser/cocoa/bookmark_bubble_controller.h
index 506af6b..c8bfd0b 100644
--- a/chrome/browser/cocoa/bookmark_bubble_controller.h
+++ b/chrome/browser/cocoa/bookmark_bubble_controller.h
@@ -16,6 +16,9 @@ class BookmarkNode;
 // The bubble asks the delegate to perform an edit when needed.
 - (void)editBookmarkNode:(const BookmarkNode*)node;
 
+// The bubble tells the delegate when it will go away.
+- (void)bubbleWindowWillClose:(NSWindow*)window;
+
 @end
 
 // Controller for the bookmark bubble.  The bookmark bubble is a
@@ -58,6 +61,7 @@ class BookmarkNode;
 - (IBAction)remove:(id)sender;
 - (IBAction)cancel:(id)sender;
 - (IBAction)folderChanged:(id)sender;
+
 @end
 
 
diff --git a/chrome/browser/cocoa/bookmark_bubble_controller.mm b/chrome/browser/cocoa/bookmark_bubble_controller.mm
index 2c95fa5..6fc0c2c 100644
--- a/chrome/browser/cocoa/bookmark_bubble_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bubble_controller.mm
@@ -58,8 +58,15 @@
   [self autorelease];
 }
 
-- (void)windowDidLoad {
-  NSWindow* window = [self window];
+
+// We want this to be a child of a browser window.  addChildWindow:
+// (called from this function) will bring the window on-screen;
+// unfortunately, [NSWindowController showWindow:] will also bring it
+// on-screen (but will cause unexpected changes to the window's
+// position).  We cannot have an addChildWindow: and a subsequent
+// showWindow:. Thus, we have our own version.
+- (void)showWindow:(id)sender {
+  NSWindow* window = [self window];  // completes nib load
   NSPoint origin = [parentWindow_ convertBaseToScreen:topLeftForBubble_];
   origin.y -= NSHeight([window frame]);
   [window setFrameOrigin:origin];
@@ -73,10 +80,13 @@
   }
 
   [self fillInFolderList];
+
+  [[self window] makeKeyAndOrderFront:self];
 }
 
 - (void)close {
   [parentWindow_ removeChildWindow:[self window]];
+  [delegate_ bubbleWindowWillClose:[self window]];
   [super close];
 }
 
@@ -134,6 +144,12 @@
 - (void)windowDidResignKey:(NSNotification*)notification {
   DCHECK_EQ([notification object], [self window]);
 
+  // We must tell our delegate our window is gone RIGHT NOW.  The
+  // performSelector: call below triggers a close but it might get
+  // processed after a request to show a new bubble (e.g. in another
+  // window).
+  [delegate_ bubbleWindowWillClose:[self window]];
+
   // Can't call close from within a window delegate method. Call close for the
   // next time through the event loop.
   [self performSelector:@selector(ok:) withObject:self afterDelay:0];
diff --git a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm
index ba3dd52..d63aaa3 100644
--- a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm
@@ -36,11 +36,19 @@
   edits_++;
 }
 
+// Tell us (a delegate) which controller it will "own".  This sets up
+// the test classes (e.g. simulates what Chromium would do after
+// creating a BookmarkBubbleController).
 - (void)setWindowController:(NSWindowController *)controller {
   window_ = static_cast<InfoBubbleWindow*>([controller window]);
-  EXPECT_TRUE([window_ isKindOfClass:[InfoBubbleWindow class]]);
+  [controller showWindow:self];
 }
 
+// The bubble tells the delegate when it will go away.
+- (void)bubbleWindowWillClose:(NSWindow*)window {
+  // empty
+ }
+
 - (BOOL)isWindowClosing {
   return [window_ isClosing];
 }
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index 1eb90c8..e8a77cb2 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -1172,27 +1172,17 @@ willPositionSheet:(NSWindow*)sheet
                                                      model:model
                                                       node:node
                                          alreadyBookmarked:alreadyBookmarked];
-    NSWindow* bookmarkBubbleWindow = [bookmarkBubbleController_ window];
-    NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
-    [nc addObserver:self
-           selector:@selector(bubbleWindowWillClose:)
-               name:NSWindowWillCloseNotification
-             object:bookmarkBubbleWindow];
     [bookmarkBubbleController_ showWindow:self];
   }
 }
 
-// Notification sent when our bubble window is closed.
-- (void)bubbleWindowWillClose:(NSNotification*)notification {
-  DCHECK([[notification object] windowController] == bookmarkBubbleController_);
-  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
-  [nc removeObserver:self
-                name:NSWindowWillCloseNotification
-              object:[bookmarkBubbleController_ window]];
-  bookmarkBubbleController_ = nil;
+// Implement BookmarkBubbleControllerDelegate.
+- (void)bubbleWindowWillClose:(NSWindow*)window {
+  if (window == [bookmarkBubbleController_ window])
+    bookmarkBubbleController_ = nil;
 }
 
-// Implement BookmarkBubbleControllerDelegate
+// Implement BookmarkBubbleControllerDelegate.
 - (void)editBookmarkNode:(const BookmarkNode*)node {
   // A BookmarkEditorController is a sheet that owns itself, and
   // deallocates itself when closed.