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

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

 // Copyright (c) 2012 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/website_settings/website_settings_popup_view.h"
 
 #include <algorithm>
 
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 123 matching lines @@
   views::Label* status_;
 
   DISALLOW_COPY_AND_ASSIGN(PopupHeaderView);
 };
 
 // Website Settings are not supported for internal Chrome pages. Instead of the
 // |WebsiteSettingsPopupView|, the |InternalPageInfoPopupView| is
 // 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);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 // Popup Header
 ////////////////////////////////////////////////////////////////////////////////
 
 PopupHeaderView::PopupHeaderView(views::ButtonListener* close_button_listener)
     : name_(nullptr), status_(nullptr) {
   views::GridLayout* layout = new views::GridLayout(this);
@@ skipping 61 matching lines @@
 void PopupHeaderView::SetIdentityStatus(const base::string16& status,
                                         SkColor text_color) {
   status_->SetText(status);
   status_->SetEnabledColor(text_color);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // 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));
 
   const int kSpacing = 4;
   SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal, kSpacing,
                                         kSpacing, kSpacing));
   views::ImageView* icon_view = new views::ImageView();
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   icon_view->SetImage(rb.GetImageSkiaNamed(IDR_PRODUCT_LOGO_26));
   AddChildView(icon_view);
 
   views::Label* label =
       new views::Label(l10n_util::GetStringUTF16(IDS_PAGE_INFO_INTERNAL_PAGE));
   label->SetMultiLine(true);
   label->SetAllowCharacterBreak(true);
   label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   AddChildView(label);
 
-  views::BubbleDelegateView::CreateBubble(this)->Show();
-  SizeToContents();
+  views::BubbleDelegateView::CreateBubble(this);
 }
 
 InternalPageInfoPopupView::~InternalPageInfoPopupView() {
 }
 
 void InternalPageInfoPopupView::OnWidgetDestroying(views::Widget* widget) {
   is_popup_showing = false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // WebsiteSettingsPopupView
 ////////////////////////////////////////////////////////////////////////////////
 
 WebsiteSettingsPopupView::~WebsiteSettingsPopupView() {
 }
 
 // static
 void WebsiteSettingsPopupView::ShowPopup(views::View* anchor_view,
                                          Profile* profile,
                                          content::WebContents* web_contents,
                                          const GURL& url,
                                          const content::SSLStatus& ssl) {
   is_popup_showing = true;
+  views::BubbleDelegateView* popup = nullptr;
   if (InternalChromePage(url)) {
-    new InternalPageInfoPopupView(anchor_view);
+    popup = new InternalPageInfoPopupView(anchor_view, nullptr);
   } else {
-    new WebsiteSettingsPopupView(anchor_view, profile, web_contents, url, ssl);
+    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)) {
+    InternalPageInfoPopupView* popup =
+        new InternalPageInfoPopupView(nullptr, web_contents->GetNativeView());
+    popup->SetAnchorRect(anchor_rect);
+    popup->GetWidget()->Show();
+  } else {
+    WebsiteSettingsPopupView* popup =
+        new WebsiteSettingsPopupView(nullptr, web_contents->GetNativeView(),
+                                     profile, web_contents, url, ssl);
+    popup->SetAnchorRect(anchor_rect);
+    popup->GetWidget()->Show();
   }
 }
 
 // static
 bool WebsiteSettingsPopupView::IsPopupShowing() {
   return is_popup_showing;
 }
 
 WebsiteSettingsPopupView::WebsiteSettingsPopupView(
     views::View* anchor_view,
+    gfx::NativeView parent_window,
     Profile* profile,
     content::WebContents* web_contents,
     const GURL& url,
     const content::SSLStatus& ssl)
     : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT),
       web_contents_(web_contents),
       header_(nullptr),
       tabbed_pane_(nullptr),
       permissions_tab_(nullptr),
       site_data_content_(nullptr),
       cookie_dialog_link_(nullptr),
       permissions_content_(nullptr),
       connection_tab_(nullptr),
       identity_info_content_(nullptr),
       certificate_dialog_link_(nullptr),
       reset_decisions_button_(nullptr),
       help_center_content_(nullptr),
       cert_id_(0),
       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));
 
   views::GridLayout* layout = new views::GridLayout(this);
   SetLayoutManager(layout);
   const int content_column = 0;
   views::ColumnSet* column_set = layout->AddColumnSet(content_column);
   column_set->AddColumn(views::GridLayout::FILL,
                         views::GridLayout::FILL,
@@ skipping 24 matching lines @@
   tabbed_pane_->AddTabAtIndex(
       TAB_ID_CONNECTION,
       l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_TAB_LABEL_CONNECTION),
       connection_tab_);
   DCHECK_EQ(tabbed_pane_->GetTabCount(), NUM_TAB_IDS);
   tabbed_pane_->set_listener(this);
 
   set_margins(gfx::Insets(kPopupMarginTop, kPopupMarginLeft,
                           kPopupMarginBottom, kPopupMarginRight));
 
-  views::BubbleDelegateView::CreateBubble(this)->Show();
-  SizeToContents();
+  views::BubbleDelegateView::CreateBubble(this);
 
   presenter_.reset(new WebsiteSettings(
       this, profile,
       TabSpecificContentSettings::FromWebContents(web_contents),
       InfoBarService::FromWebContents(web_contents), url, ssl,
       content::CertStore::GetInstance()));
 }
 
 void WebsiteSettingsPopupView::OnPermissionChanged(
     const WebsiteSettingsUI::PermissionInfo& permission) {
@@ skipping 433 matching lines @@
     // desktop we should link to that here, too.
     web_contents_->OpenURL(content::OpenURLParams(
         GURL(chrome::kChromeUIContentSettingsURL), content::Referrer(),
         NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK, false));
     presenter_->RecordWebsiteSettingsAction(
         WebsiteSettings::WEBSITE_SETTINGS_SITE_SETTINGS_OPENED);
   } else {
     NOTREACHED();
   }
 }