Commit 7a15c817 authored by rohitrao's avatar rohitrao Committed by Commit bot

Reverts recent changes to StatusBubbleMac.

Recent cleanup and performance changes caused a regression in the fullscreen
transition animation, leading to this revert.

This reverts commit 9ec18e41.
This reverts commit b62daf58.
This reverts commit 014fd0a1.

TBR=erikchen@chromium.org
BUG=470180,464754,454502
TEST=None

Review URL: https://codereview.chromium.org/1042633003

Cr-Commit-Position: refs/heads/master@{#322728}
parent 4eb15530
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
#include "base/debug/stack_trace.h"
#include "base/mac/mac_util.h" #include "base/mac/mac_util.h"
#include "base/mac/scoped_block.h" #include "base/mac/scoped_block.h"
#include "base/mac/sdk_forward_declarations.h" #include "base/mac/sdk_forward_declarations.h"
...@@ -143,32 +142,6 @@ const CGFloat kExpansionDurationSeconds = 0.125; ...@@ -143,32 +142,6 @@ const CGFloat kExpansionDurationSeconds = 0.125;
@end @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) StatusBubbleMac::StatusBubbleMac(NSWindow* parent, id delegate)
: parent_(parent), : parent_(parent),
delegate_(delegate), delegate_(delegate),
...@@ -204,25 +177,16 @@ void StatusBubbleMac::SetURL(const GURL& url, const std::string& languages) { ...@@ -204,25 +177,16 @@ void StatusBubbleMac::SetURL(const GURL& url, const std::string& languages) {
url_ = url; url_ = url;
languages_ = languages; languages_ = languages;
CGFloat bubble_width = NSWidth([window_ frame]); NSRect frame = [window_ frame];
// Reset frame size when bubble is hidden.
if (state_ == kBubbleHidden) { if (state_ == kBubbleHidden) {
// TODO(rohitrao): The window size is expected to be (1,1) whenever the is_expanded_ = false;
// window is hidden, but the GPU bots are hitting cases where this is not frame.size.width = NSWidth(CalculateWindowFrame(/*expand=*/false));
// true. Instead of enforcing this invariant with a DCHECK, add temporary [window_ setFrame:frame display:NO];
// 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>(bubble_width - int text_width = static_cast<int>(NSWidth(frame) -
kBubbleViewTextPositionX - kBubbleViewTextPositionX -
kTextPadding); kTextPadding);
...@@ -296,10 +260,8 @@ void StatusBubbleMac::SetText(const base::string16& text, bool is_url) { ...@@ -296,10 +260,8 @@ void StatusBubbleMac::SetText(const base::string16& text, bool is_url) {
show = false; show = false;
if (show) { if (show) {
// Call StartShowing() first to update the current bubble state before
// calculating a new size.
StartShowing();
UpdateSizeAndPosition(); UpdateSizeAndPosition();
StartShowing();
} else { } else {
StartHiding(); StartHiding();
} }
...@@ -322,22 +284,21 @@ void StatusBubbleMac::Hide() { ...@@ -322,22 +284,21 @@ void StatusBubbleMac::Hide() {
} }
} }
NSRect frame = CalculateWindowFrame(/*expand=*/false);
if (!fade_out) { if (!fade_out) {
// No animation is in progress, so the opacity can be set directly. // No animation is in progress, so the opacity can be set directly.
[window_ setAlphaValue:0.0]; [window_ setAlphaValue:0.0];
SetState(kBubbleHidden); SetState(kBubbleHidden);
frame.size = ui::kWindowSizeDeterminedLater.size;
} }
// Stop any width animation and reset the bubble size. // Stop any width animation and reset the bubble size.
if (!immediate_) { if (!immediate_) {
[NSAnimationContext beginGrouping]; [NSAnimationContext beginGrouping];
[[NSAnimationContext currentContext] setDuration:kMinimumTimeInterval]; [[NSAnimationContext currentContext] setDuration:kMinimumTimeInterval];
[[window_ animator] setFrame:frame display:NO]; [[window_ animator] setFrame:CalculateWindowFrame(/*expand=*/false)
display:NO];
[NSAnimationContext endGrouping]; [NSAnimationContext endGrouping];
} else { } else {
[window_ setFrame:frame display:NO]; [window_ setFrame:CalculateWindowFrame(/*expand=*/false) display:NO];
} }
[status_text_ release]; [status_text_ release];
...@@ -485,14 +446,7 @@ void StatusBubbleMac::Detach() { ...@@ -485,14 +446,7 @@ void StatusBubbleMac::Detach() {
DCHECK(is_attached()); DCHECK(is_attached());
// Magic setFrame: See http://crbug.com/58506 and http://crrev.com/3564021 . // Magic setFrame: See http://crbug.com/58506 and http://crrev.com/3564021 .
// TODO(rohitrao): Does the frame size actually matter here? Can we always [window_ setFrame:CalculateWindowFrame(/*expand=*/false) display:NO];
// 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 ... [parent_ removeChildWindow:window_]; // See crbug.com/28107 ...
[window_ orderOut:nil]; // ... and crbug.com/29054. [window_ orderOut:nil]; // ... and crbug.com/29054.
...@@ -518,8 +472,6 @@ void StatusBubbleMac::SetState(StatusBubbleState state) { ...@@ -518,8 +472,6 @@ void StatusBubbleMac::SetState(StatusBubbleState state) {
return; return;
if (state == kBubbleHidden) { if (state == kBubbleHidden) {
is_expanded_ = false;
// When hidden (with alpha of 0), make the window have the minimum size, // 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 // 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 // origin to 0,0 as that will cause the window to use more space in
...@@ -765,26 +717,6 @@ void StatusBubbleMac::UpdateSizeAndPosition() { ...@@ -765,26 +717,6 @@ void StatusBubbleMac::UpdateSizeAndPosition() {
if (!window_) if (!window_)
return; 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;
[window_ setFrame:frame display:YES];
}
return;
}
SetFrameAvoidingMouse(CalculateWindowFrame(/*expand=*/false), SetFrameAvoidingMouse(CalculateWindowFrame(/*expand=*/false),
GetMouseLocation()); GetMouseLocation());
} }
......
...@@ -667,19 +667,3 @@ TEST_F(StatusBubbleMacTest, BubbleAvoidsMouse) { ...@@ -667,19 +667,3 @@ TEST_F(StatusBubbleMacTest, BubbleAvoidsMouse) {
ASSERT_TRUE(CheckAvoidsMouse(x, smallValue)); ASSERT_TRUE(CheckAvoidsMouse(x, smallValue));
} }
} }
TEST_F(StatusBubbleMacTest, ReparentBubble) {
// The second window is borderless, like the window used in fullscreen mode.
base::scoped_nsobject<NSWindow> fullscreenParent(
[[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
styleMask:NSBorderlessWindowMask
backing:NSBackingStoreBuffered
defer:NO]);
// Switch parents with the bubble hidden.
bubble_->SwitchParentWindow(fullscreenParent);
// Switch back to the original parent with the bubble showing.
bubble_->SetStatus(UTF8ToUTF16("Showing"));
bubble_->SwitchParentWindow(test_window());
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment