Reland performance fixes for StatusBubbleMac.

chrome/browser/ui/cocoa/status_bubble_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/status_bubble_mac.h"
 
 #include <limits>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/debug/stack_trace.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_block.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #import "chrome/browser/ui/cocoa/bubble_view.h"
 #include "chrome/browser/ui/elide_url.h"
 #include "net/base/net_util.h"
@@ skipping 114 matching lines @@
     changes([NSAnimationContext currentContext]);
     // At this point, -animationForKey should have been called by CoreAnimation
     // to set up the animation to run. Verify this.
     DCHECK(completionHandler_ == nil);
     [NSAnimationContext endGrouping];
   }
 }
 
 @end
 
+// Mac implementation of the status bubble.
+//
+// Child windows interact with Spaces in interesting ways, so this code has to
+// follow these rules:
+//
+// 1) NSWindows cannot have zero size.  At times when the status bubble window
+//    has no specific size (for example, when hidden), its size is set to
+//    ui::kWindowSizeDeterminedLater.
+//
+// 2) Child window frames are in the coordinate space of the screen, not of the
+//    parent window.  If a child window has its origin at (0, 0), Spaces will
+//    position it in the corner of the screen but group it with the parent
+//    window in Spaces.  This causes Chrome windows to have a large (mostly
+//    blank) area in Spaces.  To avoid this, child windows always have their
+//    origin set to the lower-left corner of the window.
+//
+// 3) Detached child windows may show up as top-level windows in Spaces.  To
+//    avoid this, once the status bubble is Attach()ed to the parent, it is
+//    never detached (except in rare cases when reparenting to a fullscreen
+//    window).
+//
+// 4) To avoid unnecessary redraws, if a bubble is in the kBubbleHidden state,
+//    its size is always set to ui::kWindowSizeDeterminedLater.  The proper
+//    width for the current URL or status text is not calculated until the
+//    bubble leaves the kBubbleHidden state.
+
 StatusBubbleMac::StatusBubbleMac(NSWindow* parent, id delegate)
     : parent_(parent),
       delegate_(delegate),
       window_(nil),
       status_text_(nil),
       url_text_(nil),
       state_(kBubbleHidden),
       immediate_(false),
       is_expanded_(false),
       timer_factory_(this),
