Skip to content

Commit

Permalink
[Mac] Fix the problem that a popup could be closed while it is still …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
jianli@chromium.org committed Jun 20, 2013
1 parent 8b5f65c commit 0323baf
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 4 deletions.
9 changes: 8 additions & 1 deletion ui/message_center/cocoa/popup_collection.mm
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,14 @@ - (void)removeNotification:(const std::string&)notificationID {
}

- (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];
}
Expand Down
4 changes: 1 addition & 3 deletions ui/message_center/cocoa/popup_collection_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,7 @@ void WaitForAnimationEnded() {
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<message_center::Notification> notification;
notification.reset(new message_center::Notification(
Expand Down
3 changes: 3 additions & 0 deletions ui/message_center/cocoa/popup_controller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ - (void)showWithAnimation:(NSRect)newBounds {
NSViewAnimationEndFrameKey : [NSValue valueWithRect:newBounds],
NSViewAnimationEffectKey : NSViewAnimationFadeInEffect
};
DCHECK(!boundsAnimation_);
boundsAnimation_.reset([[NSViewAnimation alloc]
initWithViewAnimations:[NSArray arrayWithObject:animationDict]]);
[boundsAnimation_ setDuration:[popupCollection_ popupAnimationDuration]];
Expand All @@ -237,6 +238,7 @@ - (void)closeWithAnimation {
NSViewAnimationTargetKey : [self window],
NSViewAnimationEffectKey : NSViewAnimationFadeOutEffect
};
DCHECK(!boundsAnimation_);
boundsAnimation_.reset([[NSViewAnimation alloc]
initWithViewAnimations:[NSArray arrayWithObject:animationDict]]);
[boundsAnimation_ setDuration:[popupCollection_ popupAnimationDuration]];
Expand All @@ -261,6 +263,7 @@ - (void)setBounds:(NSRect)newBounds {
NSViewAnimationTargetKey : [self window],
NSViewAnimationEndFrameKey : [NSValue valueWithRect:newBounds]
};
DCHECK(!boundsAnimation_);
boundsAnimation_.reset([[NSViewAnimation alloc]
initWithViewAnimations:[NSArray arrayWithObject:animationDict]]);
[boundsAnimation_ setDuration:[popupCollection_ popupAnimationDuration]];
Expand Down

0 comments on commit 0323baf

Please sign in to comment.