MacViews: support Views permission bubble

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

Index: chrome/browser/ui/views/website_settings/permissions_bubble_view.h
diff --git a/chrome/browser/ui/views/website_settings/permissions_bubble_view.h b/chrome/browser/ui/views/website_settings/permissions_bubble_view.h
index ccc94dea2b10823d948fdb4f0145a939c1fa485b..78280cb204b61756fb1b354a61b28fb839756689 100644
--- a/chrome/browser/ui/views/website_settings/permissions_bubble_view.h
+++ b/chrome/browser/ui/views/website_settings/permissions_bubble_view.h
@@ -5,11 +5,13 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_WEBSITE_SETTINGS_PERMISSIONS_BUBBLE_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_WEBSITE_SETTINGS_PERMISSIONS_BUBBLE_VIEW_H_
 
+#include <memory>
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "chrome/browser/ui/website_settings/permission_bubble_view.h"
+#include "ui/gfx/geometry/point.h"
 #include "ui/views/bubble/bubble_border.h"
 
 namespace views {
@@ -21,6 +23,19 @@ class PermissionsBubbleDialogDelegateView;
 
 class PermissionBubbleViewViews : public PermissionBubbleView {
  public:
+  // A PermissionBubbleViewViews owns an AnchorDelegate, which is constructed by
+  // calling PermissionBubbleViewViews::CreateAnchorDelegate. The AnchorDelegate
+  // is responsible for determining the anchor view (or point, for platforms
+  // without an appropriate native view) and the immediate parent NativeView of
+  // this view.
+  class AnchorDelegate {

[msw, 2016/05/18 18:57:00]

I don't really like the added complexity of this AnchorDelegate, can we just
have some file-local functions in permissions_bubble_view.cc that return the
view/point/arrow?

namespace {

views::View* GetAnchorView(Browser* browser) {
#if defined(OS_MACOSX)
  return nullptr;
#else
  // Inline PermissionBubbleViewViews::GetAnchorView code.
}

gfx::Point GetAnchorPoint(Browser* browser) {
#if defined(OS_MACOSX)
  return gfx::ScreenPointFromNSPoint(
      [PermissionBubbleController getAnchorPointForBrowser:browser]);
#else
  // An anchor view is used instead of an anchor point on non-Mac platforms.
  return gfx::Point();
}

views::BubbleBorder::Arrow GetAnchorArrow(Browser* browser) {
#if defined(OS_MACOSX)
  return [PermissionBubbleController hasVisibleLocationBarForBrowser:browser]
      ? views::BubbleBorder::TOP_LEFT
      : views::BubbleBorder::NONE;
#else
  // Inline PermissionBubbleViewViews::GetAnchorArrow code.
}

}  // namespace

[Elly Fong-Jones, 2016/05/18 19:13:11]

I think this technique will run into trouble for mac_views_browser=1 builds,
which are OS_MACOSX, but want to use Views for everything. There's no
preprocessor macro or build flag (atm) for detecting mac_views_browser=1 either
:(

How would you feel about this, as methods on PermissionBubbleViewViews:

private views::View* GetAnchorView(Browser* browser);
private gfx::Point GetAnchorPoint(Browser* browser);
private views::BubbleBorder::Arrow GetAnchorArrow(Browser* browser);

and then having two separate implementations of those functions, one in
chrome/browser/ui/cocoa and one in chrome/browser/ui/views?

[msw, 2016/05/18 19:26:21]

Ah, and this views bubble is used for the cocoa browser window too? Could we
rely on some runtime check like GetBrowserViewForBrowser returning null in Cocoa
or getting a valid BrowserWindowCocoa? If not, your suggestion seems a bit
cleaner than the current patch set, but if you disagree, I can review it as-is.

+   public:
+    AnchorDelegate() {}
+    virtual views::View* GetAnchorView() = 0;
+    virtual gfx::Point GetAnchorPoint() = 0;
+    virtual views::BubbleBorder::Arrow GetAnchorArrow() = 0;
+  };
+
   explicit PermissionBubbleViewViews(Browser* browser);
   ~PermissionBubbleViewViews() override;
 
@@ -40,12 +55,14 @@ class PermissionBubbleViewViews : public PermissionBubbleView {
   void Deny();
 
  private:
-  views::View* GetAnchorView();
-  views::BubbleBorder::Arrow GetAnchorArrow();
+  // This function has separate implementations for Views-based and Cocoa-based
+  // browsers.
+  static std::unique_ptr<AnchorDelegate> CreateAnchorDelegate(Browser* browser);
 
   Browser* browser_;
   Delegate* delegate_;
   PermissionsBubbleDialogDelegateView* bubble_delegate_;
+  std::unique_ptr<AnchorDelegate> anchor_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(PermissionBubbleViewViews);
 };