Fixes of settings view layout bugs.

ui/message_center/views/notifier_settings_view.cc

Index: ui/message_center/views/notifier_settings_view.cc
diff --git a/ui/message_center/views/notifier_settings_view.cc b/ui/message_center/views/notifier_settings_view.cc
index e0c3fba7f4398e22b9a315ad923728d959ce35b9..b1c6eb0efbe40a6fcad07505192b1c32cbcff3a9 100644
--- a/ui/message_center/views/notifier_settings_view.cc
+++ b/ui/message_center/views/notifier_settings_view.cc
@@ -49,6 +49,7 @@ class EntryView : public views::View {
   // Overridden from views::View:
   virtual void Layout() OVERRIDE;
   virtual gfx::Size GetPreferredSize() OVERRIDE;
+  virtual void OnFocus() OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(EntryView);
@@ -64,11 +65,12 @@ EntryView::~EntryView() {
 void EntryView::Layout() {
   DCHECK_EQ(1, child_count());
   views::View* contents = child_at(0);
-  gfx::Size size = contents->GetPreferredSize();
+  int contents_width = width() - kMarginWidth * 2;
+  int contents_height = contents->GetHeightForWidth(contents_width);

[dharcourt, 2013/04/05 16:07:03]

Nit: contents_width/contents_height should be content_width/content_height
(singular, like "horse race" instead of "horses race")... and I know we have
many other places in the code where this is wrong :-).

[Jun Mukai, 2013/04/06 03:14:04]

Done.

   int y = 0;
-  if (size.height() < height())
-    y = (height() - size.height()) / 2;
-  contents->SetBounds(kMarginWidth, y, width() - kMarginWidth, size.height());
+  if (contents_height < height())
+    y = (height() - contents_height) / 2;

[dharcourt, 2013/04/05 16:07:03]

This _might_ be simpler as y = std::max((height() - content_height) / 2, 0),
your call.

[Jun Mukai, 2013/04/06 03:14:04]

Done.

+  contents->SetBounds(kMarginWidth, y, contents_width, contents_height);
 }
 
 gfx::Size EntryView::GetPreferredSize() {
@@ -78,6 +80,10 @@ gfx::Size EntryView::GetPreferredSize() {
   return size;
 }
 
+void EntryView::OnFocus() {
+  ScrollRectToVisible(GetLocalBounds());
+}
+
 // The separator line between the title and the scroll view. Currently
 // it is achieved as a top border of the scroll view.
 class Separator : public views::Border {
@@ -141,7 +147,7 @@ class NotifierSettingsView::NotifierButton : public views::CustomButton,
         views::BoxLayout::kHorizontal, 0, 0, kSpaceInButtonComponents));
     checkbox_->SetChecked(notifier_->enabled);
     checkbox_->set_listener(this);
-    checkbox_->set_focusable(true);
+    checkbox_->set_focusable(false);

[dewittj, 2013/04/05 20:48:41]

I believe you could just remove this line, since focusable is default false.

[Jun Mukai, 2013/04/06 03:14:04]

Done.

     AddChildView(checkbox_);
     UpdateIconImage(notifier_->icon);
     AddChildView(new views::Label(notifier_->name));
@@ -264,14 +270,16 @@ NotifierSettingsView::NotifierSettingsView(
       IDS_MESSAGE_CENTER_SETTINGS_DIALOG_DESCRIPTION));
   top_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   top_label->SetMultiLine(true);
-  top_label->SizeToFit(kMinimumWindowWidth - kMarginWidth);
+  top_label->SizeToFit(kMinimumWindowWidth - kMarginWidth * 2);
   contents_view->AddChildView(new EntryView(top_label));
 
   std::vector<Notifier*> notifiers;
   delegate_->GetNotifierList(&notifiers);
   for (size_t i = 0; i < notifiers.size(); ++i) {
     NotifierButton* button = new NotifierButton(notifiers[i], this);
-    contents_view->AddChildView(new EntryView(button));
+    EntryView* entry = new EntryView(button);
+    entry->set_focusable(true);
+    contents_view->AddChildView(entry);
     buttons_.insert(button);
   }
   scroller_->SetContents(contents_view);
@@ -299,6 +307,14 @@ bool NotifierSettingsView::CanResize() const {
 void NotifierSettingsView::Layout() {
   int title_height = title_entry_->GetPreferredSize().height();
   title_entry_->SetBounds(0, 0, width(), title_height);
+  views::View* contents_view = scroller_->contents();

[dewittj, 2013/04/05 20:48:41]

You are more familiar with scroll views than me but it strikes me as strange
that we are manually resizing the contents view instead of telling the
scrollview that it has a different amount of room now. 

[Jun Mukai, 2013/04/06 03:14:04]

That is a tricky part of the scroll view.
The size change of the ScrollView changes the visible rectangle size of its
contents, and that has no relationship with the layout of the contents view.
So essentially, if the contents view is wider than the width of scroll view, it
will show the horizontal scroll bar rather than changing the width of contents
view.

This is good to ideal scrollers, but we want to allow the vertical scroll bar
only here and wrap the text properly. This actually needs to change the width of
the contents view properly.

+  int contents_width = width();
+  int contents_height = contents_view->GetHeightForWidth(contents_width);

[dharcourt, 2013/04/05 16:07:03]

Nit: Same contents_width/contents_height -> content_width/content_height.

[Jun Mukai, 2013/04/06 03:14:04]

Done.

+  if (title_height + contents_height > kMinimumWindowHeight) {
+    contents_width -= scroller_->GetScrollBarWidth();
+    contents_height = contents_view->GetHeightForWidth(contents_width);
+  }
+  contents_view->SetBounds(0, 0, contents_width, contents_height);
   scroller_->SetBounds(0, title_height, width(), height() - title_height);
 }