Make page info and permission bubbles anchor from the top right in hosted app windows

This CL fixes a bug where permission prompts and page info dialogs go off screen when
hosted app windows are maximised. This only affects platforms that don't support bubble
anchor adjustment (see calls to set_adjust_if_offscreen(false) in
BubbleDialogDelegateView::CreateBubble()). These platforms are Linux, Mac and Windows 7
with Aero disabled.

This patch fixes the issue by making permission and page info dialogs anchored off
the app menu use TOP_RIGHT instead of TOP_LEFT for their arrow position leading the
bubble to position itself within the main window's bounds by default.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355950&signed_aid=ziHkOFUJALIjs_7uaUaPtQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=355951&signed_aid=9B2p0GLHiuHOMC-Ha-rXBQ==&inline=1

Bug: 877032
Change-Id: I98b1fe1e4fe8018886ae960c8601f5d518c9ea5e
Reviewed-on: https://chromium-review.googlesource.com/1201507
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589455}

chrome/browser/ui/cocoa/browser_dialogs_views_mac.cc

@@ -57,17 +57,19 @@ void ShowPageInfoBubbleViews(Browser* browser,
     return;
   }
 
-  views::View* anchor_view =
-      bubble_anchor_util::GetPageInfoAnchorView(browser, anchor);
+  bubble_anchor_util::AnchorConfiguration configuration =
+      bubble_anchor_util::GetPageInfoAnchorConfiguration(browser, anchor);
   gfx::Rect anchor_rect =
-      anchor_view ? gfx::Rect()
-                  : bubble_anchor_util::GetPageInfoAnchorRect(browser);
+      configuration.anchor_view
+          ? gfx::Rect()
+          : bubble_anchor_util::GetPageInfoAnchorRect(browser);
   gfx::NativeWindow parent_window = browser->window()->GetNativeWindow();
   views::BubbleDialogDelegateView* bubble =
       PageInfoBubbleView::CreatePageInfoBubble(
-          anchor_view, anchor_rect, parent_window, browser->profile(),
-          web_contents, virtual_url, security_info);
+          configuration.anchor_view, anchor_rect, parent_window,
+          browser->profile(), web_contents, virtual_url, security_info);
   bubble->GetWidget()->Show();
+  bubble->set_arrow(configuration.bubble_arrow);
   KeepBubbleAnchored(bubble, GetPageInfoDecoration(parent_window));
 }
 

chrome/browser/ui/views/bubble_anchor_util_views.cc

