(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..77360b05acba5d0eb997223614e85d1fc89b9545 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

[msw, 2012/08/08 17:27:08]

nit: add a comma: "|WebsiteSettingsPopupView|, the"

[markusheintz_, 2012/08/08 18:00:35]

Done.

+// displayed.
+class WebsiteSettingsInternalPageView : public views::BubbleDelegateView {
+ public:
+  explicit WebsiteSettingsInternalPageView(views::View* anchor_view);
+  virtual ~WebsiteSettingsInternalPageView();
+
+ private:
+  // views::BubbleDelegate implementations.
+  virtual gfx::Rect GetAnchorRect() OVERRIDE;
+
+  DISALLOW_COPY_AND_ASSIGN(WebsiteSettingsInternalPageView);
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // Popup Header
 ////////////////////////////////////////////////////////////////////////////////
@@ -207,9 +222,49 @@ void PopupHeaderView::SetIdentityStatus(const string16& status,
   status_->SetEnabledColor(text_color);
 }
 
-///////////////////////////////////////////////////////////////////////////////
+////////////////////////////////////////////////////////////////////////////////
+// WebsiteSettingsInternalPageView

[msw, 2012/08/08 17:27:08]

nit: I don't love the name... but lack a great suggestion,
InternalPagePopupView?

[markusheintz_, 2012/08/08 18:00:35]

How about InternalPageInfoView?

[msw, 2012/08/08 18:34:10]

It's still a 'Popup' bubble, like WebsiteSettings, and both bubbles contain
'Info'; but InternalPageInfoView is an improvement, thanks!

[markusheintz_, 2012/08/08 22:10:02]

Well then let's use the descriptive name: InternalPageInfoPopupView. This name
reminds me somehow of the times when I used to code in Java :)

+////////////////////////////////////////////////////////////////////////////////
+
+WebsiteSettingsInternalPageView::WebsiteSettingsInternalPageView(
+    views::View* anchor_view)
+    : BubbleDelegateView(anchor_view, views::BubbleBorder::TOP_LEFT) {
+  const int spacing = 4;

[msw, 2012/08/08 17:27:08]

nit: this should be kSpacing, i think. (perhaps same in your other review, did I
miss that?)

[markusheintz_, 2012/08/08 18:00:35]

Done. I'm not sure which other CL you mean. I'll skipped through some recent  
CLs and it looks like you caught all issues ;-). 

[msw, 2012/08/08 18:34:10]

found it and sent another nit :p
http://codereview.chromium.org/10823229/diff/4001/ui/views/controls/tabbed_pa...

[markusheintz_, 2012/08/08 22:10:02]

And I changed the code so that I don't need to address the nit :-D

+  SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal, spacing,
+                                        spacing, spacing));
+  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());

[msw, 2012/08/08 17:27:08]

nit: consider the shorter version (very optional):
icon_view->SetImage(rb. GetImageSkiaNamed(IDR_PRODUCT_LOGO_26));

[markusheintz_, 2012/08/08 18:00:35]

Done.

+  AddChildView(icon_view);
+
+  string16 text = l10n_util::GetStringUTF16(
+      IDS_PAGE_INFO_INTERNAL_PAGE);

[msw, 2012/08/08 17:27:08]

nit: fits on line above.

[markusheintz_, 2012/08/08 18:00:35]

Done.

+  views::Label* label = new views::Label(text);
+  label->SetMultiLine(true);
+  label->SetAllowCharacterBreak(true);
+  label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+  AddChildView(label);
+
+  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 +275,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 +299,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 +345,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 +364,7 @@ gfx::Rect WebsiteSettingsPopupView::GetAnchorRect() {
 }
 
 void WebsiteSettingsPopupView::OnWidgetClosing(views::Widget* widget) {
-  if (presenter_.get())
-    presenter_->OnUIClosing();
+  presenter_->OnUIClosing();
 }
 
 void WebsiteSettingsPopupView::ButtonPressed(