mac: Use a full-size content view (reland 1).

chrome/browser/ui/cocoa/browser_window_controller_browsertest.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.
 
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 
 #import "base/mac/mac_util.h"
 #include "base/mac/sdk_forward_declarations.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 205 matching lines @@
     NSPoint icon_bottom = location_bar_view->GetPageInfoBubblePoint();
 
     NSPoint info_bar_top = NSMakePoint(0,
         NSHeight([info_bar_container_controller view].frame) -
         overlapping_tip_height);
     info_bar_top = [[info_bar_container_controller view]
         convertPoint:info_bar_top toView:nil];
     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];
+  // Checks that |view| does not draw on top of |viewBeingVerified_|.
+  void CheckView(NSView* view) {
+    NSRect viewWindowFrame = [view convertRect:[view bounds] toView:nil];
+    NSRect viewBeingVerifiedWindowFrame =
+        [viewBeingVerified_ convertRect:[viewBeingVerified_ bounds] toView:nil];
 
-    EXPECT_EQ([controller() tabStripBackgroundView],
-              [subviews objectAtIndex:0]);
-    EXPECT_EQ(contentView, [subviews objectAtIndex:1]);
+    // The views do not intersect.
+    if (!NSIntersectsRect(viewBeingVerifiedWindowFrame, viewWindowFrame))
+      return;
 
-    NSView* tabStripView = [controller() tabStripView];
-    if ([subviews containsObject:tabStripView])
-      EXPECT_EQ(tabStripView, [subviews objectAtIndex:2]);
+    // No view can be above the view being verified.
+    EXPECT_TRUE(belowViewBeingVerified_);
+
+    // If |view| is a parent of |viewBeingVerified_|, then there's nothing else
+    // to check.
+    NSView* parent = viewBeingVerified_;
+    while (parent != nil) {
+      parent = [parent superview];
+      if (parent == view)
+        return;
+    }
+
+    if ([viewBeingVerified_ layer])
+      return;
+
+    // If the view being verified doesn't have a layer, then no views that
+    // intersect it can have a layer. The exceptions are the contentView,
+    // chromeContentView and tabStripView, which are layer backed but
+    // transparent.
+    std::vector<NSView*> exceptions;

[Andre, 2014/10/09 23:15:48]

How about something like this,
NSArray* exceptions = @[ [[view window] contentView], ....
if ([exceptions containsObject:view])
  EXPECT_FALSE([view isOpaque]);
else
  EXPECT_FALSE([view layer]);

[erikchen, 2014/10/10 00:47:45]

I used your suggestion of an array literal, although I decided to go with an
early return rather than an if/else.

+    exceptions.push_back([[view window] contentView]);
+    exceptions.push_back(controller().chromeContentView);
+    exceptions.push_back(controller().tabStripView);
+    for (NSView* exception : exceptions) {
+      if (view != exception)
+        continue;
+
+      EXPECT_FALSE([exception 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
+  // |viewBeingVerified_|.
+  void CheckViewAndSubviews(NSView* view) {
+    // If this is the view being verified, then there's nothing to check. Don't
+    // recurse into its subviews. All future views encountered in the recursion
+    // are above the view being verified.
+    if (view == viewBeingVerified_) {
+      belowViewBeingVerified_ = NO;
+      return;
+    }
+
+    CheckView(view);
+
+    // Perform the recursion.
+    for (NSView* subview in [view subviews])
+      CheckViewAndSubviews(subview);
+  }
+
+  void CheckViewOnTop(NSView* view) {
+    belowViewBeingVerified_ = YES;
+    viewBeingVerified_ = view;
+    CheckViewAndSubviews([[[view window] contentView] superview]);
+  }
+
+  // Nothing should draw on top of the window controls.
+  void VerifyWindowControlsZOrder() {
+    CheckViewOnTop(
+        [[controller() window] standardWindowButton:NSWindowCloseButton]);
+    CheckViewOnTop(
+        [[controller() window] standardWindowButton:NSWindowMiniaturizeButton]);
+
+    // There is no zoom button on OSX 10.10+.

[Andre, 2014/10/09 23:15:48]

I may have misremembered this, but I thought in 10.10 ZoomButton returns the
green fullscreen button while FullscreenButton returns nil.

[erikchen, 2014/10/10 00:47:45]

You're right. I was mistaken. Although the new nomenclature is really
confusing... :(

+    NSView* view =
+        [[controller() window] standardWindowButton:NSWindowZoomButton];
+    if (view)
+      CheckViewOnTop(view);
+
+    // There is no fullscreen button on OSX 10.6.
+    view =
+        [[controller() window] standardWindowButton:NSWindowFullScreenButton];
+    if (view)
+      CheckViewOnTop(view);
   }
 
  private:
+  NSView* viewBeingVerified_;
+  BOOL belowViewBeingVerified_;

[Andre, 2014/10/09 23:15:48]

Can we pass these as parameters and return values instead of using these
instance variables?
I think it will read better.

[erikchen, 2014/10/10 00:47:45]

I think that passing the parameters would reduce the readability. Instead, I
performed a significant refactor on all of the names. I also added many
comments.

+
   DISALLOW_COPY_AND_ASSIGN(BrowserWindowControllerTest);
 };
 
 // Tests that adding the first profile moves the Lion fullscreen button over
 // correctly.
 // DISABLED_ because it regularly times out: http://crbug.com/159002.
 IN_PROC_BROWSER_TEST_F(BrowserWindowControllerTest,
                        DISABLED_ProfileAvatarFullscreenButton) {
   if (base::mac::IsOSSnowLeopard())
     return;
@@ skipping 238 matching lines @@
 
   chrome::ExecuteCommand(browser(), IDC_SHOW_BOOKMARK_BAR);
   WaitForBookmarkBarAnimationToFinish();
   EXPECT_FALSE([controller() isBookmarkBarVisible]);
   EXPECT_EQ(std::min(GetExpectedTopInfoBarTipHeight(), max_tip_height),
             [[controller() infoBarContainerController] overlappingTipHeight]);
 }
 
 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();
 }