Center permission bubble if location bar is hidden in MacOS.

chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_browsertest.mm

Index: chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_browsertest.mm
diff --git a/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_browsertest.mm b/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_browsertest.mm
new file mode 100644
index 0000000000000000000000000000000000000000..72c93aac13dbcfaecb07b13fae9373b94bd31e6f
--- /dev/null
+++ b/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa_browsertest.mm
@@ -0,0 +1,148 @@
+// Copyright 2015 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.
+
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#import "chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.h"
+#include "chrome/browser/ui/test/permission_bubble_browsertest.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#import "ui/base/cocoa/fullscreen_window_manager.h"
+

[groby-ooo-7-16, 2015/04/08 18:39:15]

Put all this (constants, test helper classes, static function) into an anonymous
namespace, please.

[hcarmona, 2015/04/10 18:54:07]

Done.

+// To be used when getting the anchor with a location bar. The bubble should be
+// in the top-right corner.
+const int kTolerance = 50;

[groby-ooo-7-16, 2015/04/08 18:39:15]

Still a huge tolerance. Why so big?

[hcarmona, 2015/04/10 18:54:07]

I've changed my expected value to be exactly the anchor and the test expects it
to be within 5 pts.
This test seems like it could fail very easily just by moving the anchor a
little. I also don't want to
get a UI element and figure out the exact point programmatically because then I
feel like it's testing
the implementation rather than the behavior.

Suggestions?

+
+class TestPermissionBubbleCocoa : public PermissionBubbleCocoa {
+ public:
+  TestPermissionBubbleCocoa(NSWindow* parent_window)
+      : PermissionBubbleCocoa(parent_window) {}
+
+  // Returns the base class value for testing |HasLocationBar|.
+  bool HasLocationBar() override {

[groby-ooo-7-16, 2015/04/08 18:39:15]

You could just friend the test, saving you a lot of pain.
(FRIEND_TEST_ALL_PREFIXES)

[hcarmona, 2015/04/10 18:54:07]

Friended.

+    return PermissionBubbleCocoa::HasLocationBar();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestPermissionBubbleCocoa);
+};
+
+class MockPermissionBubbleCocoa : public PermissionBubbleCocoa {
+ public:
+  MockPermissionBubbleCocoa(NSWindow* parent_window)
+      : PermissionBubbleCocoa(parent_window) {}
+
+  MOCK_METHOD0(HasLocationBar, bool());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockPermissionBubbleCocoa);
+};
+
+static void CloseController(Browser* browser) {

[groby-ooo-7-16, 2015/04/08 18:39:15]

No need to be static in anon namespace.

[hcarmona, 2015/04/10 18:54:07]

Done.

+  browser->tab_strip_model()->CloseSelectedTabs();
+
+  NSWindow* native_window = browser->window()->GetNativeWindow();
+  BrowserWindowController* controller =
+      [BrowserWindowController browserWindowControllerForWindow:native_window];
+
+  [controller close];
+}
+
+// Default Window Test /////////////////////////////////////////////////////////

[groby-ooo-7-16, 2015/04/08 18:39:15]

I'd get rid of these separators - the test names should already express that.

[hcarmona, 2015/04/10 18:54:07]

Done.

