Consolidate PageInfo bubble anchoring logic.

Lots of things want to anchor off the page info icon. E.g.,
permission prompts, the device chooser UI, the first run bubble
and the page info bubble itself.

Unfortunately, the page info bubble is not always present (in
fullscreen for example), and we want to anchor toolkit-views bubbles
off a BrowserView as well as off a BrowserWindowCocoa. So anchoring
is not simple. Currently the logic that deals with this is scattered
around the codebase, and some of it is broken (e.g. crbug/745291).

This CL consolidates the anchoring for permission prompts and the
device chooser UI, for both of their Cocoa and Views versions. The
first run bubble will also use this in a follow-up.

The PageInfo bubble anchoring is a bit simpler since it can't be
shown in fullscreen. It's consolidated partially, but it has some
unnecessary plumbing as well, so more cleanups there are left for
another follow-up).

Bug: 728160, 745291
Change-Id: Id8b3bb9284f8dcc83299aa92123d4ee6e58dddc4
Reviewed-on: https://chromium-review.googlesource.com/575791
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488494}

chrome/browser/ui/BUILD.gn

@@ -1462,6 +1462,7 @@ split_static_library("ui") {
       "autofill/save_card_bubble_controller_impl.cc",
       "autofill/save_card_bubble_controller_impl.h",
       "autofill/save_card_bubble_view.h",
+      "bubble_anchor_util.h",
       "desktop_ios_promotion/desktop_ios_promotion_footnote_delegate.h",
 
       # This test header is included because it contains forward declarations
@@ -1497,6 +1498,7 @@ split_static_library("ui") {
       "views/bookmarks/bookmark_bubble_view.h",
       "views/bookmarks/bookmark_editor_view.cc",
       "views/bookmarks/bookmark_editor_view.h",
+      "views/bubble_anchor_util_views.h",
       "views/chrome_browser_main_extra_parts_views.cc",
       "views/chrome_browser_main_extra_parts_views.h",
       "views/chrome_constrained_window_views_client.cc",
@@ -1764,6 +1766,7 @@ split_static_library("ui") {
         "views/bookmarks/bookmark_menu_delegate.cc",
         "views/bookmarks/bookmark_menu_delegate.h",
         "views/browser_dialogs_views.cc",
+        "views/bubble_anchor_util_views.cc",
         "views/certificate_selector.cc",
         "views/certificate_selector.h",
         "views/certificate_viewer_win.cc",
@@ -2683,6 +2686,7 @@ split_static_library("ui") {
         "cocoa/browser_window_touch_bar.mm",
         "cocoa/browser_window_utils.h",
         "cocoa/browser_window_utils.mm",
+        "cocoa/bubble_anchor_util_views_mac.mm",
         "cocoa/bubble_combobox.h",
         "cocoa/bubble_combobox.mm",
         "cocoa/bubble_sync_promo_controller.h",

chrome/browser/ui/bubble_anchor_util.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_UI_BUBBLE_ANCHOR_UTIL_H_
+#define CHROME_BROWSER_UI_BUBBLE_ANCHOR_UTIL_H_
+
+namespace gfx {
+class Rect;
+}
+
+class Browser;
+
+namespace bubble_anchor_util {
+
+// Offset from the window edge to show bubbles when there is no location bar.
+// E.g., when in fullscreen or in a Hosted App window. Don't center, since that
+// could obscure a fullscreen bubble.
+constexpr int kNoToolbarLeftOffset = 40;
+
+// Returns the Rect appropriate for anchoring a bubble to |browser|'s Page Info
+// icon, or an appropriate fallback when that is not visible. This is used only
+// when the platform-specific GetPageInfoAnchorView() is unable to return an
+// actual View. This function has separate implementations for Views- and Cocoa-
+// based browsers. The anchor rect is in screen coordinates.
+gfx::Rect GetPageInfoAnchorRect(Browser* browser);
+
+}  // namespace bubble_anchor_util
+
+#endif  // CHROME_BROWSER_UI_BUBBLE_ANCHOR_UTIL_H_

chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc

@@ -7,6 +7,8 @@
 #include "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/bubble_anchor_util.h"
 #include "chrome/browser/ui/cocoa/browser_dialogs_views_mac.h"
 #include "chrome/browser/ui/cocoa/bubble_anchor_helper_views.h"
 #include "chrome/browser/ui/content_settings/content_setting_bubble_model.h"
@@ -26,13 +28,12 @@
 
 namespace chrome {
 
-void ShowPageInfoBubbleViewsAtPoint(
-    const gfx::Point& anchor_point,
+void ShowPageInfoBubbleViews(
+    gfx::NativeWindow browser_window,
     Profile* profile,
     content::WebContents* web_contents,
     const GURL& virtual_url,
-    const security_state::SecurityInfo& security_info,
-    LocationBarDecoration* decoration) {
+    const security_state::SecurityInfo& security_info) {
   // Don't show the bubble again if it's already showing. A second click on the
   // location icon in the omnibox will dismiss an open bubble. This behaviour is
   // consistent with the non-Mac views implementation.
@@ -45,10 +46,12 @@ void ShowPageInfoBubbleViewsAtPoint(
     return;
   }
 
-  views::BubbleDialogDelegateView* bubble = PageInfoBubbleView::ShowBubble(
-      nullptr, nullptr, gfx::Rect(anchor_point, gfx::Size()), profile,
+  Browser* browser = chrome::FindBrowserWithWindow(browser_window);
+  const gfx::Rect anchor = bubble_anchor_util::GetPageInfoAnchorRect(browser);
+  views::BubbleDialogDelegateView* bubble =
+      PageInfoBubbleView::ShowBubble(nullptr, nullptr, anchor, profile,
                                      web_contents, virtual_url, security_info);
-  KeepBubbleAnchored(bubble, decoration);
+  KeepBubbleAnchored(bubble, GetPageInfoDecoration(browser_window));
 }
 
 void ShowBookmarkBubbleViewsAtPoint(const gfx::Point& anchor_point,

chrome/browser/ui/cocoa/browser_dialogs_views_mac.h

@@ -31,14 +31,12 @@ struct SecurityInfo;
 
 namespace chrome {
 
-// Shows a Views page info bubble at the given anchor point.
-void ShowPageInfoBubbleViewsAtPoint(
-    const gfx::Point& anchor_point,
+// Shows a Views page info bubble on the given |browser_window|.
+void ShowPageInfoBubbleViews(gfx::NativeWindow browser_window,
                              Profile* profile,
                              content::WebContents* web_contents,
                              const GURL& virtual_url,
-    const security_state::SecurityInfo& security_info,
-    LocationBarDecoration* decoration);
+                             const security_state::SecurityInfo& security_info);
 
 // Show a Views bookmark bubble at the given point. This occurs when the
 // bookmark star is clicked or "Bookmark This Page..." is selected from a menu

chrome/browser/ui/cocoa/bubble_anchor_helper.h

@@ -15,7 +15,12 @@ bool HasVisibleLocationBarForBrowser(Browser* browser);
 // Returns a point screen coordinates at the bottom left of the location bar
 // when location bar is present and a point near the left edge of the screen
 // otherwise in order to not obscure the fullscreen request bubble.
-NSPoint GetPermissionBubbleAnchorPointForBrowser(Browser* browser,
+NSPoint GetPageInfoAnchorPointForBrowser(Browser* browser);
+
+// TODO(tapted): Remove this overload. It's needed for the unit tests for Cocoa
+// permission bubbles to force an anchoring type, which can be removed once
+// http://crbug.com/740827 is fixed.
+NSPoint GetPageInfoAnchorPointForBrowser(Browser* browser,
                                          bool has_location_bar);
 
 #endif  // CHROME_BROWSER_UI_COCOA_BUBBLE_ANCHOR_HELPER_H_

chrome/browser/ui/cocoa/bubble_anchor_helper.mm

@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/bubble_anchor_util.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_context.h"
@@ -18,14 +19,6 @@
 #include "components/prefs/pref_service.h"
 #import "ui/base/cocoa/cocoa_base_utils.h"
 
-namespace {
-
-// Offset from the screen edge to show dialogs when there is no location bar.
-// Don't center, since that could obscure a fullscreen bubble.
-constexpr NSInteger kFullscreenLeftOffset = 40;
-
-}  // namespace
-
 bool HasVisibleLocationBarForBrowser(Browser* browser) {
   if (!browser->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR))
     return false;
@@ -48,7 +41,12 @@ bool HasVisibleLocationBarForBrowser(Browser* browser) {
   return false;
 }
 
-NSPoint GetPermissionBubbleAnchorPointForBrowser(Browser* browser,
+NSPoint GetPageInfoAnchorPointForBrowser(Browser* browser) {
+  return GetPageInfoAnchorPointForBrowser(
+      browser, HasVisibleLocationBarForBrowser(browser));
+}
+
+NSPoint GetPageInfoAnchorPointForBrowser(Browser* browser,
                                          bool has_location_bar) {
   NSPoint anchor;
   NSWindow* parentWindow = browser->window()->GetNativeWindow();
@@ -60,7 +58,8 @@ NSPoint GetPermissionBubbleAnchorPointForBrowser(Browser* browser,
     // Position the bubble on the left of the screen if there is no page info
     // button to point at.
     NSRect contentFrame = [[parentWindow contentView] frame];
-    anchor = NSMakePoint(NSMinX(contentFrame) + kFullscreenLeftOffset,
+    anchor = NSMakePoint(
+        NSMinX(contentFrame) + bubble_anchor_util::kNoToolbarLeftOffset,
         NSMaxY(contentFrame));
   }
 

chrome/browser/ui/cocoa/bubble_anchor_helper_views.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_UI_COCOA_BUBBLE_ANCHOR_HELPER_VIEWS_H_
 #define CHROME_BROWSER_UI_COCOA_BUBBLE_ANCHOR_HELPER_VIEWS_H_
 
+#include "ui/gfx/native_widget_types.h"
+
 namespace views {
 class BubbleDialogDelegateView;
 }
@@ -12,8 +14,10 @@ class BubbleDialogDelegateView;
 class LocationBarDecoration;
 
 // Returns the manage password icon decoration in the omnibox.
-LocationBarDecoration* GetManagePasswordDecoration(
-    views::BubbleDialogDelegateView* bubble);
+LocationBarDecoration* GetManagePasswordDecoration(gfx::NativeWindow window);
+
+// Returns the page info decoration in the omnibox.
+LocationBarDecoration* GetPageInfoDecoration(gfx::NativeWindow window);
 
 // Monitors |bubble|'s parent window for size changes, and updates the bubble
 // anchor. The monitor will be deleted when |bubble| is closed. If |decoration|

chrome/browser/ui/cocoa/bubble_anchor_helper_views.mm

@@ -55,14 +55,20 @@ class BubbleAnchorHelper : public views::WidgetObserver {
 
 }  // namespace
 
-LocationBarDecoration* GetManagePasswordDecoration(
-    views::BubbleDialogDelegateView* bubble) {
-  BrowserWindowController* window_controller = [BrowserWindowController
-      browserWindowControllerForWindow:[bubble->parent_window() window]];
+LocationBarDecoration* GetManagePasswordDecoration(gfx::NativeWindow window) {
+  BrowserWindowController* window_controller =
+      [BrowserWindowController browserWindowControllerForWindow:window];
   LocationBarViewMac* location_bar = [window_controller locationBarBridge];
   return location_bar ? location_bar->manage_passwords_decoration() : nullptr;
 }
 
+LocationBarDecoration* GetPageInfoDecoration(gfx::NativeWindow window) {
+  BrowserWindowController* window_controller =
+      [BrowserWindowController browserWindowControllerForWindow:window];
+  LocationBarViewMac* location_bar = [window_controller locationBarBridge];
+  return location_bar ? location_bar->GetPageInfoDecoration() : nullptr;
+}
+
 void KeepBubbleAnchored(views::BubbleDialogDelegateView* bubble,
                         LocationBarDecoration* decoration) {
   new BubbleAnchorHelper(bubble, decoration);

chrome/browser/ui/cocoa/bubble_anchor_util_views_mac.mm

+// Copyright 2017 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/views/bubble_anchor_util_views.h"
+
+#include "chrome/browser/ui/cocoa/bubble_anchor_helper.h"
+#include "ui/gfx/geometry/rect.h"
+#import "ui/gfx/mac/coordinate_conversion.h"
+
+// This file contains the bubble_anchor_util implementation for
+// BrowserWindowCocoa.
+
+namespace bubble_anchor_util {
+
+gfx::Rect GetPageInfoAnchorRect(Browser* browser) {
+  // Note the Cocoa browser currently only offers anchor points, not rects.
+  return gfx::Rect(
+      gfx::ScreenPointFromNSPoint(GetPageInfoAnchorPointForBrowser(browser)),
+      gfx::Size());
+}
+
+// Stub implementation for a Cocoa browser window for calls coming from
+// chrome/browser/ui/views code. GetPageInfoAnchorRect() should always take
+// precedence.
+views::View* GetPageInfoAnchorView(Browser* browser) {
+  return nullptr;
+}
+
+}  // namespace bubble_anchor_util

chrome/browser/ui/cocoa/page_info/page_info_bubble_controller.mm

@@ -15,8 +15,10 @@
 #import "chrome/browser/certificate_viewer.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/cocoa/browser_dialogs_views_mac.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#include "chrome/browser/ui/cocoa/bubble_anchor_helper.h"
 #import "chrome/browser/ui/cocoa/info_bubble_view.h"
 #import "chrome/browser/ui/cocoa/info_bubble_window.h"
 #import "chrome/browser/ui/cocoa/location_bar/location_bar_decoration.h"
@@ -126,16 +128,9 @@ bool g_is_bubble_showing = false;
 // proper anchor point for the bubble. The returned point is in screen
 // coordinates.
 NSPoint AnchorPointForWindow(NSWindow* parent) {
-  BrowserWindowController* controller = [parent windowController];
-  NSPoint origin = NSZeroPoint;
-  if ([controller isKindOfClass:[BrowserWindowController class]]) {
-    LocationBarViewMac* location_bar = [controller locationBarBridge];
-    if (location_bar) {
-      NSPoint bubble_point = location_bar->GetPageInfoBubblePoint();
-      origin = ui::ConvertPointFromWindowToScreen(parent, bubble_point);
-    }
-  }
-  return origin;
+  Browser* browser = chrome::FindBrowserWithWindow(parent);
+  DCHECK(browser);
+  return GetPageInfoAnchorPointForBrowser(browser);
 }
 
 }  // namespace
@@ -1268,14 +1263,8 @@ void PageInfoUIBridge::Show(gfx::NativeWindow parent,
                             const GURL& virtual_url,
                             const security_state::SecurityInfo& security_info) {
   if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
-    BrowserWindowController* controller =
-        [BrowserWindowController browserWindowControllerForWindow:parent];
-    LocationBarViewMac* location_bar = [controller locationBarBridge];
-    LocationBarDecoration* decoration =
-        location_bar ? location_bar->GetPageInfoDecoration() : nullptr;
-    chrome::ShowPageInfoBubbleViewsAtPoint(
-        gfx::ScreenPointFromNSPoint(AnchorPointForWindow(parent)), profile,
-        web_contents, virtual_url, security_info, decoration);
+    chrome::ShowPageInfoBubbleViews(parent, profile, web_contents, virtual_url,
+                                    security_info);
     return;
   }
 

chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_cocoa.mm

@@ -58,10 +58,6 @@
 // Should only be used outside this class for tests.
 - (NSPoint)getExpectedAnchorPoint;
 
-// Returns true if the browser has support for the location bar.
-// Should only be used outside this class for tests.
-- (bool)hasLocationBar;
-
 // Update |tableView_| when chooser options changed.
 - (void)updateTableView;
 
@@ -220,16 +216,11 @@
 }
 
 - (NSPoint)getExpectedAnchorPoint {
-  return GetPermissionBubbleAnchorPointForBrowser(browser_,
-                                                  [self hasLocationBar]);
-}
-
-- (bool)hasLocationBar {
-  return HasVisibleLocationBarForBrowser(browser_);
+  return GetPageInfoAnchorPointForBrowser(browser_);
 }
 
 - (info_bubble::BubbleArrowLocation)getExpectedArrowLocation {
-  return [self hasLocationBar] ? info_bubble::kTopLeading
+  return HasVisibleLocationBarForBrowser(browser_) ? info_bubble::kTopLeading
                                                    : info_bubble::kNoArrow;
 }
 

chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_views_mac.mm

@@ -9,21 +9,12 @@
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/cocoa/browser_window_controller.h"
-#import "chrome/browser/ui/cocoa/bubble_anchor_helper.h"
 #import "chrome/browser/ui/cocoa/bubble_anchor_helper_views.h"
-#import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 #import "chrome/browser/ui/cocoa/permission_bubble/chooser_bubble_ui_cocoa.h"
 #include "chrome/browser/ui/permission_bubble/chooser_bubble_delegate.h"
-#import "ui/base/cocoa/cocoa_base_utils.h"
 #include "ui/base/material_design/material_design_controller.h"
-#import "ui/gfx/mac/coordinate_conversion.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
 
-// Implementation of ChooserBubbleUiView's anchor methods for Cocoa browsers. In
-// Cocoa browsers there is no parent views::View for the permission bubble, so
-// these methods supply an anchor point instead.
-
 std::unique_ptr<BubbleUi> ChooserBubbleDelegate::BuildBubbleUi() {
   if (!ui::MaterialDesignController::IsSecondaryUiMaterial()) {
     return base::MakeUnique<ChooserBubbleUiCocoa>(
@@ -34,26 +25,11 @@ std::unique_ptr<BubbleUi> ChooserBubbleDelegate::BuildBubbleUi() {
 }
 
 void ChooserBubbleUi::CreateAndShow(views::BubbleDialogDelegateView* delegate) {
-  // Set |parent_window_| because some valid anchors can become hidden.
-  gfx::NativeView parent =
-      platform_util::GetViewForWindow(browser_->window()->GetNativeWindow());
+  gfx::NativeWindow parent_window = browser_->window()->GetNativeWindow();
+  gfx::NativeView parent = platform_util::GetViewForWindow(parent_window);
   DCHECK(parent);
+  // Set |parent_window_| because some valid anchors can become hidden.
   delegate->set_parent_window(parent);
   views::BubbleDialogDelegateView::CreateBubble(delegate)->Show();
-  KeepBubbleAnchored(delegate);
-}
-
-views::View* ChooserBubbleUi::GetAnchorView() {
-  return nullptr;
-}
-
-gfx::Point ChooserBubbleUi::GetAnchorPoint() {
-  return gfx::ScreenPointFromNSPoint(GetPermissionBubbleAnchorPointForBrowser(
-      browser_, HasVisibleLocationBarForBrowser(browser_)));
-}
-
-views::BubbleBorder::Arrow ChooserBubbleUi::GetAnchorArrow() {
-  return HasVisibleLocationBarForBrowser(browser_)
-             ? views::BubbleBorder::TOP_LEFT
-             : views::BubbleBorder::NONE;
+  KeepBubbleAnchored(delegate, GetPageInfoDecoration(parent_window));
 }

chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm

@@ -150,7 +150,7 @@ const NSSize kPermissionIconSize = {18, 18};
 }
 
 + (NSPoint)getAnchorPointForBrowser:(Browser*)browser {
-  return GetPermissionBubbleAnchorPointForBrowser(
+  return GetPageInfoAnchorPointForBrowser(
       browser,
       [PermissionBubbleController hasVisibleLocationBarForBrowser:browser]);
 }

chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller_unittest.mm

@@ -15,6 +15,7 @@
 #include "chrome/browser/permissions/mock_permission_request.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/bubble_anchor_util.h"
 #include "chrome/browser/ui/cocoa/browser_window_controller.h"
 #include "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 #import "chrome/browser/ui/cocoa/location_bar/location_icon_decoration.h"
@@ -287,9 +288,6 @@ TEST_F(PermissionBubbleControllerTest, AnchorPositionWithLocationBar) {
 }
 
 TEST_F(PermissionBubbleControllerTest, AnchorPositionWithoutLocationBar) {
-  // See comment in bubble_anchor_helper.mm where this is defined.
-  const NSInteger kFullscreenLeftOffset = 40;
-
   base::mac::ScopedObjCClassSwizzler locationSwizzle(
       [PermissionBubbleController class], [MockBubbleNoLocationBar class],
       @selector(hasVisibleLocationBarForBrowser:));
@@ -299,8 +297,8 @@ TEST_F(PermissionBubbleControllerTest, AnchorPositionWithoutLocationBar) {
   // Expected anchor location will be top left when there's no location bar.
   NSWindow* window = browser()->window()->GetNativeWindow();
   NSRect frame = [[window contentView] frame];
-  NSPoint expected =
-      NSMakePoint(NSMinX(frame) + kFullscreenLeftOffset, NSMaxY(frame));
+  NSPoint expected = NSMakePoint(
+      NSMinX(frame) + bubble_anchor_util::kNoToolbarLeftOffset, NSMaxY(frame));
   expected = ui::ConvertPointFromWindowToScreen(window, expected);
   EXPECT_NSEQ(expected, anchor);
 }

chrome/browser/ui/cocoa/permission_bubble/permission_prompt_impl_views_mac.mm

@@ -3,35 +3,10 @@
 // found in the LICENSE file.
 
 #include "base/memory/ptr_util.h"
-#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/browser_window.h"
-#import "chrome/browser/ui/cocoa/browser_window_controller.h"
-#import "chrome/browser/ui/cocoa/bubble_anchor_helper.h"
-#import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 #import "chrome/browser/ui/cocoa/permission_bubble/permission_bubble_cocoa.h"
-#import "chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.h"
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h"
-#import "ui/base/cocoa/cocoa_base_utils.h"
 #include "ui/base/material_design/material_design_controller.h"
-#import "ui/gfx/mac/coordinate_conversion.h"
-
-// Implementation of PermissionPromptImpl's anchor methods for Cocoa
-// browsers. In Cocoa browsers there is no parent views::View for the permission
-// bubble, so these methods supply an anchor point instead.
-
-views::View* PermissionPromptImpl::GetAnchorView() {
-  return nullptr;
-}
-
-gfx::Point PermissionPromptImpl::GetAnchorPoint() {
-  return gfx::ScreenPointFromNSPoint(GetPermissionBubbleAnchorPointForBrowser(
-      browser_, HasVisibleLocationBarForBrowser(browser_)));
-}
-
-views::BubbleBorder::Arrow PermissionPromptImpl::GetAnchorArrow() {
-  return views::BubbleBorder::TOP_LEFT;
-}
 
 // static
 std::unique_ptr<PermissionPrompt> PermissionPrompt::Create(

chrome/browser/ui/cocoa/tab_dialogs_views_mac.mm

@@ -78,7 +78,7 @@ void TabDialogsViewsMac::ShowManagePasswordsBubble(bool user_action) {
   bubble_view->set_parent_window(parent);
   views::BubbleDialogDelegateView::CreateBubble(bubble_view);
   bubble_view->ShowForReason(reason);
-  KeepBubbleAnchored(bubble_view, GetManagePasswordDecoration(bubble_view));
+  KeepBubbleAnchored(bubble_view, GetManagePasswordDecoration(window));
 }
 
 void TabDialogsViewsMac::HideManagePasswordsBubble() {

chrome/browser/ui/views/bubble_anchor_util_views.cc

+// Copyright 2017 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/views/bubble_anchor_util_views.h"
+
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+
+// This file contains the bubble_anchor_util implementation for a Views
+// browser window (BrowserView).
+
+namespace bubble_anchor_util {
+
+views::View* GetPageInfoAnchorView(Browser* browser) {
+  if (!browser->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR))
+    return nullptr;  // Fall back to GetAnchorPoint().
+
+  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
+  return browser_view->GetLocationBarView()->GetSecurityBubbleAnchorView();
+}
+
+gfx::Rect GetPageInfoAnchorRect(Browser* browser) {
+  // GetPageInfoAnchorView() should be preferred when there is a location bar.
+  DCHECK(!browser->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR));
+
+  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
+  // Get position in view (taking RTL UI into account).
+  int x_within_browser_view = browser_view->GetMirroredXInView(
+      bubble_anchor_util::kNoToolbarLeftOffset);
+  // Get position in screen, taking browser view origin into account. This is
+  // 0,0 in fullscreen on the primary display, but not on secondary displays, or
+  // in Hosted App windows.
+  gfx::Point browser_view_origin = browser_view->GetBoundsInScreen().origin();
+  browser_view_origin += gfx::Vector2d(x_within_browser_view, 0);
+  return gfx::Rect(browser_view_origin, gfx::Size());
+}
+
+}  // namespace bubble_anchor_util

chrome/browser/ui/views/bubble_anchor_util_views.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_BUBBLE_ANCHOR_UTIL_VIEWS_H_
+#define CHROME_BROWSER_UI_VIEWS_BUBBLE_ANCHOR_UTIL_VIEWS_H_
+
+#include "chrome/browser/ui/bubble_anchor_util.h"
+
+namespace views {
+class View;
+}
+
+class Browser;
+
+namespace bubble_anchor_util {
+
+// Returns the PageInfo anchor View for |browser|, or null if it should not be
+// used.
+views::View* GetPageInfoAnchorView(Browser* browser);
+
+}  // namespace bubble_anchor_util
+
+#endif  // CHROME_BROWSER_UI_VIEWS_BUBBLE_ANCHOR_UTIL_VIEWS_H_

chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.cc

@@ -4,35 +4,40 @@
 
 #include "chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.h"
 
-#include <stddef.h>
-
-#include <memory>
-
-#include "base/macros.h"
-#include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/chooser_controller/chooser_controller.h"
-#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
-#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/permission_bubble/chooser_bubble_delegate.h"
+#include "chrome/browser/ui/views/bubble_anchor_util_views.h"
 #include "chrome/browser/ui/views/device_chooser_content_view.h"
 #include "components/bubble/bubble_controller.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
-#include "ui/views/controls/link.h"
 #include "ui/views/controls/styled_label.h"
 #include "ui/views/controls/table/table_view_observer.h"
 #include "ui/views/window/dialog_client_view.h"
 
+namespace {
+
+constexpr views::BubbleBorder::Arrow kChooserAnchorArrow =
+    views::BubbleBorder::TOP_LEFT;
+
+views::View* GetChooserAnchorView(Browser* browser) {
+  return bubble_anchor_util::GetPageInfoAnchorView(browser);
+}
+
+gfx::Rect GetChooserAnchorRect(Browser* browser) {
+  return bubble_anchor_util::GetPageInfoAnchorRect(browser);
+}
+
+}  // namespace
+
 ///////////////////////////////////////////////////////////////////////////////
 // View implementation for the chooser bubble.
 class ChooserBubbleUiViewDelegate : public views::BubbleDialogDelegateView,
                                     public views::TableViewObserver {
  public:
   ChooserBubbleUiViewDelegate(
-      views::View* anchor_view,
-      const gfx::Point& anchor_point,
-      views::BubbleBorder::Arrow anchor_arrow,
+      Browser* browser,
       std::unique_ptr<ChooserController> chooser_controller);
   ~ChooserBubbleUiViewDelegate() override;
 
@@ -57,8 +62,7 @@ class ChooserBubbleUiViewDelegate : public views::BubbleDialogDelegateView,
 
   // Updates the anchor's arrow and view. Also repositions the bubble so it's
   // displayed in the correct location.
-  void UpdateAnchor(views::View* anchor_view,
-                    views::BubbleBorder::Arrow anchor_arrow);
+  void UpdateAnchor(Browser* browser);
 
   void set_bubble_reference(BubbleReference bubble_reference);
   void UpdateTableView() const;
@@ -71,11 +75,10 @@ class ChooserBubbleUiViewDelegate : public views::BubbleDialogDelegateView,
 };
 
 ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
-    views::View* anchor_view,
-    const gfx::Point& anchor_point,
-    views::BubbleBorder::Arrow anchor_arrow,
+    Browser* browser,
     std::unique_ptr<ChooserController> chooser_controller)
-    : views::BubbleDialogDelegateView(anchor_view, anchor_arrow),
+    : views::BubbleDialogDelegateView(GetChooserAnchorView(browser),
+                                      kChooserAnchorArrow),
       device_chooser_content_view_(nullptr) {
   // ------------------------------------
   // | Chooser bubble title             |
@@ -94,8 +97,8 @@ ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
 
   device_chooser_content_view_ =
       new DeviceChooserContentView(this, std::move(chooser_controller));
-  if (!anchor_view)
-    SetAnchorRect(gfx::Rect(anchor_point, gfx::Size()));
+  if (!GetAnchorView())
+    SetAnchorRect(GetChooserAnchorRect(browser));
   chrome::RecordDialogCreation(chrome::DialogIdentifier::CHOOSER_UI);
 }
 
@@ -154,16 +157,11 @@ void ChooserBubbleUiViewDelegate::OnSelectionChanged() {
   GetDialogClientView()->UpdateDialogButtons();
 }
 
-void ChooserBubbleUiViewDelegate::UpdateAnchor(
-    views::View* anchor_view,
-    views::BubbleBorder::Arrow anchor_arrow) {
-  if (GetAnchorView() == anchor_view && arrow() == anchor_arrow)
-    return;
-
-  set_arrow(anchor_arrow);
-
-  // Reposition the bubble based on the updated arrow and view.
+void ChooserBubbleUiViewDelegate::UpdateAnchor(Browser* browser) {
+  views::View* anchor_view = GetChooserAnchorView(browser);
   SetAnchorView(anchor_view);
+  if (!anchor_view)
+    SetAnchorRect(GetChooserAnchorRect(browser));
 }
 
 void ChooserBubbleUiViewDelegate::set_bubble_reference(
@@ -184,9 +182,8 @@ ChooserBubbleUi::ChooserBubbleUi(
     : browser_(browser), chooser_bubble_ui_view_delegate_(nullptr) {
   DCHECK(browser_);
   DCHECK(chooser_controller);
-  chooser_bubble_ui_view_delegate_ = new ChooserBubbleUiViewDelegate(
-      GetAnchorView(), GetAnchorPoint(), GetAnchorArrow(),
-      std::move(chooser_controller));
+  chooser_bubble_ui_view_delegate_ =
+      new ChooserBubbleUiViewDelegate(browser, std::move(chooser_controller));
 }
 
 ChooserBubbleUi::~ChooserBubbleUi() {
@@ -211,10 +208,8 @@ void ChooserBubbleUi::Close() {
 }
 
 void ChooserBubbleUi::UpdateAnchorPosition() {
-  if (chooser_bubble_ui_view_delegate_) {
-    chooser_bubble_ui_view_delegate_->UpdateAnchor(GetAnchorView(),
-                                                   GetAnchorArrow());
-  }
+  if (chooser_bubble_ui_view_delegate_)
+    chooser_bubble_ui_view_delegate_->UpdateAnchor(browser_);
 }
 
 void ChooserBubbleUi::OnWidgetClosing(views::Widget* widget) {

chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.h

@@ -9,13 +9,10 @@
 
 #include "base/macros.h"
 #include "components/bubble/bubble_ui.h"
-#include "ui/gfx/geometry/point.h"
-#include "ui/views/bubble/bubble_border.h"
 #include "ui/views/widget/widget_observer.h"
 
 namespace views {
 class BubbleDialogDelegateView;
-class View;
 }
 
 class Browser;
@@ -42,12 +39,9 @@ class ChooserBubbleUi : public BubbleUi, public views::WidgetObserver {
   void OnWidgetClosing(views::Widget* widget) override;
 
  private:
-  // These functions have separate implementations for Views-based and
-  // Cocoa-based browsers, to allow this bubble to be used in either.
+  // Has separate implementations for Views-based and Cocoa-based browsers, to
+  // allow this bubble to be used in either.
   void CreateAndShow(views::BubbleDialogDelegateView* delegate);
-  views::View* GetAnchorView();
-  gfx::Point GetAnchorPoint();
-  views::BubbleBorder::Arrow GetAnchorArrow();
 
   Browser* browser_;  // Weak.
   // Weak. Owned by its parent view.

chrome/browser/ui/views/permission_bubble/chooser_bubble_ui_views.cc

@@ -4,18 +4,13 @@
 
 #include "chrome/browser/ui/views/permission_bubble/chooser_bubble_ui.h"
 
-#include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/chooser_controller/chooser_controller.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/permission_bubble/chooser_bubble_delegate.h"
-#include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
-#include "chrome/browser/ui/views/frame/top_container_view.h"
-#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
-#include "chrome/browser/ui/views/location_bar/location_icon_view.h"
-#include "ui/views/controls/image_view.h"
+#include "ui/views/bubble/bubble_dialog_delegate.h"
+#include "ui/views/widget/widget.h"
 
 // The Views browser implementation of ChooserBubbleUi's anchor methods.
 // Views browsers have a native View to anchor the bubble to, which these
@@ -35,27 +30,3 @@ void ChooserBubbleUi::CreateAndShow(views::BubbleDialogDelegateView* delegate) {
   delegate->set_parent_window(parent);
   views::BubbleDialogDelegateView::CreateBubble(delegate)->Show();
 }
-
-views::View* ChooserBubbleUi::GetAnchorView() {
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser_);
-
-  if (browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR)) {
-    return browser_view->GetLocationBarView()
-        ->location_icon_view()
-        ->GetImageView();
-  }
-  if (browser_view->IsFullscreenBubbleVisible())
-    return browser_view->exclusive_access_bubble()->GetView();
-
-  return browser_view->top_container();
-}
-
-gfx::Point ChooserBubbleUi::GetAnchorPoint() {
-  return gfx::Point();
-}
-
-views::BubbleBorder::Arrow ChooserBubbleUi::GetAnchorArrow() {
-  if (browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR))
-    return views::BubbleBorder::TOP_LEFT;
-  return views::BubbleBorder::NONE;
-}

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

@@ -6,42 +6,51 @@
 
 #include <stddef.h>
 
-#include "base/macros.h"
-#include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/permissions/permission_request.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
-#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/layout_constants.h"
-#include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
+#include "chrome/browser/ui/views/bubble_anchor_util_views.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row.h"
 #include "chrome/browser/ui/views/page_info/permission_selector_row_observer.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
-#include "ui/accessibility/ax_node_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/text_constants.h"
-#include "ui/views/background.h"
 #include "ui/views/bubble/bubble_dialog_delegate.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
-#include "ui/views/layout/grid_layout.h"
 
 namespace {
 
 // (Square) pixel size of icon.
-const int kIconSize = 18;
+constexpr int kIconSize = 18;
+
+// The type of arrow to display on the permission bubble.
+constexpr views::BubbleBorder::Arrow kPermissionAnchorArrow =
+    views::BubbleBorder::TOP_LEFT;
+
+// Returns the view to anchor the permission bubble to. May be null.
+views::View* GetPermissionAnchorView(Browser* browser) {
+  return bubble_anchor_util::GetPageInfoAnchorView(browser);
+}
+
+// Returns the anchor rect to anchor the permission bubble to, as a fallback.
+// Only used if GetPermissionAnchorView() returns nullptr.
+gfx::Rect GetPermissionAnchorRect(Browser* browser) {
+  return bubble_anchor_util::GetPageInfoAnchorRect(browser);
+}
 
 }  // namespace
 
@@ -71,11 +80,9 @@ class PermissionsBubbleDialogDelegateView
   base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
   void SizeToContents() override;
 
-  // Updates the anchor's arrow and view. Also repositions the bubble so it's
-  // displayed in the correct location.
-  void UpdateAnchor(views::View* anchor_view,
-                    const gfx::Point& anchor_point,
-                    views::BubbleBorder::Arrow anchor_arrow);
+  // Repositions the bubble so it's displayed in the correct location based on
+  // the updated anchor view, or anchor rect if that is (or became) null.
+  void UpdateAnchor();
 
  private:
   PermissionPromptImpl* owner_;
@@ -92,6 +99,7 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
   DCHECK(!requests.empty());
 
   set_close_on_deactivate(false);
+  set_arrow(kPermissionAnchorArrow);
 
   ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
   SetLayoutManager(new views::BoxLayout(
@@ -225,25 +233,11 @@ bool PermissionsBubbleDialogDelegateView::Close() {
   return true;
 }
 
-void PermissionsBubbleDialogDelegateView::UpdateAnchor(
-    views::View* anchor_view,
-    const gfx::Point& anchor_point,
-    views::BubbleBorder::Arrow anchor_arrow) {
-  set_arrow(anchor_arrow);
-
-  // Update the border in the bubble: will either add or remove the arrow.
-  views::BubbleFrameView* frame =
-      views::BubbleDialogDelegateView::GetBubbleFrameView();
-  views::BubbleBorder::Arrow adjusted_arrow = anchor_arrow;
-  if (base::i18n::IsRTL())
-    adjusted_arrow = views::BubbleBorder::horizontal_mirror(adjusted_arrow);
-  frame->SetBubbleBorder(std::unique_ptr<views::BubbleBorder>(
-      new views::BubbleBorder(adjusted_arrow, shadow(), color())));
-
-  // Reposition the bubble based on the updated arrow and view.
+void PermissionsBubbleDialogDelegateView::UpdateAnchor() {
+  views::View* anchor_view = GetPermissionAnchorView(owner_->browser());
   SetAnchorView(anchor_view);
-  // The anchor rect is ignored unless |anchor_view| is nullptr.
-  SetAnchorRect(gfx::Rect(anchor_point, gfx::Size()));
+  if (!anchor_view)
+    SetAnchorRect(GetPermissionAnchorRect(owner_->browser()));
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -270,9 +264,7 @@ void PermissionPromptImpl::UpdateAnchorPosition() {
   if (bubble_delegate_) {
     bubble_delegate_->set_parent_window(
         platform_util::GetViewForWindow(browser_->window()->GetNativeWindow()));
-    bubble_delegate_->UpdateAnchor(GetAnchorView(),
-                                   GetAnchorPoint(),
-                                   GetAnchorArrow());
+    bubble_delegate_->UpdateAnchor();
   }
 }
 
@@ -331,6 +323,5 @@ void PermissionPromptImpl::Show() {
 
   bubble_delegate_->SizeToContents();
 
-  bubble_delegate_->UpdateAnchor(GetAnchorView(), GetAnchorPoint(),
-                                 GetAnchorArrow());
+  bubble_delegate_->UpdateAnchor();
 }

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h

@@ -5,18 +5,8 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_PERMISSION_BUBBLE_PERMISSION_PROMPT_IMPL_H_
 #define CHROME_BROWSER_UI_VIEWS_PERMISSION_BUBBLE_PERMISSION_PROMPT_IMPL_H_
 
-#include <memory>
-#include <string>
-
-#include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "chrome/browser/ui/permission_bubble/permission_prompt.h"
-#include "ui/gfx/geometry/point.h"
-#include "ui/views/bubble/bubble_border.h"
-
-namespace views {
-class View;
-}
 
 class Browser;
 class PermissionsBubbleDialogDelegateView;
@@ -36,20 +26,11 @@ class PermissionPromptImpl : public PermissionPrompt {
   void Accept();
   void Deny();
 
+  Browser* browser() { return browser_; }
+
  private:
   void Show();
 
-  // These three functions have separate implementations for Views-based and
-  // Cocoa-based browsers, to allow this bubble to be used in either.
-
-  // Returns the view to anchor the permission bubble to. May be null.
-  views::View* GetAnchorView();
-  // Returns the anchor point to anchor the permission bubble to, as a fallback.
-  // Only used if GetAnchorView() returns nullptr.
-  gfx::Point GetAnchorPoint();
-  // Returns the type of arrow to display on the permission bubble.
-  views::BubbleBorder::Arrow GetAnchorArrow();
-
   Browser* browser_;
   Delegate* delegate_;
   PermissionsBubbleDialogDelegateView* bubble_delegate_;

chrome/browser/ui/views/permission_bubble/permission_prompt_impl_views.cc

@@ -3,50 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/memory/ptr_util.h"
-#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/browser_window.h"
-#include "chrome/browser/ui/views/exclusive_access_bubble_views.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
-#include "chrome/browser/ui/views/frame/top_container_view.h"
-#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
-#include "chrome/browser/ui/views/location_bar/location_icon_view.h"
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h"
-#include "content/public/browser/web_contents.h"
-#include "ui/gfx/geometry/point.h"
-#include "ui/gfx/geometry/vector2d.h"
-#include "ui/views/controls/image_view.h"
-
-// The Views browser implementation of PermissionPromptImpl's
-// anchor methods. Views browsers have a native View to anchor the bubble to,
-// which these functions provide.
-
-// Left margin for the bubble when anchored to the top of the screen in
-// fullscreen mode.
-const int kFullscreenLeftMargin = 40;
-
-views::View* PermissionPromptImpl::GetAnchorView() {
-  if (!browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR))
-    return nullptr;  // Fall back to GetAnchorPoint().
-
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser_);
-  return browser_view->GetLocationBarView()->GetSecurityBubbleAnchorView();
-}
-
-gfx::Point PermissionPromptImpl::GetAnchorPoint() {
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser_);
-  // Get position in view (taking RTL displays into account).
-  int x_within_browser_view =
-      browser_view->GetMirroredXInView(kFullscreenLeftMargin);
-  // Get position in screen (taking browser view origin into account, which may
-  // not be 0,0 if there are multiple displays).
-  gfx::Point browser_view_origin = browser_view->GetBoundsInScreen().origin();
-  return browser_view_origin + gfx::Vector2d(x_within_browser_view, 0);
-}
-
-views::BubbleBorder::Arrow PermissionPromptImpl::GetAnchorArrow() {
-  return views::BubbleBorder::TOP_LEFT;
-}
 
 // static
 std::unique_ptr<PermissionPrompt> PermissionPrompt::Create(