(Views only) Create a separate class for the contents of the Website Settings UI that is displayed …

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 aed4c2192ddef24f70d9385eb53e248fac04525f..745fe48efe9f15adcc34146df8e4342b7347cbcb 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
@@ -134,6 +134,21 @@ class PopupHeaderView : public views::View {
   DISALLOW_COPY_AND_ASSIGN(PopupHeaderView);
 };
 
+// Website Settings are not supported for internal Chrome pages. Instead of the
+// |WebsiteSettingsPopupView| the |WebsiteSettingsInternalPageView| is
+// displayed.
+class WebsiteSettingsInternalPageView : public views::BubbleDelegateView {
+ public:
+  explicit WebsiteSettingsInternalPageView(views::View* anchor_view);
+  virtual ~WebsiteSettingsInternalPageView();
+
+  // views::BubbleDelegate implementations.
+  virtual gfx::Rect GetAnchorRect() OVERRIDE;

[tfarina, 2012/08/08 02:32:46]

nit: can this be private? please.

[markusheintz_, 2012/08/08 13:20:13]

Done.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(WebsiteSettingsInternalPageView);
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // Popup Header
 ////////////////////////////////////////////////////////////////////////////////
@@ -207,9 +222,67 @@ void PopupHeaderView::SetIdentityStatus(const string16& status,
   status_->SetEnabledColor(text_color);
 }
 
-///////////////////////////////////////////////////////////////////////////////
+////////////////////////////////////////////////////////////////////////////////
+// WebsiteSettingsInternalPageView
+////////////////////////////////////////////////////////////////////////////////
+
+WebsiteSettingsInternalPageView::WebsiteSettingsInternalPageView(
+    views::View* anchor_view)
+    : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT) {
+  views::GridLayout* layout = new views::GridLayout(this);

[msw, 2012/08/07 22:21:49]

GridLayout seems overkill here, please try a BoxLayout.

[tfarina, 2012/08/08 02:32:46]

I don't like doing many things in constructors. Is there another way to
initialize all this stuff outside of the constructor?

[markusheintz_, 2012/08/08 13:20:13]

In general I agree with you. But in this particular case what would be the
benefit?

I would need to add an additional Init method that is called right after the
object is created. This means I add additional code with no benefit. Please
correct me if I'm wrong :).

[markusheintz_, 2012/08/08 13:20:13]

Done.

+  SetLayoutManager(layout);
+  views::ColumnSet* column_set = layout->AddColumnSet(0);
+  column_set->AddColumn(views::GridLayout::FILL,
+                        views::GridLayout::FILL,
+                        0,  // Resize weight.
+                        views::GridLayout::USE_PREF,
+                        0,
+                        0);
+  column_set->AddPaddingColumn(0, kIconMarginLeft);
+  column_set->AddColumn(views::GridLayout::FILL,
+                        views::GridLayout::FILL,
+                        1,
+                        views::GridLayout::USE_PREF,
+                        0,
+                        0);
+
+  layout->StartRow(1, 0);
+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  const gfx::Image& icon = rb.GetNativeImageNamed(IDR_PRODUCT_LOGO_26);
+  views::ImageView* icon_view = new views::ImageView();
+  icon_view->SetImage(icon.ToImageSkia());
+  layout->AddView(icon_view, 1, 1, views::GridLayout::LEADING,
+                  views::GridLayout::LEADING);
+
+  string16 text = l10n_util::GetStringUTF16(
+      IDS_PAGE_INFO_INTERNAL_PAGE);
+  views::Label* label = new views::Label(text);
+  label->SetMultiLine(true);
+  label->SetAllowCharacterBreak(true);
+  label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+  layout->AddView(label, 1, 1, views::GridLayout::LEADING,
+                  views::GridLayout::CENTER);
+
+  views::BubbleDelegateView::CreateBubble(this);
+  Show();
+  SizeToContents();
+}
+
+WebsiteSettingsInternalPageView::~WebsiteSettingsInternalPageView() {
+}
+
+gfx::Rect WebsiteSettingsInternalPageView::GetAnchorRect() {
+  // Compensate for some built-in padding in the icon. This will make the arrow
+  // point to the middle of the icon.
+  gfx::Rect anchor(BubbleDelegateView::GetAnchorRect());
+  anchor.Inset(0, anchor_view() ? kLocationIconBottomMargin : 0);
+  return anchor;
+}
+
+////////////////////////////////////////////////////////////////////////////////
 // WebsiteSettingsPopupView
-///////////////////////////////////////////////////////////////////////////////
+////////////////////////////////////////////////////////////////////////////////
 
 WebsiteSettingsPopupView::~WebsiteSettingsPopupView() {
 }
@@ -220,7 +293,10 @@ void WebsiteSettingsPopupView::ShowPopup(views::View* anchor_view,
                                          TabContents* tab_contents,
                                          const GURL& url,
                                          const content::SSLStatus& ssl) {
-  new WebsiteSettingsPopupView(anchor_view, profile, tab_contents, url, ssl);
+  if (InternalChromePage(url))
+    new WebsiteSettingsInternalPageView(anchor_view);
+  else
+    new WebsiteSettingsPopupView(anchor_view, profile, tab_contents, url, ssl);
 }
 
 WebsiteSettingsPopupView::WebsiteSettingsPopupView(
@@ -241,47 +317,6 @@ WebsiteSettingsPopupView::WebsiteSettingsPopupView(
       cert_id_(0),
       connection_info_content_(NULL),
       page_info_content_(NULL) {
-  if (InternalChromePage(url)) {
-    views::GridLayout* layout = new views::GridLayout(this);
-    SetLayoutManager(layout);
-    views::ColumnSet* column_set = layout->AddColumnSet(0);
-    column_set->AddColumn(views::GridLayout::FILL,
-                          views::GridLayout::FILL,
-                          0,  // Resize weight.
-                          views::GridLayout::USE_PREF,
-                          0,
-                          0);
-    column_set->AddPaddingColumn(0, kIconMarginLeft);
-    column_set->AddColumn(views::GridLayout::FILL,
-                          views::GridLayout::FILL,
-                          1,
-                          views::GridLayout::USE_PREF,
-                          0,
-                          0);
-
-    layout->StartRow(1, 0);
-
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    const gfx::Image& icon = rb.GetNativeImageNamed(IDR_PRODUCT_LOGO_26);
-    views::ImageView* icon_view = new views::ImageView();
-    icon_view->SetImage(icon.ToImageSkia());
-    layout->AddView(icon_view, 1, 1, views::GridLayout::LEADING,
-                    views::GridLayout::LEADING);
-
-    string16 text = l10n_util::GetStringUTF16(
-        IDS_PAGE_INFO_INTERNAL_PAGE);
-    views::Label* label = new views::Label(text);
-    label->SetMultiLine(true);
-    label->SetAllowCharacterBreak(true);
-    label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-    layout->AddView(label, 1, 1, views::GridLayout::LEADING,
-                    views::GridLayout::CENTER);
-
-    views::BubbleDelegateView::CreateBubble(this);
-    Show();
-    SizeToContents();
-  } else {
-    // Non internal chrome page.
     views::GridLayout* layout = new views::GridLayout(this);
     SetLayoutManager(layout);
     const int content_column = 0;
@@ -328,14 +363,11 @@ WebsiteSettingsPopupView::WebsiteSettingsPopupView(
                                          url,
                                          ssl,
                                          content::CertStore::GetInstance()));
-  }
 }
 
 void WebsiteSettingsPopupView::OnPermissionChanged(
     PermissionSelectorView* permission_selector) {
   DCHECK(permission_selector);
-  // It's not necessary to check that the |presenter_| is not NULL since for
-  // internal chrome pages OnPermissionChanged can't be called.
   presenter_->OnSitePermissionChanged(
       permission_selector->GetPermissionType(),
       permission_selector->GetSelectedSetting());
@@ -350,8 +382,7 @@ gfx::Rect WebsiteSettingsPopupView::GetAnchorRect() {
 }
 
 void WebsiteSettingsPopupView::OnWidgetClosing(views::Widget* widget) {
-  if (presenter_.get())
-    presenter_->OnUIClosing();
+  presenter_->OnUIClosing();
 }
 
 void WebsiteSettingsPopupView::ButtonPressed(