From 6ee7d1161d55d6cb3f21e9bf702ae7f726d50fb8 Mon Sep 17 00:00:00 2001 From: "viettrungluu@chromium.org" Date: Mon, 30 Nov 2009 15:38:32 +0000 Subject: Mac: improve apparent z-order problems when switching Spaces. Remove the status bubble as child window when hidden, rather than just setting its opacity to 0. I'm not sure about the effects, if any, of doing this rather than destroying the window completely. This doesn't eliminate z-order problems with Spaces, but should improve it considerably. This may also ameliorate the problems with moving windows between Spaces using Expose. BUG=28107, 24956 TEST=Create a bunch of windows; get the status bubble to appear in the active (key) window and then get it to disappear; switch to another spaces and back; make sure the active window is on top; repeat. Review URL: http://codereview.chromium.org/434120 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33287 0039d316-1c4b-4281-b951-d872f2087c98 --- .../cocoa/chrome_event_processing_window.mm | 2 +- chrome/browser/cocoa/status_bubble_mac.h | 10 ++++++- chrome/browser/cocoa/status_bubble_mac.mm | 33 +++++++++++++++------- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/chrome/browser/cocoa/chrome_event_processing_window.mm b/chrome/browser/cocoa/chrome_event_processing_window.mm index 99ed6af..da23e7e 100644 --- a/chrome/browser/cocoa/chrome_event_processing_window.mm +++ b/chrome/browser/cocoa/chrome_event_processing_window.mm @@ -69,7 +69,7 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int); - (BOOL)redispatchEvent:(NSEvent*)event { DCHECK(event); - DCHECK([event window] == self); + DCHECK_EQ([event window], self); eventHandled_ = YES; redispatchingEvent_ = YES; [NSApp sendEvent:event]; diff --git a/chrome/browser/cocoa/status_bubble_mac.h b/chrome/browser/cocoa/status_bubble_mac.h index 8d33f04..e43b81f 100644 --- a/chrome/browser/cocoa/status_bubble_mac.h +++ b/chrome/browser/cocoa/status_bubble_mac.h @@ -63,9 +63,17 @@ class StatusBubbleMac : public StatusBubble { // it does.) void Create(); - // Attaches the status bubble window to its parent window. + // Attaches the status bubble window to its parent window. Safe to call even + // when already attached. void Attach(); + // Detaches the status bubble window from its parent window. + void Detach(); + + // Is the status bubble attached to the browser window? It should be attached + // when shown and during any fades, but should be detached when hidden. + bool is_attached() { return [window_ parentWindow] != nil; } + // Begins fading the status bubble window in or out depending on the value // of |show|. This must be called from the appropriate fade state, // kBubbleShowingFadeIn or kBubbleHidingFadeOut, or from the appropriate diff --git a/chrome/browser/cocoa/status_bubble_mac.mm b/chrome/browser/cocoa/status_bubble_mac.mm index 55b57df..daea726f 100644 --- a/chrome/browser/cocoa/status_bubble_mac.mm +++ b/chrome/browser/cocoa/status_bubble_mac.mm @@ -107,7 +107,7 @@ StatusBubbleMac::~StatusBubbleMac() { if (window_) { [[[window_ animationForKey:kFadeAnimationKey] delegate] invalidate]; - [parent_ removeChildWindow:window_]; + Detach(); [window_ release]; window_ = nil; } @@ -333,24 +333,31 @@ void StatusBubbleMac::Create() { [animation_dictionary setObject:animation forKey:kFadeAnimationKey]; [window_ setAnimations:animation_dictionary]; - Attach(); + // Don't |Attach()| since we don't know the appropriate state; let the + // |SetState()| call do that. [view setCornerFlags:kRoundedTopRightCorner]; MouseMoved(gfx::Point(), false); } void StatusBubbleMac::Attach() { - // If the parent window is offscreen when the child is added, the child will - // never be displayed, even when the parent moves on-screen. This method - // may be called several times during the process of creating or showing a - // status bubble to attach the bubble to its parent window. - if (![window_ parentWindow] && [parent_ isVisible]) + // This method may be called several times during the process of creating or + // showing a status bubble to attach the bubble to its parent window. + if (!is_attached()) [parent_ addChildWindow:window_ ordered:NSWindowAbove]; } +void StatusBubbleMac::Detach() { + // This method may be called several times in the process of hiding or + // destroying a status bubble. + if (is_attached()) + [parent_ removeChildWindow:window_]; +} + void StatusBubbleMac::AnimationDidStop(CAAnimation* animation, bool finished) { DCHECK([NSThread isMainThread]); DCHECK(state_ == kBubbleShowingFadeIn || state_ == kBubbleHidingFadeOut); + DCHECK(is_attached()); if (finished) { // Because of the mechanism used to interrupt animations, this is never @@ -368,9 +375,17 @@ void StatusBubbleMac::AnimationDidStop(CAAnimation* animation, bool finished) { } void StatusBubbleMac::SetState(StatusBubbleState state) { + // We must be hidden or attached, but not both. + DCHECK((state_ == kBubbleHidden) ^ is_attached()); + if (state == state_) return; + if (state == kBubbleHidden) + Detach(); + else + Attach(); + if ([delegate_ respondsToSelector:@selector(statusBubbleWillEnterState:)]) [delegate_ statusBubbleWillEnterState:state]; @@ -397,8 +412,6 @@ void StatusBubbleMac::Fade(bool show) { if (state_ == target_state) return; - Attach(); - if (immediate_) { [window_ setAlphaValue:opacity]; SetState(target_state); @@ -461,7 +474,7 @@ void StatusBubbleMac::TimerFired() { } void StatusBubbleMac::StartShowing() { - Attach(); + // Note that |SetState()| will |Attach()| or |Detach()| as required. if (state_ == kBubbleHidden) { // Arrange to begin fading in after a delay. -- cgit v1.1