summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorerikchen <erikchen@chromium.org>2014-10-10 12:56:44 -0700
committerCommit bot <commit-bot@chromium.org>2014-10-10 19:56:54 +0000
commit9fa914507cbcce4fb7f05501106d0a5183d3dff9 (patch)
tree733a7c20de8c5138ac0d103c92456ed2c3176137
parent22c781e61a21d95a102e0e72c76f05c2bbe58093 (diff)
downloadchromium_src-9fa914507cbcce4fb7f05501106d0a5183d3dff9.zip
chromium_src-9fa914507cbcce4fb7f05501106d0a5183d3dff9.tar.gz
chromium_src-9fa914507cbcce4fb7f05501106d0a5183d3dff9.tar.bz2
mac: Use a full-size content view (reland 1).
The original attempt to land the CL incorrectly interacted with another recently landed CL that added the tabStripBackgroundView (https://codereview.chromium.org/611453004). This CL was causing that view to become layer backed, which caused it to draw on top of non-layer backed window content, including the window controls. This reland reworks the logic so that the tabStripBackgroundView is added to the root view and does not become layer backed. This reland also updates browser_tests to catch similar such errors in the future. --------------Original CL Description--------------- Make the window's contentView have the same size as the window itself. Adjust the view hierarchy so that Chrome does not add subviews directly to the root view. By default, the contentView does not occupy the full size of a framed window. Chrome still wants to draw in the title bar. Historically, Chrome has done this by adding subviews directly to the root view. This causes several problems. The most egregious is related to layer ordering when the root view does not have a layer. By making the window's contentView full-sized, there is no longer any need to add subviews to the root view. I deleted the tests in presentation_mode_controller_unittest.mm since they were layout tests for the browser_window_controller, but the tests in browser_window_layout_unittest.mm are both more robust and more comprehensive. I fixed a bug in moveViewsForImmersiveFullscreen:... where the tabStripView was being added to the source window rather than the destination window. This CL is mostly a reland of https://codereview.chromium.org/399803002/. Original CL Committed: https://crrev.com/746dbb9cfefaac243704956db431ff9bb9b0485b Original CL Cr-Commit-Position: refs/heads/master@{#298584} BUG=417923 Review URL: https://codereview.chromium.org/646703002 Cr-Commit-Position: refs/heads/master@{#299168}
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller.mm9
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm141
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller_private.mm39
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller_unittest.mm60
-rw-r--r--chrome/browser/ui/cocoa/framed_browser_window.mm3
-rw-r--r--chrome/browser/ui/cocoa/fullscreen_window.mm1
-rw-r--r--chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm228
-rw-r--r--chrome/browser/ui/cocoa/tabs/tab_window_controller.h17
-rw-r--r--chrome/browser/ui/cocoa/tabs/tab_window_controller.mm54
-rw-r--r--chrome/browser/ui/cocoa/version_independent_window.h33
-rw-r--r--chrome/browser/ui/cocoa/version_independent_window.mm62
-rw-r--r--chrome/chrome_tests_unit.gypi1
-rw-r--r--chrome/common/chrome_switches.cc4
-rw-r--r--chrome/common/chrome_switches.h1
14 files changed, 289 insertions, 364 deletions
diff --git a/chrome/browser/ui/cocoa/browser_window_controller.mm b/chrome/browser/ui/cocoa/browser_window_controller.mm
index f08833a..c81ef74 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller.mm
@@ -946,10 +946,11 @@ using content::WebContents;
// Disable subview resizing while resizing the window, or else we will get
// unwanted renderer resizes. The calling code must call layoutSubviews to
// make things right again.
- NSView* contentView = [window contentView];
- [contentView setAutoresizesSubviews:NO];
+ NSView* chromeContentView = [self chromeContentView];
+ BOOL autoresizesSubviews = [chromeContentView autoresizesSubviews];
+ [chromeContentView setAutoresizesSubviews:NO];
[window setFrame:windowFrame display:NO];
- [contentView setAutoresizesSubviews:YES];
+ [chromeContentView setAutoresizesSubviews:autoresizesSubviews];
return YES;
}
@@ -1573,7 +1574,7 @@ using content::WebContents;
if (!downloadShelfController_.get()) {
downloadShelfController_.reset([[DownloadShelfController alloc]
initWithBrowser:browser_.get() resizeDelegate:self]);
- [[[self window] contentView] addSubview:[downloadShelfController_ view]];
+ [self.chromeContentView addSubview:[downloadShelfController_ view]];
}
}
diff --git a/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm b/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
index c8c0e22..a01111c 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
@@ -68,6 +68,96 @@ enum ViewID {
VIEW_ID_COUNT,
};
+// Checks that no views draw on top of the supposedly exposed view.
+class ViewExposedChecker {
+ public:
+ ViewExposedChecker() : below_exposed_view_(YES) {}
+ ~ViewExposedChecker() {}
+
+ void SetExceptions(NSArray* exceptions) {
+ exceptions_.reset([exceptions retain]);
+ }
+
+ // Checks that no views draw in front of |view|, with the exception of
+ // |exceptions|.
+ void CheckViewExposed(NSView* view) {
+ below_exposed_view_ = YES;
+ exposed_view_.reset([view retain]);
+ CheckViewsDoNotObscure([[[view window] contentView] superview]);
+ }
+
+ private:
+ // Checks that |view| does not draw on top of |exposed_view_|.
+ void CheckViewDoesNotObscure(NSView* view) {
+ NSRect viewWindowFrame = [view convertRect:[view bounds] toView:nil];
+ NSRect viewBeingVerifiedWindowFrame =
+ [exposed_view_ convertRect:[exposed_view_ bounds] toView:nil];
+
+ // The views do not intersect.
+ if (!NSIntersectsRect(viewBeingVerifiedWindowFrame, viewWindowFrame))
+ return;
+
+ // No view can be above the view being checked.
+ EXPECT_TRUE(below_exposed_view_);
+
+ // If |view| is a parent of |exposed_view_|, then there's nothing else
+ // to check.
+ NSView* parent = exposed_view_;
+ while (parent != nil) {
+ parent = [parent superview];
+ if (parent == view)
+ return;
+ }
+
+ if ([exposed_view_ layer])
+ return;
+
+ // If the view being verified doesn't have a layer, then no views that
+ // intersect it can have a layer.
+ if ([exceptions_ containsObject:view]) {
+ EXPECT_FALSE([view isOpaque]);
+ return;
+ }
+
+ EXPECT_TRUE(![view layer]) << [[view description] UTF8String] << " " <<
+ [NSStringFromRect(viewWindowFrame) UTF8String];
+ }
+
+ // Recursively checks that |view| and its subviews do not draw on top of
+ // |exposed_view_|. The recursion passes through all views in order of
+ // back-most in Z-order to front-most in Z-order.
+ void CheckViewsDoNotObscure(NSView* view) {
+ // If this is the view being checked, don't recurse into its subviews. All
+ // future views encountered in the recursion are in front of the view being
+ // checked.
+ if (view == exposed_view_) {
+ below_exposed_view_ = NO;
+ return;
+ }
+
+ CheckViewDoesNotObscure(view);
+
+ // Perform the recursion.
+ for (NSView* subview in [view subviews])
+ CheckViewsDoNotObscure(subview);
+ }
+
+ // The method CheckViewExposed() recurses through the views in the view
+ // hierarchy and checks that none of the views obscure |exposed_view_|.
+ base::scoped_nsobject<NSView> exposed_view_;
+
+ // While this flag is true, the views being recursed through are below
+ // |exposed_view_| in Z-order. After the recursion passes |exposed_view_|,
+ // this flag is set to false.
+ BOOL below_exposed_view_;
+
+ // Exceptions are allowed to overlap |exposed_view_|. Exceptions must still
+ // be Z-order behind |exposed_view_|.
+ base::scoped_nsobject<NSArray> exceptions_;
+
+ DISALLOW_COPY_AND_ASSIGN(ViewExposedChecker);
+};
+
} // namespace
@interface InfoBarContainerController(TestingAPI)
@@ -223,22 +313,29 @@ class BrowserWindowControllerTest : public InProcessBrowserTest {
return icon_bottom.y - info_bar_top.y;
}
- // The traffic lights should always be in front of the content view and the
- // tab strip view. Since the traffic lights change across OSX versions, this
- // test verifies that the contentView is in the back, and if the tab strip
- // view is a sibling, it is directly in front of the content view.
- void VerifyTrafficLightZOrder() const {
- NSView* contentView = [[controller() window] contentView];
- NSView* rootView = [contentView superview];
- NSArray* subviews = [rootView subviews];
-
- EXPECT_EQ([controller() tabStripBackgroundView],
- [subviews objectAtIndex:0]);
- EXPECT_EQ(contentView, [subviews objectAtIndex:1]);
-
- NSView* tabStripView = [controller() tabStripView];
- if ([subviews containsObject:tabStripView])
- EXPECT_EQ(tabStripView, [subviews objectAtIndex:2]);
+ // Nothing should draw on top of the window controls.
+ void VerifyWindowControlsZOrder() {
+ NSWindow* window = [controller() window];
+ ViewExposedChecker checker;
+
+ // The exceptions are the contentView, chromeContentView and tabStripView,
+ // which are layer backed but transparent.
+ NSArray* exceptions = @[
+ [window contentView],
+ controller().chromeContentView,
+ controller().tabStripView
+ ];
+ checker.SetExceptions(exceptions);
+
+ checker.CheckViewExposed([window standardWindowButton:NSWindowCloseButton]);
+ checker.CheckViewExposed(
+ [window standardWindowButton:NSWindowMiniaturizeButton]);
+ checker.CheckViewExposed([window standardWindowButton:NSWindowZoomButton]);
+
+ // There is no fullscreen button on OSX 10.6 or OSX 10.10+.
+ NSView* view = [window standardWindowButton:NSWindowFullScreenButton];
+ if (view)
+ checker.CheckViewExposed(view);
}
private:
@@ -500,15 +597,17 @@ IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest,
IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest, TrafficLightZOrder) {
// Verify z order immediately after creation.
- VerifyTrafficLightZOrder();
+ VerifyWindowControlsZOrder();
- // Toggle overlay, then verify z order.
+ // Verify z order in and out of overlay.
[controller() showOverlay];
+ VerifyWindowControlsZOrder();
[controller() removeOverlay];
- VerifyTrafficLightZOrder();
+ VerifyWindowControlsZOrder();
- // Toggle immersive fullscreen, then verify z order.
+ // Toggle immersive fullscreen, then verify z order. In immersive fullscreen,
+ // there are no window controls.
[controller() enterImmersiveFullscreen];
[controller() exitImmersiveFullscreen];
- VerifyTrafficLightZOrder();
+ VerifyWindowControlsZOrder();
}
diff --git a/chrome/browser/ui/cocoa/browser_window_controller_private.mm b/chrome/browser/ui/cocoa/browser_window_controller_private.mm
index a28c8b0..3874f20 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller_private.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller_private.mm
@@ -358,13 +358,10 @@ willPositionSheet:(NSWindow*)sheet
[tabStripView removeFromSuperview];
}
- // Ditto for the content view.
- base::scoped_nsobject<NSView> contentView(
- [[sourceWindow contentView] retain]);
// Disable autoresizing of subviews while we move views around. This prevents
// spurious renderer resizes.
- [contentView setAutoresizesSubviews:NO];
- [contentView removeFromSuperview];
+ [self.chromeContentView setAutoresizesSubviews:NO];
+ [self.chromeContentView removeFromSuperview];
// Have to do this here, otherwise later calls can crash because the window
// has no delegate.
@@ -376,9 +373,11 @@ willPositionSheet:(NSWindow*)sheet
// drawOverlayRect:]. I'm pretty convinced this is an Apple bug, but there is
// no visual impact. I have been unable to tickle it away with other window
// or view manipulation Cocoa calls. Stack added to suppressions_mac.txt.
- [contentView setAutoresizesSubviews:YES];
- [destWindow setContentView:contentView];
- [self moveContentViewToBack:contentView];
+ [self.chromeContentView setAutoresizesSubviews:YES];
+ [[destWindow contentView] addSubview:self.chromeContentView
+ positioned:NSWindowBelow
+ relativeTo:nil];
+ [self.chromeContentView setFrame:[[destWindow contentView] bounds]];
// Move the incognito badge if present.
if ([self shouldShowAvatar]) {
@@ -392,7 +391,7 @@ willPositionSheet:(NSWindow*)sheet
// Add the tab strip after setting the content view and moving the incognito
// badge (if any), so that the tab strip will be on top (in the z-order).
if ([self hasTabStrip])
- [self insertTabStripView:tabStripView intoWindow:[self window]];
+ [self insertTabStripView:tabStripView intoWindow:destWindow];
[sourceWindow setWindowController:nil];
[self setWindow:destWindow];
@@ -700,7 +699,7 @@ willPositionSheet:(NSWindow*)sheet
for (NSWindow* window in [[NSApplication sharedApplication] windows]) {
if ([window
isKindOfClass:NSClassFromString(@"NSToolbarFullScreenWindow")]) {
- [window.contentView setHidden:YES];
+ [[window contentView] setHidden:YES];
}
}
}
@@ -986,18 +985,18 @@ willPositionSheet:(NSWindow*)sheet
- (void)setContentViewSubviews:(NSArray*)subviews {
// Subviews already match.
- if ([[self.window.contentView subviews] isEqual:subviews])
+ if ([[self.chromeContentView subviews] isEqual:subviews])
return;
// The tabContentArea isn't a subview, so just set all the subviews.
NSView* tabContentArea = [self tabContentArea];
- if (![[self.window.contentView subviews] containsObject:tabContentArea]) {
- [self.window.contentView setSubviews:subviews];
+ if (![[self.chromeContentView subviews] containsObject:tabContentArea]) {
+ [self.chromeContentView setSubviews:subviews];
return;
}
// Remove all subviews that aren't the tabContentArea.
- for (NSView* view in [[self.window.contentView subviews] copy]) {
+ for (NSView* view in [[self.chromeContentView subviews] copy]) {
if (view != tabContentArea)
[view removeFromSuperview];
}
@@ -1006,17 +1005,17 @@ willPositionSheet:(NSWindow*)sheet
NSInteger index = [subviews indexOfObject:tabContentArea];
for (int i = index - 1; i >= 0; --i) {
NSView* view = [subviews objectAtIndex:i];
- [self.window.contentView addSubview:view
- positioned:NSWindowBelow
- relativeTo:nil];
+ [self.chromeContentView addSubview:view
+ positioned:NSWindowBelow
+ relativeTo:nil];
}
// Add in the subviews above the tabContentArea.
for (NSUInteger i = index + 1; i < [subviews count]; ++i) {
NSView* view = [subviews objectAtIndex:i];
- [self.window.contentView addSubview:view
- positioned:NSWindowAbove
- relativeTo:nil];
+ [self.chromeContentView addSubview:view
+ positioned:NSWindowAbove
+ relativeTo:nil];
}
}
diff --git a/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm b/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm
index bdc6a3f..06af3cf 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm
@@ -20,6 +20,7 @@
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/cocoa/cocoa_profile_test.h"
+#import "chrome/browser/ui/cocoa/fast_resize_view.h"
#include "chrome/browser/ui/cocoa/find_bar/find_bar_bridge.h"
#include "chrome/browser/ui/cocoa/tabs/tab_strip_view.h"
#import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
@@ -241,21 +242,63 @@ TEST_F(BrowserWindowControllerTest, TestIncognitoWidthSpace) {
namespace {
+// Returns the frame of the view in window coordinates.
+NSRect FrameInWindowForView(NSView* view) {
+ return [[view superview] convertRect:[view frame] toView:nil];
+}
+
+// Whether the view's frame is within the bounds of the superview.
+BOOL ViewContainmentValid(NSView* view) {
+ if (NSIsEmptyRect([view frame]))
+ return YES;
+
+ return NSContainsRect([[view superview] bounds], [view frame]);
+}
+
+// Checks the view hierarchy rooted at |view| to ensure that each view is
+// properly contained.
+BOOL ViewHierarchyContainmentValid(NSView* view) {
+ // TODO(erikchen): Fix these views to have correct containment.
+ // http://crbug.com/397665.
+ if ([view isKindOfClass:NSClassFromString(@"DownloadShelfView")])
+ return YES;
+ if ([view isKindOfClass:NSClassFromString(@"BookmarkBarToolbarView")])
+ return YES;
+ if ([view isKindOfClass:NSClassFromString(@"BrowserActionsContainerView")])
+ return YES;
+
+ if (!ViewContainmentValid(view)) {
+ LOG(ERROR) << "View violates containment: " <<
+ [[view description] UTF8String];
+ return NO;
+ }
+
+ for (NSView* subview in [view subviews]) {
+ BOOL result = ViewHierarchyContainmentValid(subview);
+ if (!result)
+ return NO;
+ }
+
+ return YES;
+}
+
// Verifies that the toolbar, infobar, tab content area, and download shelf
// completely fill the area under the tabstrip.
void CheckViewPositions(BrowserWindowController* controller) {
- NSRect contentView = [[[controller window] contentView] bounds];
- NSRect tabstrip = [[controller tabStripView] frame];
- NSRect toolbar = [[controller toolbarView] frame];
- NSRect infobar = [[controller infoBarContainerView] frame];
- NSRect contentArea = [[controller tabContentArea] frame];
+ EXPECT_TRUE(ViewHierarchyContainmentValid([[controller window] contentView]));
+
+ NSRect contentView = FrameInWindowForView([[controller window] contentView]);
+ NSRect tabstrip = FrameInWindowForView([controller tabStripView]);
+ NSRect toolbar = FrameInWindowForView([controller toolbarView]);
+ NSRect infobar = FrameInWindowForView([controller infoBarContainerView]);
+ NSRect tabContent = FrameInWindowForView([controller tabContentArea]);
NSRect download = NSZeroRect;
if ([[[controller downloadShelf] view] superview])
download = [[[controller downloadShelf] view] frame];
EXPECT_EQ(NSMinY(contentView), NSMinY(download));
- EXPECT_EQ(NSMaxY(download), NSMinY(contentArea));
- EXPECT_EQ(NSMaxY(contentArea), NSMinY(infobar));
+ EXPECT_EQ(NSMaxY(download), NSMinY(tabContent));
+ EXPECT_EQ(NSMaxY(tabContent), NSMinY(infobar));
// Bookmark bar frame is random memory when hidden.
if ([controller bookmarkBarVisible]) {
@@ -620,7 +663,7 @@ TEST_F(BrowserWindowControllerTest, TestFindBarOnTop) {
[controller_ addFindBar:bridge.find_bar_cocoa_controller()];
// Test that the Z-order of the find bar is on top of everything.
- NSArray* subviews = [[[controller_ window] contentView] subviews];
+ NSArray* subviews = [controller_.chromeContentView subviews];
NSUInteger findBar_index =
[subviews indexOfObject:[controller_ findBarView]];
EXPECT_NE(NSNotFound, findBar_index);
@@ -888,6 +931,7 @@ TEST_F(BrowserWindowFullScreenControllerTest, DISABLED_TestActivate) {
styleMask:NSBorderlessWindowMask
backing:NSBackingStoreBuffered
defer:NO]);
+ [[testFullscreenWindow_ contentView] setWantsLayer:YES];
return testFullscreenWindow_.get();
}
@end
diff --git a/chrome/browser/ui/cocoa/framed_browser_window.mm b/chrome/browser/ui/cocoa/framed_browser_window.mm
index af35352..c290f70 100644
--- a/chrome/browser/ui/cocoa/framed_browser_window.mm
+++ b/chrome/browser/ui/cocoa/framed_browser_window.mm
@@ -69,7 +69,8 @@ const CGFloat kWindowGradientHeight = 24.0;
if ((self = [super initWithContentRect:contentRect
styleMask:styleMask
backing:NSBackingStoreBuffered
- defer:YES])) {
+ defer:YES
+ wantsViewsOverTitlebar:hasTabStrip])) {
// The 10.6 fullscreen code copies the title to a different window, which
// will assert if it's nil.
[self setTitle:@""];
diff --git a/chrome/browser/ui/cocoa/fullscreen_window.mm b/chrome/browser/ui/cocoa/fullscreen_window.mm
index 9125b21..4530253 100644
--- a/chrome/browser/ui/cocoa/fullscreen_window.mm
+++ b/chrome/browser/ui/cocoa/fullscreen_window.mm
@@ -27,6 +27,7 @@
// Borderless windows don't usually show up in the Windows menu so whine at
// Cocoa until it complies. See -dealloc and -setTitle: as well.
[NSApp addWindowsItem:self title:@"" filename:NO];
+ [[self contentView] setWantsLayer:YES];
}
return self;
}
diff --git a/chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm b/chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm
deleted file mode 100644
index c9511bc..0000000
--- a/chrome/browser/ui/cocoa/presentation_mode_controller_unittest.mm
+++ /dev/null
@@ -1,228 +0,0 @@
-// Copyright 2014 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.
-
-#import "chrome/browser/ui/cocoa/browser_window_controller.h"
-#import "chrome/browser/ui/cocoa/browser_window_controller_private.h"
-#include "chrome/browser/ui/cocoa/cocoa_profile_test.h"
-#import "chrome/browser/ui/cocoa/fast_resize_view.h"
-#import "chrome/browser/ui/cocoa/presentation_mode_controller.h"
-#import "chrome/browser/ui/cocoa/tabs/tab_strip_view.h"
-
-namespace {
-
-// The height of the AppKit menu bar.
-const CGFloat kMenuBarHeight = 22;
-
-// Returns the frame of the view in window coordinates.
-NSRect FrameInWindow(NSView* view) {
- return [view convertRect:[view bounds] toView:nil];
-}
-
-// Returns the min Y of the view in window coordinates.
-CGFloat MinYInWindow(NSView* view) {
- return NSMinY(FrameInWindow(view));
-}
-
-// Returns the max Y of the view in window coordinates.
-CGFloat MaxYInWindow(NSView* view) {
- return NSMaxY(FrameInWindow(view));
-}
-
-// Returns the view positioned highest in the toolbar area.
-NSView* HighestViewInToolbarArea(BrowserWindowController* controller) {
- return [controller tabStripView];
-}
-
-// Returns the view positioned lowest in the toolbar area.
-NSView* LowestViewInToolbarArea(BrowserWindowController* controller) {
- return [[controller bookmarkBarController] view];
-}
-
-// Check the layout of the views in the toolbar area when none of them overlap.
-void CheckToolbarLayoutNoOverlap(BrowserWindowController* controller) {
- EXPECT_EQ(MinYInWindow([controller tabStripView]),
- MaxYInWindow([[controller toolbarController] view]));
- EXPECT_EQ(MinYInWindow([[controller toolbarController] view]),
- MaxYInWindow([[controller bookmarkBarController] view]));
-}
-
-// Check the layout of all of the views when none of them overlap.
-void CheckLayoutNoOverlap(BrowserWindowController* controller) {
- CheckToolbarLayoutNoOverlap(controller);
- EXPECT_EQ(MinYInWindow([[controller bookmarkBarController] view]),
- MaxYInWindow([[controller infoBarContainerController] view]));
- EXPECT_EQ(MinYInWindow([[controller infoBarContainerController] view]),
- MaxYInWindow([controller tabContentArea]));
-}
-
-} // namespace
-
-// -------------------MockPresentationModeController----------------------------
-// Mock of PresentationModeController that eliminates dependencies on AppKit.
-@interface MockPresentationModeController : PresentationModeController
-@end
-
-@implementation MockPresentationModeController
-
-- (void)setSystemFullscreenModeTo:(base::mac::FullScreenMode)mode {
-}
-
-- (CGFloat)floatingBarVerticalOffset {
- return kMenuBarHeight;
-}
-
-@end
-
-// -------------------MockBrowserWindowController-------------------------------
-// Mock of BrowserWindowController that eliminates dependencies on AppKit.
-@interface MockBrowserWindowController : BrowserWindowController {
- @public
- MockPresentationModeController* presentationController_;
- BOOL isInAppKitFullscreen_;
-}
-@end
-
-@implementation MockBrowserWindowController
-
-// Use the mock presentation controller.
-// Superclass override.
-- (PresentationModeController*)newPresentationModeControllerWithStyle:
- (fullscreen_mac::SlidingStyle)style {
- presentationController_ =
- [[MockPresentationModeController alloc] initWithBrowserController:self
- style:style];
- return presentationController_;
-}
-
-// Manually control whether the floating bar (omnibox, tabstrip, etc.) has
-// focus.
-// Superclass override.
-- (BOOL)floatingBarHasFocus {
- return NO;
-}
-
-// Superclass override.
-- (BOOL)isInAppKitFullscreen {
- return isInAppKitFullscreen_;
-}
-
-// Superclass override.
-- (BOOL)placeBookmarkBarBelowInfoBar {
- return NO;
-}
-@end
-
-// -------------------PresentationModeControllerTest----------------------------
-class PresentationModeControllerTest : public CocoaProfileTest {
- public:
- virtual void SetUp() override {
- CocoaProfileTest::SetUp();
- ASSERT_TRUE(browser());
-
- controller_ = [[MockBrowserWindowController alloc] initWithBrowser:browser()
- takeOwnership:NO];
- [[controller_ bookmarkBarController]
- updateState:BookmarkBar::SHOW
- changeType:BookmarkBar::DONT_ANIMATE_STATE_CHANGE];
- }
-
- virtual void TearDown() override {
- [controller_ close];
- CocoaProfileTest::TearDown();
- }
-
- public:
- MockBrowserWindowController* controller_;
-};
-
-// Tests the layout of views in Canonical Fullscreen (emulating AppKit
-// Fullscreen API).
-TEST_F(PresentationModeControllerTest, CanonicalFullscreenAppKitLayout) {
- // Check initial layout.
- CGFloat windowHeight = NSHeight([[controller_ window] frame]);
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabStripView]));
- CheckLayoutNoOverlap(controller_);
-
- // No change after adjusting UI for Canonical Fullscreen.
- controller_->isInAppKitFullscreen_ = YES;
- [controller_
- adjustUIForSlidingFullscreenStyle:fullscreen_mac::OMNIBOX_TABS_PRESENT];
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabStripView]));
- CheckLayoutNoOverlap(controller_);
-
- // The menu bar is starting to animate in. All views should slide down by a
- // small amount. The content area doesn't change size.
- [controller_->presentationController_ setMenuBarRevealProgress:0.3];
- EXPECT_LT(MaxYInWindow([controller_ tabStripView]), windowHeight - 1);
- EXPECT_GT(MaxYInWindow([controller_ tabStripView]),
- windowHeight - kMenuBarHeight + 1);
- CheckToolbarLayoutNoOverlap(controller_);
- EXPECT_EQ(MinYInWindow([[controller_ bookmarkBarController] view]),
- MaxYInWindow([[controller_ infoBarContainerController] view]));
- EXPECT_LT(MinYInWindow([[controller_ infoBarContainerController] view]),
- MaxYInWindow([controller_ tabContentArea]));
-
- // The menu bar is fully visible. All views should slide down by the size of
- // the menu bar. The content area doesn't change size.
- [controller_->presentationController_ setMenuBarRevealProgress:1];
- EXPECT_FLOAT_EQ(windowHeight - kMenuBarHeight,
- MaxYInWindow([controller_ tabStripView]));
- CheckToolbarLayoutNoOverlap(controller_);
- EXPECT_EQ(MinYInWindow([[controller_ bookmarkBarController] view]),
- MaxYInWindow([[controller_ infoBarContainerController] view]));
- EXPECT_LT(MinYInWindow([[controller_ infoBarContainerController] view]),
- MaxYInWindow([controller_ tabContentArea]));
-
- // The menu bar has disappeared. All views should return to normal.
- [controller_->presentationController_ setMenuBarRevealProgress:0];
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabStripView]));
- CheckLayoutNoOverlap(controller_);
-}
-
-// Tests the layout of views in Presentation Mode (emulating AppKit Fullscreen
-// API).
-TEST_F(PresentationModeControllerTest, PresentationModeAppKitLayout) {
- // Check initial layout.
- CGFloat windowHeight = NSHeight([[controller_ window] frame]);
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabStripView]));
- CheckLayoutNoOverlap(controller_);
-
- // Adjust UI for Presentation Mode.
- controller_->isInAppKitFullscreen_ = YES;
- [controller_
- adjustUIForSlidingFullscreenStyle:fullscreen_mac::OMNIBOX_TABS_HIDDEN];
-
- // In AppKit Fullscreen, the titlebar disappears. This test can't remove the
- // titlebar without non-trivially changing the view hierarchy. Instead, it
- // adjusts the expectations to sometimes use contentHeight instead of
- // windowHeight (the two should be the same in AppKit Fullscreen).
- CGFloat contentHeight = NSHeight([[[controller_ window] contentView] bounds]);
- CheckToolbarLayoutNoOverlap(controller_);
- EXPECT_EQ(windowHeight, MinYInWindow(LowestViewInToolbarArea(controller_)));
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabContentArea]));
-
- // The menu bar is starting to animate in. All views except the content view
- // should slide down by a small amount.
- [controller_->presentationController_ setMenuBarRevealProgress:0.3];
- [controller_->presentationController_ changeToolbarFraction:0.3];
- CheckToolbarLayoutNoOverlap(controller_);
- EXPECT_LT(MinYInWindow(LowestViewInToolbarArea(controller_)), contentHeight);
- EXPECT_GT(MaxYInWindow(HighestViewInToolbarArea(controller_)), contentHeight);
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabContentArea]));
-
- // The menu bar is fully visible. All views should slide down by the size of
- // the menu bar.
- [controller_->presentationController_ setMenuBarRevealProgress:1];
- [controller_->presentationController_ changeToolbarFraction:1];
- CheckToolbarLayoutNoOverlap(controller_);
- EXPECT_EQ(contentHeight, MaxYInWindow(HighestViewInToolbarArea(controller_)));
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabContentArea]));
-
- // The menu bar has disappeared. All views should return to normal.
- [controller_->presentationController_ setMenuBarRevealProgress:0];
- [controller_->presentationController_ changeToolbarFraction:0];
- CheckToolbarLayoutNoOverlap(controller_);
- EXPECT_EQ(windowHeight, MinYInWindow(LowestViewInToolbarArea(controller_)));
- EXPECT_EQ(windowHeight, MaxYInWindow([controller_ tabContentArea]));
-}
diff --git a/chrome/browser/ui/cocoa/tabs/tab_window_controller.h b/chrome/browser/ui/cocoa/tabs/tab_window_controller.h
index 7238b09..1c4e9e1 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_window_controller.h
+++ b/chrome/browser/ui/cocoa/tabs/tab_window_controller.h
@@ -23,10 +23,20 @@
@interface TabWindowController : NSWindowController<NSWindowDelegate> {
@private
+ // Wrapper view around web content, and the developer tools view.
base::scoped_nsobject<FastResizeView> tabContentArea_;
base::scoped_nsobject<NSView> tabStripBackgroundView_;
+
+ // The tab strip overlaps the titlebar of the window.
base::scoped_nsobject<TabStripView> tabStripView_;
+ // No views should be added directly to the root view. Views that overlap
+ // the title bar should be added to the window's contentView. All other views
+ // should be added to chromeContentView_. This allows tab dragging and
+ // fullscreen logic to easily move the views that don't need special
+ // treatment.
+ base::scoped_nsobject<NSView> chromeContentView_;
+
// The child window used during dragging to achieve the opacity tricks.
NSWindow* overlayWindow_;
@@ -40,6 +50,7 @@
@property(readonly, nonatomic) NSView* tabStripBackgroundView;
@property(readonly, nonatomic) TabStripView* tabStripView;
@property(readonly, nonatomic) FastResizeView* tabContentArea;
+@property(readonly, nonatomic) NSView* chromeContentView;
// This is the designated initializer for this class.
- (id)initTabWindowControllerWithTabStrip:(BOOL)hasTabStrip;
@@ -157,6 +168,12 @@
// The tab strip should always be inserted directly above the content view.
- (void)insertTabStripView:(NSView*)tabStripView intoWindow:(NSWindow*)window;
+// The tab strip background view should always be inserted as the back-most
+// subview of the root view. It cannot be a subview of the contentView, as that
+// would cause it to become layer backed, which would cause it to draw on top
+// of non-layer backed content like the window controls.
+- (void)insertTabStripBackgroundViewIntoWindow:(NSWindow*)window;
+
@end
@interface TabWindowController(ProtectedMethods)
diff --git a/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm b/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
index b5069cb..33f7fc0 100644
--- a/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
+++ b/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
@@ -15,8 +15,14 @@
#import "ui/base/cocoa/focus_tracker.h"
#include "ui/base/theme_provider.h"
-@interface TabWindowController(PRIVATE)
+@interface TabWindowController ()
- (void)setUseOverlay:(BOOL)useOverlay;
+
+// The tab strip background view should always be inserted as the back-most
+// subview of the root view. It cannot be a subview of the contentView, as that
+// would cause it to become layer backed, which would cause it to draw on top
+// of non-layer backed content like the window controls.
+- (void)insertTabStripBackgroundViewIntoWindow:(NSWindow*)window;
@end
@interface TabWindowOverlayWindow : NSWindow
@@ -49,7 +55,10 @@
@implementation TabWindowController
- (id)initTabWindowControllerWithTabStrip:(BOOL)hasTabStrip {
- NSRect contentRect = NSMakeRect(60, 229, 750, 600);
+ const CGFloat kDefaultWidth = 750;
+ const CGFloat kDefaultHeight = 600;
+
+ NSRect contentRect = NSMakeRect(60, 229, kDefaultWidth, kDefaultHeight);
base::scoped_nsobject<FramedBrowserWindow> window(
[[FramedBrowserWindow alloc] initWithContentRect:contentRect
hasTabStrip:hasTabStrip]);
@@ -59,11 +68,18 @@
if ((self = [super initWithWindow:window])) {
[[self window] setDelegate:self];
- tabContentArea_.reset([[FastResizeView alloc] initWithFrame:
- NSMakeRect(0, 0, 750, 600)]);
+ chromeContentView_.reset([[NSView alloc]
+ initWithFrame:NSMakeRect(0, 0, kDefaultWidth, kDefaultHeight)]);
+ [chromeContentView_
+ setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
+ [chromeContentView_ setWantsLayer:YES];
+ [[[self window] contentView] addSubview:chromeContentView_];
+
+ tabContentArea_.reset(
+ [[FastResizeView alloc] initWithFrame:[chromeContentView_ bounds]]);
[tabContentArea_ setAutoresizingMask:NSViewWidthSizable |
NSViewHeightSizable];
- [[[self window] contentView] addSubview:tabContentArea_];
+ [chromeContentView_ addSubview:tabContentArea_];
// tabStripBackgroundView_ draws the theme image behind the tab strip area.
// When making a tab dragging window (setUseOverlay:), this view stays in
@@ -78,14 +94,13 @@
kBrowserFrameViewPaintHeight)]);
[tabStripBackgroundView_
setAutoresizingMask:NSViewWidthSizable | NSViewMinYMargin];
- [windowView addSubview:tabStripBackgroundView_
- positioned:NSWindowBelow
- relativeTo:nil];
+ [self insertTabStripBackgroundViewIntoWindow:window];
[self moveContentViewToBack:[window contentView]];
tabStripView_.reset([[TabStripView alloc]
- initWithFrame:NSMakeRect(0, 0, 750, chrome::kTabStripHeight)]);
+ initWithFrame:NSMakeRect(
+ 0, 0, kDefaultWidth, chrome::kTabStripHeight)]);
[tabStripView_ setAutoresizingMask:NSViewWidthSizable |
NSViewMinYMargin];
if (hasTabStrip)
@@ -106,6 +121,10 @@
return tabContentArea_;
}
+- (NSView*)chromeContentView {
+ return chromeContentView_;
+}
+
- (void)removeOverlay {
[self setUseOverlay:NO];
if (closeDeferred_) {
@@ -141,8 +160,9 @@
[overlayWindow_ setBackgroundColor:[NSColor clearColor]];
[overlayWindow_ setOpaque:NO];
[overlayWindow_ setDelegate:self];
+ [[overlayWindow_ contentView] setWantsLayer:YES];
- originalContentView_ = [window contentView];
+ originalContentView_ = self.chromeContentView;
[window addChildWindow:overlayWindow_ ordered:NSWindowAbove];
// Explicitly set the responder to be nil here (for restoring later).
@@ -168,8 +188,10 @@
// places. The TabStripView always needs to be in front of the window's
// content view and therefore it should always be added after the content
// view is set.
- [window setContentView:originalContentView_];
- [self moveContentViewToBack:originalContentView_];
+ [[window contentView] addSubview:originalContentView_
+ positioned:NSWindowBelow
+ relativeTo:nil];
+ originalContentView_.frame = [[window contentView] bounds];
[self insertTabStripView:[self tabStripView] intoWindow:window];
[[window cr_windowView] updateTrackingAreas];
@@ -334,6 +356,14 @@
}
}
+- (void)insertTabStripBackgroundViewIntoWindow:(NSWindow*)window {
+ DCHECK(tabStripBackgroundView_);
+ NSView* rootView = [[window contentView] superview];
+ [rootView addSubview:tabStripBackgroundView_
+ positioned:NSWindowBelow
+ relativeTo:nil];
+}
+
// Called when the size of the window content area has changed. Override to
// position specific views. Base class implementation does nothing.
- (void)layoutSubviews {
diff --git a/chrome/browser/ui/cocoa/version_independent_window.h b/chrome/browser/ui/cocoa/version_independent_window.h
index 24acce1..afa4e48 100644
--- a/chrome/browser/ui/cocoa/version_independent_window.h
+++ b/chrome/browser/ui/cocoa/version_independent_window.h
@@ -19,35 +19,25 @@
@end
-// In OSX 10.10, adding subviews to the root view for the NSView hierarchy
-// produces warnings. To eliminate the warnings, we resize the contentView to
-// fill the window, and add subviews to that. When this class is used on OSX
-// 10.9 and lower, subviews are added directly to the root view, and the
-// contentView is not resized.
-// http://crbug.com/380412
+// By default, the contentView does not occupy the full size of a framed
+// window. Chrome still wants to draw in the title bar. Historically, Chrome
+// has done this by adding subviews directly to the root view. This causes
+// several problems. The most egregious is related to layer ordering when the
+// root view does not have a layer. By giving the contentView the same size as
+// the window, there is no longer any need to add subviews to the root view.
//
-// For code to be 10.9 and 10.10 compatible, views should be added to [window
-// cr_windowView] instead of [[window contentView] superview].
+// No views should be manually added to [[window contentView] superview].
+// Instead, they should be added to [window cr_windowView].
//
// If the window does not have a titlebar, then its contentView already has the
// same size as the window. In this case, this class has no effect.
//
// This class currently does not support changing the window's style after the
// window has been initialized.
-//
-// Since the contentView's size varies between OSes, several NSWindow methods
-// are no longer well defined.
-// - setContentSize:
-// - setContentMinSize:
-// - setContentMaxSize:
-// - setContentAspectRatio:
-// The implementation of this class on OSX 10.10 uses a hacked subclass of
-// NSView. It currently does not support the above 4 methods.
@interface VersionIndependentWindow : ChromeEventProcessingWindow {
@private
- // Holds the view that replaces [window contentView]. This view's size is the
- // same as the window's size.
- // Empty on 10.9 and lower, or if there is no titlebar.
+ // Holds the view that replaces [window contentView]. This view has the same
+ // size as the window. Empty if there is no titlebar.
base::scoped_nsobject<NSView> chromeWindowView_;
}
@@ -55,7 +45,8 @@
- (instancetype)initWithContentRect:(NSRect)contentRect
styleMask:(NSUInteger)windowStyle
backing:(NSBackingStoreType)bufferingType
- defer:(BOOL)deferCreation;
+ defer:(BOOL)deferCreation
+ wantsViewsOverTitlebar:(BOOL)wantsViewsOverTitlebar;
@end
diff --git a/chrome/browser/ui/cocoa/version_independent_window.mm b/chrome/browser/ui/cocoa/version_independent_window.mm
index 06a139e..570dc70 100644
--- a/chrome/browser/ui/cocoa/version_independent_window.mm
+++ b/chrome/browser/ui/cocoa/version_independent_window.mm
@@ -25,21 +25,14 @@
@implementation FullSizeContentView
-// This method is directly called by NSWindow during a window resize on OSX
-// 10.10.0, beta 2. We must override it to prevent the content view from
-// shrinking.
+// This method is directly called by AppKit during a live window resize.
+// Override it to prevent the content view from shrinking.
- (void)setFrameSize:(NSSize)size {
if ([self superview])
size = [[self superview] bounds].size;
[super setFrameSize:size];
}
-// The contentView gets moved around during certain full-screen operations.
-// This is less than ideal, and should eventually be removed.
-- (void)viewDidMoveToSuperview {
- [self setFrame:[[self superview] bounds]];
-}
-
@end
@implementation NSWindow (VersionIndependentWindow)
@@ -71,18 +64,31 @@
styleMask:(NSUInteger)windowStyle
backing:(NSBackingStoreType)bufferingType
defer:(BOOL)deferCreation {
+ return [self initWithContentRect:contentRect
+ styleMask:windowStyle
+ backing:bufferingType
+ defer:deferCreation
+ wantsViewsOverTitlebar:NO];
+}
+
+- (instancetype)initWithContentRect:(NSRect)contentRect
+ styleMask:(NSUInteger)windowStyle
+ backing:(NSBackingStoreType)bufferingType
+ defer:(BOOL)deferCreation
+ wantsViewsOverTitlebar:(BOOL)wantsViewsOverTitlebar {
self = [super initWithContentRect:contentRect
styleMask:windowStyle
backing:bufferingType
defer:deferCreation];
if (self) {
- if ([VersionIndependentWindow
- shouldUseFullSizeContentViewForStyle:windowStyle]) {
+ if (wantsViewsOverTitlebar &&
+ [VersionIndependentWindow
+ shouldUseFullSizeContentViewForStyle:windowStyle]) {
chromeWindowView_.reset([[FullSizeContentView alloc] init]);
[chromeWindowView_
setAutoresizingMask:NSViewWidthSizable | NSViewHeightSizable];
- [chromeWindowView_ setFrame:[[[self contentView] superview] bounds]];
[self setContentView:chromeWindowView_];
+ [chromeWindowView_ setFrame:[[chromeWindowView_ superview] bounds]];
}
}
return self;
@@ -91,13 +97,7 @@
#pragma mark - Private Methods
+ (BOOL)shouldUseFullSizeContentViewForStyle:(NSUInteger)windowStyle {
- // TODO(erikchen): Once OSX Yosemite is released, consider removing this
- // class entirely.
- // http://crbug.com/398574
- if (!CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kEnableFullSizeContentView))
- return NO;
- return (windowStyle & NSTitledWindowMask) && base::mac::IsOSYosemiteOrLater();
+ return windowStyle & NSTitledWindowMask;
}
- (NSView*)chromeWindowView {
@@ -106,30 +106,6 @@
#pragma mark - NSWindow Overrides
-#ifndef NDEBUG
-
-- (void)setContentSize:(NSSize)size {
- DCHECK(!chromeWindowView_);
- [super setContentSize:size];
-}
-
-- (void)setContentMinSize:(NSSize)size {
- DCHECK(!chromeWindowView_);
- [super setContentMinSize:size];
-}
-
-- (void)setContentMaxSize:(NSSize)size {
- DCHECK(!chromeWindowView_);
- [super setContentMaxSize:size];
-}
-
-- (void)setContentAspectRatio:(NSSize)ratio {
- DCHECK(!chromeWindowView_);
- [super setContentAspectRatio:ratio];
-}
-
-#endif // NDEBUG
-
+ (NSRect)frameRectForContentRect:(NSRect)cRect styleMask:(NSUInteger)aStyle {
if ([self shouldUseFullSizeContentViewForStyle:aStyle])
return cRect;
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index 1b94240..033dd09 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -1078,7 +1078,6 @@
'browser/ui/cocoa/passwords/manage_passwords_bubble_pending_view_controller_unittest.mm',
'browser/ui/cocoa/passwords/manage_passwords_controller_test.h',
'browser/ui/cocoa/passwords/manage_passwords_controller_test.mm',
- 'browser/ui/cocoa/presentation_mode_controller_unittest.mm',
'browser/ui/cocoa/profiles/avatar_button_controller_unittest.mm',
'browser/ui/cocoa/profiles/avatar_icon_controller_unittest.mm',
'browser/ui/cocoa/profiles/avatar_label_button_unittest.mm',
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index c40f52f..428b857 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1288,10 +1288,6 @@ const char kAppsKeepChromeAliveInTests[] = "apps-keep-chrome-alive-in-tests";
const char kDisableSystemFullscreenForTesting[] =
"disable-system-fullscreen-for-testing";
-// Makes the browser window's contentView take up the full size of the
-// window in OSX Yosemite.
-const char kEnableFullSizeContentView[] = "enable-full-size-content-view";
-
// A process type (switches::kProcessType) that relaunches the browser. See
// chrome/browser/mac/relauncher.h.
const char kRelauncherProcess[] = "relauncher";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 036674e..44851aa 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -367,7 +367,6 @@ extern const char kMigrateDataDirForSxS[];
#if defined(OS_MACOSX)
extern const char kAppsKeepChromeAliveInTests[];
extern const char kDisableSystemFullscreenForTesting[];
-extern const char kEnableFullSizeContentView[];
extern const char kRelauncherProcess[];
#endif