@@ skipping 15 matching lines @@
 }
 
 void StatusBubbleMac::SetStatus(const base::string16& status) {
   SetText(status, false);
 }
 
 void StatusBubbleMac::SetURL(const GURL& url, const std::string& languages) {
   url_ = url;
   languages_ = languages;
 
-  NSRect frame = [window_ frame];
-
-  // Reset frame size when bubble is hidden.
+  CGFloat bubble_width = NSWidth([window_ frame]);
   if (state_ == kBubbleHidden) {
-    is_expanded_ = false;
-    frame.size.width = NSWidth(CalculateWindowFrame(/*expand=*/false));
-    [window_ setFrame:frame display:NO];
+    // TODO(rohitrao): The window size is expected to be (1,1) whenever the
+    // window is hidden, but the GPU bots are hitting cases where this is not
+    // true.  Instead of enforcing this invariant with a DCHECK, add temporary
+    // logging to try and debug it and fix up the window size if needed.
+    // This logging is temporary and should be removed: crbug.com/467998
+    NSRect frame = [window_ frame];
+    if (!CGSizeEqualToSize(frame.size, ui::kWindowSizeDeterminedLater.size)) {
+      LOG(ERROR) << "Window size should be (1,1), but is instead ("
+                 << frame.size.width << "," << frame.size.height << ")";
+      LOG(ERROR) << base::debug::StackTrace().ToString();
+      frame.size = ui::kWindowSizeDeterminedLater.size;
+      [window_ setFrame:frame display:NO];
+    }
+    bubble_width = NSWidth(CalculateWindowFrame(/*expand=*/false));
   }
 
-  int text_width = static_cast<int>(NSWidth(frame) -
+  int text_width = static_cast<int>(bubble_width -
                                     kBubbleViewTextPositionX -
                                     kTextPadding);
 
   // Scale from view to window coordinates before eliding URL string.
   NSSize scaled_width = NSMakeSize(text_width, 0);
   scaled_width = [[parent_ contentView] convertSize:scaled_width fromView:nil];
   text_width = static_cast<int>(scaled_width.width);
   NSFont* font = [[window_ contentView] font];
   gfx::FontList font_list_chr(
       gfx::Font(base::SysNSStringToUTF8([font fontName]), [font pointSize]));
@@ skipping 53 matching lines @@
 
   bool show = true;
   if ([*main length] > 0)
     [[window_ contentView] setContent:*main];
   else if ([*backup length] > 0)
     [[window_ contentView] setContent:*backup];
   else
     show = false;
 
   if (show) {
+    // Call StartShowing() first to update the current bubble state before
+    // calculating a new size.
+    StartShowing();
     UpdateSizeAndPosition();
-    StartShowing();
   } else {
     StartHiding();
   }
 }
 
 void StatusBubbleMac::Hide() {
   CancelTimer();
   CancelExpandTimer();
   is_expanded_ = false;
 
   bool fade_out = false;
   if (state_ == kBubbleHidingFadeOut || state_ == kBubbleShowingFadeIn) {
     SetState(kBubbleHidingFadeOut);
 
     if (!immediate_) {
       // An animation is in progress.  Cancel it by starting a new animation.
       // Use kMinimumTimeInterval to set the opacity as rapidly as possible.
       fade_out = true;
       AnimateWindowAlpha(0.0, kMinimumTimeInterval);
     }
   }
 
+  NSRect frame = CalculateWindowFrame(/*expand=*/false);
   if (!fade_out) {
     // No animation is in progress, so the opacity can be set directly.
     [window_ setAlphaValue:0.0];
     SetState(kBubbleHidden);
+    frame.size = ui::kWindowSizeDeterminedLater.size;
   }
 
   // Stop any width animation and reset the bubble size.
   if (!immediate_) {
     [NSAnimationContext beginGrouping];
     [[NSAnimationContext currentContext] setDuration:kMinimumTimeInterval];
-    [[window_ animator] setFrame:CalculateWindowFrame(/*expand=*/false)
-                         display:NO];
+    [[window_ animator] setFrame:frame display:NO];
     [NSAnimationContext endGrouping];
   } else {
-    [window_ setFrame:CalculateWindowFrame(/*expand=*/false) display:NO];
+    [window_ setFrame:frame display:NO];
   }
 
   [status_text_ release];
   status_text_ = nil;
   [url_text_ release];
   url_text_ = nil;
 }
 
 void StatusBubbleMac::SetFrameAvoidingMouse(
     NSRect window_frame, const gfx::Point& mouse_pos) {
@@ skipping 127 matching lines @@
   [window_ orderFront:nil];
   [parent_ addChildWindow:window_ ordered:NSWindowAbove];
 
   [[window_ contentView] setThemeProvider:parent_];
 }
 
 void StatusBubbleMac::Detach() {
   DCHECK(is_attached());
 
   // Magic setFrame: See http://crbug.com/58506 and http://crrev.com/3564021 .
-  [window_ setFrame:CalculateWindowFrame(/*expand=*/false) display:NO];
+  // TODO(rohitrao): Does the frame size actually matter here?  Can we always
+  // set it to kWindowSizeDeterminedLater?
+  NSRect frame = [window_ frame];
+  frame.size = ui::kWindowSizeDeterminedLater.size;
+  if (state_ != kBubbleHidden) {
+    frame = CalculateWindowFrame(/*expand=*/false);
+  }
+  [window_ setFrame:frame display:NO];
   [parent_ removeChildWindow:window_];  // See crbug.com/28107 ...
   [window_ orderOut:nil];               // ... and crbug.com/29054.
 
   [[window_ contentView] setThemeProvider:nil];
 }
 
 void StatusBubbleMac::AnimationDidStop() {
   DCHECK([NSThread isMainThread]);
   DCHECK(state_ == kBubbleShowingFadeIn || state_ == kBubbleHidingFadeOut);
   DCHECK(is_attached());
 
   if (state_ == kBubbleShowingFadeIn) {
     DCHECK_EQ([[window_ animator] alphaValue], kBubbleOpacity);
     SetState(kBubbleShown);
   } else {
     DCHECK_EQ([[window_ animator] alphaValue], 0.0);
     SetState(kBubbleHidden);
   }
 }
 
 void StatusBubbleMac::SetState(StatusBubbleState state) {
   if (state == state_)
     return;
 
   if (state == kBubbleHidden) {
+    is_expanded_ = false;
+
     // When hidden (with alpha of 0), make the window have the minimum size,
     // while still keeping the same origin. It's important to not set the
     // origin to 0,0 as that will cause the window to use more space in
     // Expose/Mission Control. See http://crbug.com/81969.
     //
     // Also, doing it this way instead of detaching the window avoids bugs with
     // Spaces and Cmd-`. See http://crbug.com/31821 and http://crbug.com/61629.
     NSRect frame = [window_ frame];
     frame.size = ui::kWindowSizeDeterminedLater.size;
     [window_ setFrame:frame display:YES];
@@ skipping 225 matching lines @@
   [NSAnimationContext beginGrouping];
   [[NSAnimationContext currentContext] setDuration:kExpansionDurationSeconds];
   [[window_ animator] setFrame:actual_window_frame display:YES];
   [NSAnimationContext endGrouping];
 }
 
 void StatusBubbleMac::UpdateSizeAndPosition() {
   if (!window_)
     return;
 
+  // There is no need to update the size if the bubble is hidden.
+  if (state_ == kBubbleHidden) {
+    // Verify that hidden bubbles always have size equal to
+    // ui::kWindowSizeDeterminedLater.
+
+    // TODO(rohitrao): The GPU bots are hitting cases where this is not true.
+    // Instead of enforcing this invariant with a DCHECK, add temporary logging
+    // to try and debug it and fix up the window size if needed.
+    // This logging is temporary and should be removed: crbug.com/467998
+    NSRect frame = [window_ frame];
+    if (!CGSizeEqualToSize(frame.size, ui::kWindowSizeDeterminedLater.size)) {
+      LOG(ERROR) << "Window size should be (1,1), but is instead ("
+                 << frame.size.width << "," << frame.size.height << ")";
+      LOG(ERROR) << base::debug::StackTrace().ToString();
+      frame.size = ui::kWindowSizeDeterminedLater.size;
+    }
+
+    // During the fullscreen animation, the parent window's origin may change
+    // without updating the status bubble.  To avoid animation glitches, always
+    // update the bubble's origin to match the parent's, even when hidden.
+    frame.origin = [parent_ frame].origin;
+    [window_ setFrame:frame display:NO];

[rohitrao (ping after 24h), 2015/04/27 23:29:48]

Updated to not force a display.  Setting this to NO does not seem to cause
animation glitches.

+    return;
+  }
+
   SetFrameAvoidingMouse(CalculateWindowFrame(/*expand=*/false),
                         GetMouseLocation());
 }
 
 void StatusBubbleMac::SwitchParentWindow(NSWindow* parent) {
   DCHECK(parent);
   DCHECK(is_attached());
 
   Detach();
   parent_ = parent;
@@ skipping 45 matching lines @@
     }
 
     // Round the top corners when the bubble is below the parent window.
     if (NSMinY(window_frame) < NSMinY(parent_frame)) {
       corner_flags |= kRoundedTopLeftCorner | kRoundedTopRightCorner;
     }
   }
 
   return corner_flags;
 }