@@ -16,22 +16,24 @@
 
 namespace bubble_anchor_util {
 
-views::View* GetPageInfoAnchorView(Browser* browser, Anchor anchor) {
+AnchorConfiguration GetPageInfoAnchorConfiguration(Browser* browser,
+                                                   Anchor anchor) {
 #if defined(OS_MACOSX)
   if (views_mode_controller::IsViewsBrowserCocoa())
-    return nullptr;
+    return {};
 #endif
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
 
   if (anchor == kLocationBar && browser_view->GetLocationBarView()->IsDrawn())
-    return browser_view->GetLocationBarView()->GetSecurityBubbleAnchorView();
+    return {browser_view->GetLocationBarView()->GetSecurityBubbleAnchorView(),
+            views::BubbleBorder::TOP_LEFT};
   // Fall back to menu button if no location bar present.
 
   views::View* app_menu_button =
       browser_view->toolbar_button_provider()->GetAppMenuButton();
   if (app_menu_button && app_menu_button->IsDrawn())
-    return app_menu_button;
-  return nullptr;
+    return {app_menu_button, views::BubbleBorder::TOP_RIGHT};
+  return {};
 }
 
 gfx::Rect GetPageInfoAnchorRect(Browser* browser) {
@@ -39,8 +41,9 @@ gfx::Rect GetPageInfoAnchorRect(Browser* browser) {
   if (views_mode_controller::IsViewsBrowserCocoa())
     return GetPageInfoAnchorRectCocoa(browser);
 #endif
-  // GetPageInfoAnchorView() should be preferred if available.
-  DCHECK_EQ(GetPageInfoAnchorView(browser), nullptr);
+  // GetPageInfoAnchorConfiguration()'s anchor_view should be preferred if
+  // available.
+  DCHECK_EQ(GetPageInfoAnchorConfiguration(browser).anchor_view, nullptr);
 
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
   // Get position in view (taking RTL UI into account).

chrome/browser/ui/views/bubble_anchor_util_views.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_VIEWS_BUBBLE_ANCHOR_UTIL_VIEWS_H_
 
 #include "chrome/browser/ui/bubble_anchor_util.h"
+#include "ui/views/bubble/bubble_border.h"
 
 namespace views {
 class View;
@@ -15,9 +16,17 @@ 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, Anchor = kLocationBar);
+struct AnchorConfiguration {
+  views::View* anchor_view = nullptr;
+  views::BubbleBorder::Arrow bubble_arrow = views::BubbleBorder::TOP_LEFT;
+};
+
+// Returns:
+// - The PageInfo |anchor| View for |browser|, or null if it should not be
+//   used.
+// - The arrow position for the PageInfo bubble.
+AnchorConfiguration GetPageInfoAnchorConfiguration(Browser* browser,
+                                                   Anchor = kLocationBar);
 
 }  // namespace bubble_anchor_util
 

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc

@@ -57,6 +57,7 @@
 #include "chrome/browser/ui/views/location_bar/content_setting_image_view.h"
 #include "chrome/browser/ui/views/location_bar/zoom_bubble_view.h"
 #include "chrome/browser/ui/views/page_action/page_action_icon_container_view.h"
+#include "chrome/browser/ui/views/page_info/page_info_bubble_view_base.h"
 #include "chrome/browser/ui/views/profiles/profile_indicator_icon.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
@@ -835,6 +836,32 @@ class HostedAppNonClientFrameViewAshTest
 
 }  // namespace
 
+// Tests that the page info dialog doesn't anchor in a way that puts it outside
+// of hosted app windows. This is important as some platforms don't support
+// bubble anchor adjustment (see |BubbleDialogDelegateView::CreateBubble()|).
+IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest,
+                       PageInfoBubblePosition) {
+  // Resize app window to only take up the left half of the screen.
+  views::Widget* widget = browser_view_->GetWidget();
+  gfx::Size screen_size =
+      display::Screen::GetScreen()
+          ->GetDisplayNearestWindow(widget->GetNativeWindow())
+          .work_area_size();
+  widget->SetBounds(
+      gfx::Rect(0, 0, screen_size.width() / 2, screen_size.height()));
+
+  // Show page info dialog (currently PWAs use page info in place of an actual
+  // app info dialog).
+  chrome::ExecuteCommand(app_browser_, IDC_HOSTED_APP_MENU_APP_INFO);
+
+  // Check the bubble anchors inside the main app window even if there's space
+  // available outside the main app window.
+  gfx::Rect page_info_bounds = PageInfoBubbleViewBase::GetPageInfoBubble()
+                                   ->GetWidget()
+                                   ->GetWindowBoundsInScreen();
+  EXPECT_TRUE(widget->GetWindowBoundsInScreen().Contains(page_info_bounds));
+}
+
 IN_PROC_BROWSER_TEST_P(HostedAppNonClientFrameViewAshTest, FocusableViews) {
   EXPECT_TRUE(browser_view_->contents_web_view()->HasFocus());
   browser_view_->GetFocusManager()->AdvanceFocus(false);

chrome/browser/ui/views/page_info/page_info_bubble_view.cc

@@ -77,8 +77,9 @@
 #include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
 #endif
 
+using bubble_anchor_util::AnchorConfiguration;
 using bubble_anchor_util::GetPageInfoAnchorRect;
-using bubble_anchor_util::GetPageInfoAnchorView;
+using bubble_anchor_util::GetPageInfoAnchorConfiguration;
 
 namespace {
 
@@ -922,14 +923,16 @@ void ShowPageInfoDialogImpl(Browser* browser,
                                            security_info, anchor);
   }
 #endif
-  views::View* anchor_view = GetPageInfoAnchorView(browser, anchor);
+  AnchorConfiguration configuration =
+      GetPageInfoAnchorConfiguration(browser, anchor);
   gfx::Rect anchor_rect =
-      anchor_view ? gfx::Rect() : GetPageInfoAnchorRect(browser);
+      configuration.anchor_view ? gfx::Rect() : GetPageInfoAnchorRect(browser);
   gfx::NativeWindow parent_window = browser->window()->GetNativeWindow();
   views::BubbleDialogDelegateView* bubble =
       PageInfoBubbleView::CreatePageInfoBubble(
-          anchor_view, anchor_rect, parent_window, browser->profile(),
-          web_contents, virtual_url, security_info);
+          configuration.anchor_view, anchor_rect, parent_window,
+          browser->profile(), web_contents, virtual_url, security_info);
+  bubble->set_arrow(configuration.bubble_arrow);
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
   auto* location_bar = browser_view->GetLocationBarView();
   if (location_bar)

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

@@ -17,13 +17,12 @@
 #include "ui/views/controls/table/table_view_observer.h"
 #include "ui/views/window/dialog_client_view.h"
 
-namespace {
+using bubble_anchor_util::AnchorConfiguration;
 
-constexpr views::BubbleBorder::Arrow kChooserAnchorArrow =
-    views::BubbleBorder::TOP_LEFT;
+namespace {
 
-views::View* GetChooserAnchorView(Browser* browser) {
-  return bubble_anchor_util::GetPageInfoAnchorView(browser);
+AnchorConfiguration GetChooserAnchorConfiguration(Browser* browser) {
+  return bubble_anchor_util::GetPageInfoAnchorConfiguration(browser);
 }
 
 gfx::Rect GetChooserAnchorRect(Browser* browser) {
@@ -79,9 +78,7 @@ class ChooserBubbleUiViewDelegate : public views::BubbleDialogDelegateView,
 ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
     Browser* browser,
     std::unique_ptr<ChooserController> chooser_controller)
-    : views::BubbleDialogDelegateView(GetChooserAnchorView(browser),
-                                      kChooserAnchorArrow),
-      device_chooser_content_view_(nullptr) {
+    : device_chooser_content_view_(nullptr) {
   // ------------------------------------
   // | Chooser bubble title             |
   // | -------------------------------- |
@@ -99,8 +96,7 @@ ChooserBubbleUiViewDelegate::ChooserBubbleUiViewDelegate(
 
   device_chooser_content_view_ =
       new DeviceChooserContentView(this, std::move(chooser_controller));
-  if (!GetAnchorView())
-    SetAnchorRect(GetChooserAnchorRect(browser));
+  UpdateAnchor(browser);
   chrome::RecordDialogCreation(chrome::DialogIdentifier::CHOOSER_UI);
 }
 
@@ -166,10 +162,11 @@ void ChooserBubbleUiViewDelegate::OnSelectionChanged() {
 }
 
 void ChooserBubbleUiViewDelegate::UpdateAnchor(Browser* browser) {
-  views::View* anchor_view = GetChooserAnchorView(browser);
-  SetAnchorView(anchor_view);
-  if (!anchor_view)
+  AnchorConfiguration configuration = GetChooserAnchorConfiguration(browser);
+  SetAnchorView(configuration.anchor_view);
+  if (!configuration.anchor_view)
     SetAnchorRect(GetChooserAnchorRect(browser));
+  set_arrow(configuration.bubble_arrow);
 }
 
 void ChooserBubbleUiViewDelegate::set_bubble_reference(

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

@@ -37,18 +37,17 @@
 #include "chrome/common/chrome_features.h"
 #endif
 
+using bubble_anchor_util::AnchorConfiguration;
+
 namespace {
 
 // (Square) pixel size of icon.
 constexpr int kPermissionIconSize = 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 view to anchor the permission bubble to (may be null) and the
+// arrow position of the bubble.
+AnchorConfiguration GetPermissionAnchorConfiguration(Browser* browser) {
+  return bubble_anchor_util::GetPageInfoAnchorConfiguration(browser);
 }
 
 // Returns the anchor rect to anchor the permission bubble to, as a fallback.
@@ -104,7 +103,6 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
   DCHECK(!requests.empty());
 
   set_close_on_deactivate(false);
-  set_arrow(kPermissionAnchorArrow);
 
 #if defined(OS_MACOSX)
   // On Mac, the browser UI flips depending on a runtime feature. TODO(tapted):
@@ -237,10 +235,12 @@ bool PermissionsBubbleDialogDelegateView::Close() {
 }
 
 void PermissionsBubbleDialogDelegateView::UpdateAnchor() {
-  views::View* anchor_view = GetPermissionAnchorView(owner_->browser());
-  SetAnchorView(anchor_view);
-  if (!anchor_view)
+  AnchorConfiguration configuration =
+      GetPermissionAnchorConfiguration(owner_->browser());
+  SetAnchorView(configuration.anchor_view);
+  if (!configuration.anchor_view)
     SetAnchorRect(GetPermissionAnchorRect(owner_->browser()));
+  set_arrow(configuration.bubble_arrow);
 }
 
 //////////////////////////////////////////////////////////////////////////////

ui/views/bubble/bubble_dialog_delegate_view.cc

@@ -202,6 +202,19 @@ View* BubbleDialogDelegateView::GetAnchorView() const {
   return anchor_view_tracker_->view();
 }
 
+void BubbleDialogDelegateView::set_arrow(BubbleBorder::Arrow arrow) {
+  if (arrow_ == arrow)
+    return;
+  arrow_ = arrow;
+
+  // If set_arrow() is called before CreateWidget(), there's no need to update
+  // the BubbleFrameView.
+  if (GetBubbleFrameView()) {
+    GetBubbleFrameView()->bubble_border()->set_arrow(arrow);
+    SizeToContents();
+  }
+}
+
 gfx::Rect BubbleDialogDelegateView::GetAnchorRect() const {
   if (!GetAnchorView())
     return anchor_rect_;

ui/views/bubble/bubble_dialog_delegate_view.h

@@ -72,7 +72,7 @@ class VIEWS_EXPORT BubbleDialogDelegateView : public DialogDelegateView,
   const gfx::Rect& anchor_rect() const { return anchor_rect_; }
 
   BubbleBorder::Arrow arrow() const { return arrow_; }
-  void set_arrow(BubbleBorder::Arrow arrow) { arrow_ = arrow; }
+  void set_arrow(BubbleBorder::Arrow arrow);
 
   void set_mirror_arrow_in_rtl(bool mirror) { mirror_arrow_in_rtl_ = mirror; }