MacViews: support Views permission bubble

chrome/browser/ui/views/website_settings/permissions_bubble_view.cc

Index: chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
diff --git a/chrome/browser/ui/views/website_settings/permissions_bubble_view.cc b/chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
index 000a4de2e6e36b9dcf5bd1d7154aed5819ee8948..5471aa49f73bee8eb27f10fd790bba63b789732b 100644
--- a/chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
+++ b/chrome/browser/ui/views/website_settings/permissions_bubble_view.cc
@@ -11,11 +11,8 @@
 #include "base/strings/string16.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.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/website_settings/permission_selector_view.h"
 #include "chrome/browser/ui/views/website_settings/permission_selector_view_observer.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_request.h"
@@ -31,6 +28,7 @@
 #include "ui/gfx/text_constants.h"
 #include "ui/gfx/vector_icons_public.h"
 #include "ui/views/bubble/bubble_delegate.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/button/menu_button.h"
@@ -149,6 +147,7 @@ class PermissionsBubbleDialogDelegateView
  public:
   PermissionsBubbleDialogDelegateView(
       views::View* anchor_view,
+      const gfx::Point& anchor_point,
       views::BubbleBorder::Arrow anchor_arrow,
       PermissionBubbleViewViews* owner,
       const std::vector<PermissionBubbleRequest*>& requests,
@@ -177,6 +176,7 @@ class PermissionsBubbleDialogDelegateView
   // 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);
 
  private:
@@ -191,6 +191,7 @@ class PermissionsBubbleDialogDelegateView
 
 PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
     views::View* anchor_view,
+    const gfx::Point& anchor_point,
     views::BubbleBorder::Arrow anchor_arrow,
     PermissionBubbleViewViews* owner,
     const std::vector<PermissionBubbleRequest*>& requests,
@@ -200,6 +201,9 @@ PermissionsBubbleDialogDelegateView::PermissionsBubbleDialogDelegateView(
       multiple_requests_(requests.size() > 1) {
   DCHECK(!requests.empty());
 
+  if (!anchor_view)
+    SetAnchorRect(gfx::Rect(anchor_point, gfx::Size()));
+
   set_close_on_deactivate(false);
 
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kVertical, 0, 0,
@@ -353,10 +357,8 @@ void PermissionsBubbleDialogDelegateView::PermissionSelectionChanged(
 
 void PermissionsBubbleDialogDelegateView::UpdateAnchor(
     views::View* anchor_view,
+    const gfx::Point& anchor_point,
     views::BubbleBorder::Arrow anchor_arrow) {
-  if (GetAnchorView() == anchor_view && arrow() == anchor_arrow)
-    return;
-
   set_arrow(anchor_arrow);
 
   // Update the border in the bubble: will either add or remove the arrow.
@@ -369,7 +371,10 @@ void PermissionsBubbleDialogDelegateView::UpdateAnchor(
       new views::BubbleBorder(adjusted_arrow, shadow(), color())));
 
   // Reposition the bubble based on the updated arrow and view.
-  SetAnchorView(anchor_view);
+  if (anchor_view)
+    SetAnchorView(anchor_view);
+  else
+    SetAnchorRect(gfx::Rect(anchor_point, gfx::Size()));
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -378,31 +383,15 @@ void PermissionsBubbleDialogDelegateView::UpdateAnchor(
 PermissionBubbleViewViews::PermissionBubbleViewViews(Browser* browser)
     : browser_(browser),
       delegate_(nullptr),
-      bubble_delegate_(nullptr) {
+      bubble_delegate_(nullptr),
+      anchor_delegate_(CreateAnchorDelegate(browser)) {
   DCHECK(browser);
+  DCHECK(browser->window());
 }
 
 PermissionBubbleViewViews::~PermissionBubbleViewViews() {
 }
 
-// static
-std::unique_ptr<PermissionBubbleView> PermissionBubbleView::Create(
-    Browser* browser) {
-  return base::WrapUnique(new PermissionBubbleViewViews(browser));
-}
-
-views::View* PermissionBubbleViewViews::GetAnchorView() {
-  BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser_);
-
-  if (browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR))
-    return browser_view->GetLocationBarView()->location_icon_view();
-
-  if (browser_view->IsFullscreenBubbleVisible())
-    return browser_view->exclusive_access_bubble()->GetView();
-
-  return browser_view->top_container();
-}
-
 views::BubbleBorder::Arrow PermissionBubbleViewViews::GetAnchorArrow() {
   if (browser_->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR))
     return views::BubbleBorder::TOP_LEFT;
@@ -420,12 +409,12 @@ void PermissionBubbleViewViews::Show(
     bubble_delegate_->CloseBubble();
 
   bubble_delegate_ = new PermissionsBubbleDialogDelegateView(

[tapted, 2016/05/11 07:06:19]

(so

bubble_delegate_ = new PermissionsBubbleDialogDelegateView(this, requests,
values);
bubble_delegate_->UpdateAnchor(anchor_delegate_->GetAnchorView(),
anchor_delegate_->GetAnchorPoint(), GetAnchorArrow());

might work here, and would reduce the number of codepaths we need to worry
about)

[Elly Fong-Jones, 2016/05/11 19:16:38]

Aha, I think I finally understand what is meant here. I made this change and it
does simplify the code quite a bit.

-      GetAnchorView(), GetAnchorArrow(), this, requests, values);
+      anchor_delegate_->GetAnchorView(), anchor_delegate_->GetAnchorPoint(),
+      GetAnchorArrow(), this, requests, values);
 
   // Set |parent_window| because some valid anchors can become hidden.
-  views::Widget* widget = views::Widget::GetWidgetForNativeWindow(
-      browser_->window()->GetNativeWindow());
-  bubble_delegate_->set_parent_window(widget->GetNativeView());
+  bubble_delegate_->set_parent_window(
+      platform_util::GetViewForWindow(browser_->window()->GetNativeWindow()));

[tapted, 2016/05/11 07:06:19]

I think set_parent_window needs to be called in UpdateAnchorPosition() as well
to handle entering fullscreen. E.g. the following should work:

1. Go to https://permission.site/
2. Click "Location", get a prompt (leave it open)
3. Click "Fullscreen" should go into fullscreen -- Browser* stays the same, but
(on Mac) this makes a new parent_window. (Location permission request should
still be visible, but it should lose its arrow)
4. Press Esc to go back to not-fullscreen. Location permission request should
still be visible.

Those steps work fine on Windows.


(On the Mac Cocoa browser it's actually horribly broken currently - eep. Step 3.
moves the bubble but (a) it has an arrow (b) it's in the wrong place (c) the
fullscreen window has a menubar/toolbar/locationbar when it shouldn't. Then on
Step 4. the bubble disappears, and clicking "location" again doesn't show it
[interestingly, switching to a different tab and back shows the bubble again]).


But fixing all that might turn out to be more complicated -- the bubble may need
to be hidden and reshown, or we may need to fix some of the bugs in the cocoa
browser first. Maybe try it out - if a set_parent_window call in
UpdateAnchorPosition fixes it then yay, but I suspect there's more to it and
it's better left for a follow-up CL.

[Elly Fong-Jones, 2016/05/11 19:16:37]

Just calling set_parent_window() does not seem to help. The bubble moves to just
under where the toolbar "would be" if it was above the top of the screen,
although the arrow does disappear.

I think this behavior will need concerted fixing in a separate CL.

 
   views::BubbleDialogDelegateView::CreateBubble(bubble_delegate_)->Show();
   bubble_delegate_->SizeToContents();
@@ -447,8 +436,11 @@ bool PermissionBubbleViewViews::IsVisible() {
 }
 
 void PermissionBubbleViewViews::UpdateAnchorPosition() {
-  if (IsVisible())
-    bubble_delegate_->UpdateAnchor(GetAnchorView(), GetAnchorArrow());
+  if (IsVisible()) {
+    bubble_delegate_->UpdateAnchor(anchor_delegate_->GetAnchorView(),
+                                   anchor_delegate_->GetAnchorPoint(),
+                                   GetAnchorArrow());
+  }
 }
 
 gfx::NativeWindow PermissionBubbleViewViews::GetNativeWindow() {