[Mac] Turn on Auto Layout for browser windows.

chrome/browser/ui/cocoa/chrome_browser_window_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 <Cocoa/Cocoa.h>
 
 #include "base/debug/debugger.h"
 #include "chrome/app/chrome_command_ids.h"
 #import "chrome/browser/ui/cocoa/chrome_browser_window.h"
 #import "chrome/browser/ui/cocoa/test/cocoa_test_helper.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
 
 class ChromeBrowserWindowTest : public CocoaTest {
  public:
   void SetUp() override {
     CocoaTest::SetUp();
     // Create a window.
     const NSUInteger mask = NSTitledWindowMask | NSClosableWindowMask |
         NSMiniaturizableWindowMask | NSResizableWindowMask;
     window_ = [[ChromeBrowserWindow alloc]
-               initWithContentRect:NSMakeRect(0, 0, 800, 600)
-               styleMask:mask
-               backing:NSBackingStoreBuffered
-               defer:NO];
+           initWithContentRect:NSMakeRect(0, 0, 800, 600)
+                     styleMask:mask
+                       backing:NSBackingStoreBuffered
+                         defer:NO
+        wantsViewsOverTitlebar:YES];

[tapted, 2017/03/21 02:16:51]

Is this needed? It suggests that passing `NO` will result in a window that does
not want auto layout. We pass `NO` when there is no tab strip, which can happen
for popups, devtools, packaged apps. I think for the reasons you point out, we
wouldn't want these windows to behave differently in aspects of layout versus
regular browser windows. E.g. packaged apps want to do similar things; drawing
over the titlebar sometimes (and don't use BorderlessWindowMask) -- see
NativeAppWindowCocoa::InstallView()  (I think we just mash a WebContentsView
into the contentView in that case, but it's the altered codepaths for resizing
the WebContentsView that required the fix in http://crbug.com/655112#c16 -- I
think we'd want the code touched there to be able to make the same assumptions
about the layout codepaths.

[Sidney San Martín, 2017/03/22 21:40:22]

Where we force Auto Layout is somewhat arbitrary. Not *every* window should be
opted in, since that includes bubbles, dialogs, and menus, but you're right that
every kind of browser window should be.

I moved +requiresConstraintBasedLayout to the window controller's
chromeContentView (which turns out to be created by TabWindowController, which
even non-tabbed browser windows use, which is still odd but outside the scope of
this CL…). Let me know how that sounds.

     if (base::debug::BeingDebugged()) {
       [window_ orderFront:nil];
     } else {
       [window_ orderBack:nil];
     }
   }
 
   void TearDown() override {
     [window_ close];
     CocoaTest::TearDown();
   }
 
   ChromeBrowserWindow* window_;
 };
 
 // Baseline test that the window creates, displays, closes, and
 // releases.
 TEST_F(ChromeBrowserWindowTest, ShowAndClose) {
   [window_ display];
 }
 
+// Verify that browser windows use Auto Layout. Since frame-based layout and

[tapted, 2017/03/21 02:16:51]

This is super nit-picky, but I'm not sure if this test is verifying that browser
windows use AutoLayout so much as verifying that the ChromeBrowserWindowTest
test harness creates a window that uses autolayout. 

Perhaps just a re-wording is appropriate here. E.g. "Verify that passing
wantsViewsOverTitlebar:YES to the  ChromeBrowserWindow initializer results in a
window that uses Auto Layout."

[Sidney San Martín, 2017/03/22 21:40:22]

Heh, great point :). Re-wording the comment like that would have shown that the
test wasn't meaningful. I moved it to BrowserWindowController's unit test, even
though the change is in its superclass, to honor the original intent: browser
windows should use Auto Layout.

+// Auto Layout behave differently in subtle ways, we shouldn't start/stop using
+// it accidentally. If we don't want Auto Layout, this test should be changed
+// to expect constraints.count == 0, not disabled.
+TEST_F(ChromeBrowserWindowTest, UsesAutoLayout) {
+  base::scoped_nsobject<NSView> view(

[tapted, 2017/03/21 02:16:51]

Comment why the extra subview is necessary for the test?

[Sidney San Martín, 2017/03/22 21:40:22]

Gone.

+      [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 10, 10)]);
+  [view setAutoresizingMask:NSViewWidthSizable];
+  [window_.contentView addSubview:view];

[tapted, 2017/03/21 02:16:51]

nit: [[window contentView] addSubview: view]

[Sidney San Martín, 2017/03/22 21:40:22]

Gone.

+  [window_ orderFront:nil];
+  [window_ layoutIfNeeded];
+  EXPECT_TRUE([[[window_ contentView] constraints] count] > 0);

[tapted, 2017/03/21 02:16:50]

nit:

EXPECT_LT(0u, [[[window_ contentView] constraints] count]);

[Sidney San Martín, 2017/03/22 21:40:21]

Oof, `EXPECT_LT(0u, count)` seems harder to understand. Since it's unsigned, how
about `EXPECT_NE(0u, count)`?

[tapted, 2017/03/22 22:40:00]

looks good!

+}