Makes InfoBubbleWindow close animations work correctly with regards to

activating other windows post-close correctly.

BUG=27711
TEST=See repro steps in bug.

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

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

4 files changed, 87 insertions, 46 deletions

diff --git a/chrome/browser/cocoa/bookmark_bubble_controller.mm b/chrome/browser/cocoa/bookmark_bubble_controller.mm
index 29ca864..2c95fa5 100644
--- a/chrome/browser/cocoa/bookmark_bubble_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bubble_controller.mm
@@ -82,9 +82,8 @@
 
 // Shows the bookmark editor sheet for more advanced editing.
 - (void)showEditor {
-  [self updateBookmarkNode];
+  [self ok:nil];
   [delegate_ editBookmarkNode:node_];
-  [self close];
 }
 
 - (IBAction)edit:(id)sender {
diff --git a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm
index 2295d48..ba3dd52 100644
--- a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm
@@ -9,30 +9,24 @@
 #import "chrome/browser/cocoa/bookmark_bubble_controller.h"
 #include "chrome/browser/cocoa/browser_test_helper.h"
 #import "chrome/browser/cocoa/cocoa_test_helper.h"
+#import "chrome/browser/cocoa/info_bubble_window.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
 @interface BBDelegate : NSObject<BookmarkBubbleControllerDelegate> {
  @private
-  BOOL windowClosed_;
+  InfoBubbleWindow* window_;
   int edits_;
 }
 
 @property (readonly) int edits;
-@property (readonly) BOOL windowClosed;
+@property (readonly, getter=isWindowClosing) BOOL windowClosing;
 
 @end
 
 @implementation BBDelegate
 
 @synthesize edits = edits_;
-@synthesize windowClosed = windowClosed_;
-
-- (void)dealloc {
-  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
-  [nc removeObserver:self];
-  [super dealloc];
-}
 
 - (NSPoint)topLeftForBubble {
   return NSMakePoint(10, 300);
@@ -43,20 +37,14 @@
 }
 
 - (void)setWindowController:(NSWindowController *)controller {
-  NSNotificationCenter* nc = [NSNotificationCenter defaultCenter];
-  [nc removeObserver:self];
-  if (controller) {
-    [nc addObserver:self
-           selector:@selector(windowWillClose:)
-               name:NSWindowWillCloseNotification
-             object:[controller window]];
-  }
-  windowClosed_ = NO;
+  window_ = static_cast<InfoBubbleWindow*>([controller window]);
+  EXPECT_TRUE([window_ isKindOfClass:[InfoBubbleWindow class]]);
 }
 
-- (void)windowWillClose:(NSNotification*)notification {
-  windowClosed_ = YES;
+- (BOOL)isWindowClosing {
+  return [window_ isClosing];
 }
+
 @end
 
 namespace {
@@ -163,10 +151,10 @@ TEST_F(BookmarkBubbleControllerTest, TestEdit) {
   EXPECT_TRUE(controller);
 
   EXPECT_EQ([delegate_ edits], 0);
-  EXPECT_FALSE([delegate_ windowClosed]);
+  EXPECT_FALSE([delegate_ isWindowClosing]);
   [controller edit:controller];
   EXPECT_EQ([delegate_ edits], 1);
-  EXPECT_TRUE([delegate_ windowClosed]);
+  EXPECT_TRUE([delegate_ isWindowClosing]);
 }
 
 // CallClose; bubble gets closed.
@@ -180,10 +168,10 @@ TEST_F(BookmarkBubbleControllerTest, TestClose) {
 
   BookmarkBubbleController* controller = ControllerForNode(node);
   EXPECT_TRUE(controller);
-  EXPECT_FALSE([delegate_ windowClosed]);
+  EXPECT_FALSE([delegate_ isWindowClosing]);
   [controller ok:controller];
   EXPECT_EQ([delegate_ edits], 0);
-  EXPECT_TRUE([delegate_ windowClosed]);
+  EXPECT_TRUE([delegate_ isWindowClosing]);
 }
 
 // User changes title and parent folder in the UI
@@ -281,7 +269,7 @@ TEST_F(BookmarkBubbleControllerTest, TestRemove) {
 
   [controller remove:controller];
   EXPECT_FALSE(model->IsBookmarked(gurl));
-  EXPECT_TRUE([delegate_ windowClosed]);
+  EXPECT_TRUE([delegate_ isWindowClosing]);
 }
 
 // Confirm picking "choose another folder" caused edit: to be called.
diff --git a/chrome/browser/cocoa/info_bubble_window.h b/chrome/browser/cocoa/info_bubble_window.h
index eea956b..925ee98 100644
--- a/chrome/browser/cocoa/info_bubble_window.h
+++ b/chrome/browser/cocoa/info_bubble_window.h
@@ -11,4 +11,15 @@
   // Is self in the process of closing.
   BOOL closing_;
 }
+
+@end
+
+// Methods to only be used by unittests.
+@interface InfoBubbleWindow (UnitTest)
+
+// Returns YES if the window is in the process of closing.
+// Can't use "windowWillClose" notification because that will be sent
+// after the closing animation has completed.
+- (BOOL)isClosing;
+
 @end
diff --git a/chrome/browser/cocoa/info_bubble_window.mm b/chrome/browser/cocoa/info_bubble_window.mm
index 9cfb432..8d066fa 100644
--- a/chrome/browser/cocoa/info_bubble_window.mm
+++ b/chrome/browser/cocoa/info_bubble_window.mm
@@ -5,6 +5,7 @@
 #import "chrome/browser/cocoa/info_bubble_window.h"
 
 #include "base/logging.h"
+#include "base/scoped_nsobject.h"
 #import "third_party/GTM/AppKit/GTMNSAnimation+Duration.h"
 
 namespace {
@@ -13,6 +14,41 @@ const NSTimeInterval kOrderInAnimationDuration = 0.3;
 const NSTimeInterval kOrderOutAnimationDuration = 0.15;
 }
 
+@interface InfoBubbleWindow (Private)
+- (void)finishCloseAfterAnimation;
+@end
+
+// A delegate object for watching the alphaValue animation on InfoBubbleWindows.
+// An InfoBubbleWindow instance cannot be the delegate for its own animation
+// because CAAnimations retain their delegates, and since the InfoBubbleWindow
+// retains its animations a retain loop would be formed.
+@interface InfoBubbleWindowCloser : NSObject {
+ @private
+  InfoBubbleWindow* window_;  // Weak. Window to close.
+}
+- (id)initWithWindow:(InfoBubbleWindow*)window;
+@end
+
+@implementation InfoBubbleWindowCloser
+
+- (id)initWithWindow:(InfoBubbleWindow*)window {
+  if ((self = [super init])) {
+    window_ = window;
+  }
+  return self;
+}
+
+// Callback for the alpha animation. Closes window_ if appropriate.
+- (void)animationDidStop:(CAAnimation*)anim finished:(BOOL)flag {
+  // When alpha reaches zero, close window_.
+  if ([window_ alphaValue] == 0.0) {
+    [window_ finishCloseAfterAnimation];
+  }
+}
+
+@end
+
+
 @implementation InfoBubbleWindow
 
 - (id)initWithContentRect:(NSRect)contentRect
@@ -37,10 +73,13 @@ const NSTimeInterval kOrderOutAnimationDuration = 0.15;
     // Notice that only the alphaValue Animation is replaced in case
     // superclasses set up animations.
     CAAnimation* alphaAnimation = [CABasicAnimation animation];
-    [alphaAnimation setDelegate:self];
+    scoped_nsobject<InfoBubbleWindowCloser> delegate(
+        [[InfoBubbleWindowCloser alloc] initWithWindow:self]);
+    [alphaAnimation setDelegate:delegate];
     NSMutableDictionary* animations =
         [NSMutableDictionary dictionaryWithDictionary:[self animations]];
     [animations setObject:alphaAnimation forKey:@"alphaValue"];
+    [self setAnimations:animations];
   }
   return self;
 }
