From 0323baf298763af020217eb11b0e86ac7a0e260a Mon Sep 17 00:00:00 2001 From: "jianli@chromium.org" Date: Thu, 20 Jun 2013 17:20:52 +0000 Subject: [PATCH] [Mac] Fix the problem that a popup could be closed while it is still in animation. This occurs when the test code creates a popup and close the collection immediately. The fix is to close the popup without animation for this case. All the popup animations are coordinated by the popup collection. Added DCHECKs in the popup controller to guard the places that might trigger the 2nd animation while the previous animation has not ended. BUG=249131 TEST=reenable the disabled test R=dewittj@chromium.org, rsesek@chromium.org Review URL: https://codereview.chromium.org/17468007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207470 0039d316-1c4b-4281-b951-d872f2087c98 --- ui/message_center/cocoa/popup_collection.mm | 9 ++++++++- ui/message_center/cocoa/popup_collection_unittest.mm | 4 +--- ui/message_center/cocoa/popup_controller.mm | 3 +++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ui/message_center/cocoa/popup_collection.mm b/ui/message_center/cocoa/popup_collection.mm index 4b81ac336a63..8dd1b1908730 100644 --- a/ui/message_center/cocoa/popup_collection.mm +++ b/ui/message_center/cocoa/popup_collection.mm @@ -228,7 +228,14 @@ class PopupCollectionObserver : public message_center::MessageCenterObserver { } - (void)removeAllNotifications { - [popups_ makeObjectsPerformSelector:@selector(closeWithAnimation)]; + // In rare cases, the popup collection would be gone while an animation is + // still playing. For exmaple, the test code could show a new notification + // and dispose the collection immediately. Close the popup without animation + // when this is the case. + if ([self isAnimating]) + [popups_ makeObjectsPerformSelector:@selector(close)]; + else + [popups_ makeObjectsPerformSelector:@selector(closeWithAnimation)]; [popups_ makeObjectsPerformSelector:@selector(markPopupCollectionGone)]; [popups_ removeAllObjects]; } diff --git a/ui/message_center/cocoa/popup_collection_unittest.mm b/ui/message_center/cocoa/popup_collection_unittest.mm index fada0efb2fec..2375d03365e7 100644 --- a/ui/message_center/cocoa/popup_collection_unittest.mm +++ b/ui/message_center/cocoa/popup_collection_unittest.mm @@ -311,9 +311,7 @@ TEST_F(PopupCollectionTest, UpdateIconAndBody) { EXPECT_EQ("3", [[popups objectAtIndex:2] notificationID]); } -// Test sometimes timesout. See http://crbug.com/249131 -TEST_F(PopupCollectionTest, - DISABLED_CloseCollectionBeforeNewPopupAnimationEnds) { +TEST_F(PopupCollectionTest, CloseCollectionBeforeNewPopupAnimationEnds) { // Add a notification and don't wait for the animation to finish. scoped_ptr notification; notification.reset(new message_center::Notification( diff --git a/ui/message_center/cocoa/popup_controller.mm b/ui/message_center/cocoa/popup_controller.mm index cadeae75bd85..496e08ac20b9 100644 --- a/ui/message_center/cocoa/popup_controller.mm +++ b/ui/message_center/cocoa/popup_controller.mm @@ -213,6 +213,7 @@ enum { NSViewAnimationEndFrameKey : [NSValue valueWithRect:newBounds], NSViewAnimationEffectKey : NSViewAnimationFadeInEffect }; + DCHECK(!boundsAnimation_); boundsAnimation_.reset([[NSViewAnimation alloc] initWithViewAnimations:[NSArray arrayWithObject:animationDict]]); [boundsAnimation_ setDuration:[popupCollection_ popupAnimationDuration]]; @@ -237,6 +238,7 @@ enum { NSViewAnimationTargetKey : [self window], NSViewAnimationEffectKey : NSViewAnimationFadeOutEffect }; + DCHECK(!boundsAnimation_); boundsAnimation_.reset([[NSViewAnimation alloc] initWithViewAnimations:[NSArray arrayWithObject:animationDict]]); [boundsAnimation_ setDuration:[popupCollection_ popupAnimationDuration]]; @@ -261,6 +263,7 @@ enum { NSViewAnimationTargetKey : [self window], NSViewAnimationEndFrameKey : [NSValue valueWithRect:newBounds] }; + DCHECK(!boundsAnimation_); boundsAnimation_.reset([[NSViewAnimation alloc] initWithViewAnimations:[NSArray arrayWithObject:animationDict]]); [boundsAnimation_ setDuration:[popupCollection_ popupAnimationDuration]]; -- 2.11.4.GIT