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

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.