@@ -54,7 +93,26 @@ const NSTimeInterval kOrderOutAnimationDuration = 0.15;
   return YES;
 }
 
-// Adds animation for info bubbles being ordered to the front and ordered out.
+- (void)close {
+  // Block the window from receiving events while it fades out.
+  closing_ = YES;
+  // Apply animations to hide self.
+  [NSAnimationContext beginGrouping];
+  [[NSAnimationContext currentContext]
+      gtm_setDuration:kOrderOutAnimationDuration];
+  [[self animator] setAlphaValue:0.0];
+  [NSAnimationContext endGrouping];
+}
+
+// Called by InfoBubbleWindowCloser when the window is to be really closed
+// after the fading animation is complete.
+- (void)finishCloseAfterAnimation {
+  if (closing_) {
+    [super close];
+  }
+}
+
+// Adds animation for info bubbles being ordered to the front.
 - (void)orderWindow:(NSWindowOrderingMode)orderingMode
          relativeTo:(NSInteger)otherWindowNumber {
   // According to the documentation '0' is the otherWindowNumber when the window
@@ -77,16 +135,6 @@ const NSTimeInterval kOrderOutAnimationDuration = 0.15;
     [[self animator] setAlphaValue:1.0];
     [[self animator] setFrame:frame display:YES];
     [NSAnimationContext endGrouping];
-  } else if (orderingMode == NSWindowOut) {
-    // Flag self as closing to block events while animation is occurring.
-    closing_ = YES;
-
-    // Apply animations to hide self.
-    [NSAnimationContext beginGrouping];
-    [[NSAnimationContext currentContext]
-        gtm_setDuration:kOrderOutAnimationDuration];
-    [[self animator] setAlphaValue:0.0];
-    [NSAnimationContext endGrouping];
   } else {
     [super orderWindow:orderingMode relativeTo:otherWindowNumber];
   }
@@ -100,12 +148,7 @@ const NSTimeInterval kOrderOutAnimationDuration = 0.15;
   }
 }
 
-// Callback for the alpha animation set up in designated initializer.
-- (void)animationDidStop:(CAAnimation*)anim finished:(BOOL)flag {
-  // When alpha reaches zero, close self.
-  if ([self alphaValue] == 0.0) {
-    [self close];
-  }
+- (BOOL)isClosing {
+  return closing_;
 }
-
 @end