[Mac] Enable MacViews site settings bubble behind --enable-mac-views-dialogs.

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

Index: chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
diff --git a/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc b/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
index 54229954826b7a0420da6b852ea3c14a066b8097..0e84e8e50a20e79ded11b063bfcfaeaf251c949d 100644
--- a/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
+++ b/chrome/browser/ui/views/website_settings/website_settings_popup_view.cc
@@ -141,13 +141,16 @@ class PopupHeaderView : public views::View {
 // displayed.
 class InternalPageInfoPopupView : public views::BubbleDelegateView {
  public:
-  explicit InternalPageInfoPopupView(views::View* anchor_view);
+  InternalPageInfoPopupView(views::View* anchor_view,

[tapted, 2015/08/17 06:00:49]

nit: Maybe a comment here. E.g. it's not obvious that either anchor_view or
parent_window is expected to be null.

[jackhou1, 2015/08/17 07:06:47]

Done.

+                            gfx::NativeView parent_window);
   ~InternalPageInfoPopupView() override;
 
   // views::BubbleDelegateView:
   void OnWidgetDestroying(views::Widget* widget) override;
 
  private:
+  friend class WebsiteSettingsPopupView;
+
   DISALLOW_COPY_AND_ASSIGN(InternalPageInfoPopupView);
 };
 
@@ -229,8 +232,12 @@ void PopupHeaderView::SetIdentityStatus(const base::string16& status,
 // InternalPageInfoPopupView
 ////////////////////////////////////////////////////////////////////////////////
 
-InternalPageInfoPopupView::InternalPageInfoPopupView(views::View* anchor_view)
+InternalPageInfoPopupView::InternalPageInfoPopupView(
+    views::View* anchor_view,
+    gfx::NativeView parent_window)
     : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT) {
+  set_parent_window(parent_window);
+
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(kLocationIconVerticalMargin, 0,
                                      kLocationIconVerticalMargin, 0));
@@ -250,8 +257,7 @@ InternalPageInfoPopupView::InternalPageInfoPopupView(views::View* anchor_view)
   label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   AddChildView(label);
 
-  views::BubbleDelegateView::CreateBubble(this)->Show();
-  SizeToContents();
+  views::BubbleDelegateView::CreateBubble(this);
 }
 
 InternalPageInfoPopupView::~InternalPageInfoPopupView() {
@@ -275,10 +281,35 @@ void WebsiteSettingsPopupView::ShowPopup(views::View* anchor_view,
                                          const GURL& url,
                                          const content::SSLStatus& ssl) {
   is_popup_showing = true;
+  views::BubbleDelegateView* popup = nullptr;
+  if (InternalChromePage(url)) {
+    popup = new InternalPageInfoPopupView(anchor_view, nullptr);
+  } else {
+    popup = new WebsiteSettingsPopupView(anchor_view, nullptr, profile,
+                                         web_contents, url, ssl);
+  }
+  popup->GetWidget()->Show();
+}
+
+// static
+void WebsiteSettingsPopupView::ShowPopupAtRect(

[tapted, 2015/08/17 06:00:49]

I *think* a helper function in an anonymous namespace would be an improvement.
The benefit is borderline, but duplicated logic usually has bigger downsides for
maintainability. I'm thinking something like the following, but msw may have
other ideas :)

views::BubbleDelegateView* CreatePopup(
    views::View* anchor_view,
    Profile* profile,
    content::WebContents* web_contents,
    const GURL& url,
    const content::SSLStatus& ssl) {
  gfx::NativeWindow parent = anchor_view ?
      nullptr : web_contents->GetNativeView();
  if (InternalChromePage(url))
    return new InternalPageInfoPopupView(anchor_view, parent);
  return new WebsiteSettingsPopupView(
      anchor_view, parent, profile, web_contents, url, ssl);
}

ShowPopup(...) {
  is_popup_showing = true;
  CreatePopup(...)->GetWidget()->Show();
}

ShowPopupAtRect(...) {
  is_popup_showing = true;
  InternalPageInfoPopupView* popup = CreatePopup(...);
  popup->SetAnchorRect(anchor_rect);
  popup->GetWidget()->Show();
}

[jackhou1, 2015/08/17 06:19:13]

I don't think ShowPopupAtRect would be able to call
BubbleDelegateView::SetAnchorRect like this. At the moment, it's calling
WebsiteSettingsPopupView::SetAnchorRect, and
InternalPageInfoPopupView::SetAnchorRect as a friend.

[tapted, 2015/08/17 06:30:57]

Ah. good point :/. That does make sense of the repetition - maybe just comment
above `InternalPageInfoPopupView* popup =..` like 
    // Use the concrete type so that SetAnchorRect can be called as a friend.

I guess the alternative is to alter the constructor, but that feels messier.

[jackhou1, 2015/08/17 06:56:31]

Done.

+    const gfx::Rect& anchor_rect,
+    Profile* profile,
+    content::WebContents* web_contents,
+    const GURL& url,
+    const content::SSLStatus& ssl) {
+  is_popup_showing = true;
   if (InternalChromePage(url)) {
-    new InternalPageInfoPopupView(anchor_view);
+    InternalPageInfoPopupView* popup =
+        new InternalPageInfoPopupView(nullptr, web_contents->GetNativeView());
+    popup->SetAnchorRect(anchor_rect);
+    popup->GetWidget()->Show();
   } else {
-    new WebsiteSettingsPopupView(anchor_view, profile, web_contents, url, ssl);
+    WebsiteSettingsPopupView* popup =
+        new WebsiteSettingsPopupView(nullptr, web_contents->GetNativeView(),
+                                     profile, web_contents, url, ssl);
+    popup->SetAnchorRect(anchor_rect);
+    popup->GetWidget()->Show();
   }
 }
 
@@ -289,6 +320,7 @@ bool WebsiteSettingsPopupView::IsPopupShowing() {
 
 WebsiteSettingsPopupView::WebsiteSettingsPopupView(
     views::View* anchor_view,
+    gfx::NativeView parent_window,
     Profile* profile,
     content::WebContents* web_contents,
     const GURL& url,
@@ -310,6 +342,8 @@ WebsiteSettingsPopupView::WebsiteSettingsPopupView(
       help_center_link_(nullptr),
       connection_info_content_(nullptr),
       weak_factory_(this) {
+  set_parent_window(parent_window);
+
   // Compensate for built-in vertical padding in the anchor view's image.
   set_anchor_view_insets(gfx::Insets(kLocationIconVerticalMargin, 0,
                                      kLocationIconVerticalMargin, 0));
@@ -354,8 +388,7 @@ WebsiteSettingsPopupView::WebsiteSettingsPopupView(
   set_margins(gfx::Insets(kPopupMarginTop, kPopupMarginLeft,
                           kPopupMarginBottom, kPopupMarginRight));
 
-  views::BubbleDelegateView::CreateBubble(this)->Show();
-  SizeToContents();
+  views::BubbleDelegateView::CreateBubble(this);
 
   presenter_.reset(new WebsiteSettings(
       this, profile,