mac: Toolbar renders incorrectly when downloads bar is open.

chrome/browser/ui/cocoa/browser_window_controller_unittest.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"
 
 #include "base/mac/mac_util.h"
 #import "base/mac/scoped_nsobject.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/signin/fake_signin_manager.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/profile_sync_service_mock.h"
 #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"

[Scott Hess - ex-Googler, 2014/07/25 23:59:05]

This seems unlikely, now that I look at it.

[erikchen, 2014/07/26 00:12:16]

Won't compile without it. FastResizeView is forward declared, and can't be
casted to NSView.

[Scott Hess - ex-Googler, 2014/07/26 00:21:27]

Weird.  So I'm guessing what you mean is that [controller tabContentArea]'s
result can't be passed to FrameInWindowForView() because it doesn't know that
it's a subclass of NSView, but it is able to figure out to call
objc_msgSend_stret because of the NSRect assignment?  Weirdo.  I wonder if
there's a warning we could be turning on to flush out that kind of ambiguity.

 #include "chrome/browser/ui/cocoa/find_bar/find_bar_bridge.h"
 #import "chrome/browser/ui/cocoa/nsview_additions.h"
 #include "chrome/browser/ui/cocoa/tabs/tab_strip_view.h"
 #import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/signin/core/browser/fake_auth_status_provider.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_error_controller.h"
@@ skipping 202 matching lines @@
   [controller_ installIncognitoBadge];
   NSRect newTabFrame = [[controller_ tabStripView] frame];
   EXPECT_GT(tabFrame.size.width, newTabFrame.size.width);
 
   controller_.release();
 }
 #endif
 
 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 true;
+
+  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 false;
+  }
+
+  for (NSView* subview in [view subviews]) {
+    BOOL result = ViewHierarchyContainmentValid(subview);
+    if (!result)
+      return false;
+  }
+
+  return true;
+}
+
 // 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];
-  NSRect download = [[[controller downloadShelf] view] 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 = FrameInWindowForView([[controller downloadShelf] view]);
+
+  // The height of the contentView varies between full height of the window
+  // (OSX 10.10+) and full height minus 22 points. Don't make any checks
+  // against NSMaxY of contentView.

[Scott Hess - ex-Googler, 2014/07/25 23:59:05]

I don't think this comment helps, since this code isn't checking the window
frame at all.  I think the comment about the tabstrip check will have to
represent the concept.

[erikchen, 2014/07/26 00:12:16]

Removed the comment.

   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));
+
+  // Toolbar should start immediately under the tabstrip, but the tabstrip is
+  // not necessarily fixed with respect to the content view.
+  EXPECT_EQ(NSMaxY(toolbar), NSMinY(tabstrip));
 
   // Bookmark bar frame is random memory when hidden.
   if ([controller bookmarkBarVisible]) {
     NSRect bookmark = [[controller bookmarkView] frame];
     EXPECT_EQ(NSMaxY(infobar), NSMinY(bookmark));
     EXPECT_EQ(NSMaxY(bookmark), NSMinY(toolbar));
     EXPECT_FALSE([[controller bookmarkView] isHidden]);
   } else {
     EXPECT_EQ(NSMaxY(infobar), NSMinY(toolbar));
     EXPECT_TRUE([[controller bookmarkView] isHidden]);
   }
-
-  // Toolbar should start immediately under the tabstrip, but the tabstrip is
-  // not necessarily fixed with respect to the content view.
-  EXPECT_EQ(NSMinY(tabstrip), NSMaxY(toolbar));
 }
 
 }  // end namespace
 
 TEST_F(BrowserWindowControllerTest, TestAdjustWindowHeight) {
   NSWindow* window = [controller_ window];
   NSRect workarea = [[window screen] visibleFrame];
 
   // Place the window well above the bottom of the screen and try to adjust its
   // height. It should change appropriately (and only downwards). Then get it to
@@ skipping 598 matching lines @@
       [[NSWindow alloc] initWithContentRect:NSMakeRect(0,0,400,400)
                                   styleMask:NSBorderlessWindowMask
                                     backing:NSBackingStoreBuffered
                                       defer:NO]);
   [[testFullscreenWindow_ contentView] cr_setWantsLayer:YES];
   return testFullscreenWindow_.get();
 }
 @end
 
 /* TODO(???): test other methods of BrowserWindowController */