+
+IN_PROC_BROWSER_TEST_F(PermissionBubbleBrowsertest, HasLocationBarByDefault) {
+  TestPermissionBubbleCocoa bubble(browser()->window()->GetNativeWindow());
+  bubble.SetDelegate(test_delegate());
+  bubble.Show(requests(), accept_states());
+  EXPECT_TRUE(bubble.HasLocationBar());
+  CloseController(browser());

[groby-ooo-7-16, 2015/04/08 18:39:15]

Why do we need this at all? Shouldn't browser tests close everything?

[hcarmona, 2015/04/10 18:54:07]

I remember having issues without it, but everything is working fine after
removing it.
Maybe I had a different issue that I've since fixed.

Done, no more |CloseController|.

+}
+
+// Fullscreen Test /////////////////////////////////////////////////////////////
+
+IN_PROC_BROWSER_TEST_F(PermissionBubbleBrowsertest, FullscreenHasLocationBar) {
+  TestPermissionBubbleCocoa bubble(browser()->window()->GetNativeWindow());
+  bubble.SetDelegate(test_delegate());
+  bubble.Show(requests(), accept_states());
+
+  NSWindow* window = browser()->window()->GetNativeWindow();
+  base::scoped_nsobject<FullscreenWindowManager> manager(
+      [[FullscreenWindowManager alloc] initWithWindow:window
+                                        desiredScreen:[NSScreen mainScreen]]);
+  EXPECT_TRUE(bubble.HasLocationBar());
+  [manager enterFullscreenMode];
+  EXPECT_TRUE(bubble.HasLocationBar());
+  [manager exitFullscreenMode];
+  EXPECT_TRUE(bubble.HasLocationBar());
+  CloseController(browser());
+}
+
+// App Test ////////////////////////////////////////////////////////////////////
+
+IN_PROC_BROWSER_TEST_F(PermissionBubbleAppBrowsertest, AppHasNoLocationBar) {
+  TestPermissionBubbleCocoa bubble(app_browser()->window()->GetNativeWindow());
+  bubble.SetDelegate(test_delegate());
+  bubble.Show(requests(), accept_states());
+  EXPECT_FALSE(bubble.HasLocationBar());
+  CloseController(app_browser());
+}
+
+// Kiosk Test //////////////////////////////////////////////////////////////////
+
+// http://crbug.com/470724
+// Kiosk mode on Mac has a location bar but it shouldn't.
+IN_PROC_BROWSER_TEST_F(PermissionBubbleKioskBrowsertest,
+                       DISABLED_KioskHasNoLocationBar) {
+  TestPermissionBubbleCocoa bubble(browser()->window()->GetNativeWindow());
+  bubble.SetDelegate(test_delegate());
+  bubble.Show(requests(), accept_states());
+  EXPECT_FALSE(bubble.HasLocationBar());
+  CloseController(browser());
+}
+
+// Anchor Position Tests ///////////////////////////////////////////////////////
+
+IN_PROC_BROWSER_TEST_F(PermissionBubbleBrowsertest,
+                       AnchorPositionWithLocationBar) {
+  testing::NiceMock<MockPermissionBubbleCocoa> bubble(
+      browser()->window()->GetNativeWindow());
+  bubble.SetDelegate(test_delegate());
+  bubble.Show(requests(), accept_states());
+  ON_CALL(bubble, HasLocationBar()).WillByDefault(testing::Return(true));
+
+  NSPoint anchor = bubble.GetAnchorPoint();
+  NSWindow* window = browser()->window()->GetNativeWindow();
+  NSRect frame = [window frame];
+
+  // Expected anchor location will be top left when there's a location bar.
+  NSPoint expected = NSMakePoint(NSMinX(frame) + 100, NSMaxY(frame) - 25);
+
+  // Anchor should be close to the left of the screen.
+  ASSERT_NEAR(expected.x, anchor.x, kTolerance);
+
+  // Anchor should be close to the top of the screen.
+  ASSERT_NEAR(expected.y, anchor.y, kTolerance);
+  CloseController(browser());
+}
+
+IN_PROC_BROWSER_TEST_F(PermissionBubbleBrowsertest,
+                       AnchorPositionWithoutLocationBar) {
+  testing::NiceMock<MockPermissionBubbleCocoa> bubble(
+      browser()->window()->GetNativeWindow());
+  bubble.SetDelegate(test_delegate());
+  bubble.Show(requests(), accept_states());
+  ON_CALL(bubble, HasLocationBar()).WillByDefault(testing::Return(false));
+
+  NSPoint anchor = bubble.GetAnchorPoint();
+  NSWindow* window = browser()->window()->GetNativeWindow();
+  NSRect frame = [window frame];
+
+  // Expected anchor location will be top center when there's no location bar.
+  NSPoint expected = NSMakePoint(frame.size.width / 2, frame.size.height);
+  expected = [window convertBaseToScreen:expected];
+
+  // Anchor should be centered.
+  EXPECT_TRUE(NSEqualPoints(expected, anchor));
+  CloseController(browser());
+}