Re-land r51526

Auto-size the views version of the options dialog.

This adds support for auto-sizing tab controls, adjusts the options dialog to
use it and takes care of any fallout due to the new layout handling. Also fixes
a couple of small bugs in the views Layout() machinery  and sanitizes layouting
of options pages.

BUG=36497
TEST=unit tests in tabbed_pane_unittest.cc and grid_layout_unittest.cc, as well as checking the options dialog in any supported language.

original issuse: http://codereview.chromium.org/2812026/show

Review URL: http://codereview.chromium.org/2883022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51628 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 94 insertions, 93 deletions

diff --git a/chrome/browser/chromeos/options/options_window_view.cc b/chrome/browser/chromeos/options/options_window_view.cc
index d284ce3..4535e8b 100644
--- a/chrome/browser/chromeos/options/options_window_view.cc
+++ b/chrome/browser/chromeos/options/options_window_view.cc
@@ -35,6 +35,48 @@
 namespace chromeos {
 
 ///////////////////////////////////////////////////////////////////////////////
+// GtkPreferencePageHost
+//
+//  Hosts a GTK preference page and takes care of sizing it appropriately.
+//
+class GtkPreferencePageHost : public views::NativeViewHost {
+ public:
+  explicit GtkPreferencePageHost(GtkWidget* widget);
+
+ private:
+  // views::View overrides:
+  virtual void ViewHierarchyChanged(bool is_add, View* parent, View* child);
+  virtual gfx::Size GetPreferredSize();
+
+  GtkWidget* widget_;
+
+  DISALLOW_COPY_AND_ASSIGN(GtkPreferencePageHost);
+};
+
+GtkPreferencePageHost::GtkPreferencePageHost(GtkWidget* widget)
+    : widget_(widget) {
+  set_background(views::Background::CreateSolidBackground(SK_ColorWHITE));
+}
+
+void GtkPreferencePageHost::ViewHierarchyChanged(bool is_add,
+                                                 View* parent,
+                                                 View* child) {
+  NativeViewHost::ViewHierarchyChanged(is_add, parent, child);
+  if (is_add && child == this)
+    Attach(widget_);
+}
+
+gfx::Size GtkPreferencePageHost::GetPreferredSize() {
+  // We need to show the widget and its children since otherwise containers like
+  // gtk_box don't compute the correct size.
+  gtk_widget_show_all(widget_);
+  GtkRequisition requisition = { 0, 0 };
+  gtk_widget_size_request(GTK_WIDGET(widget_), &requisition);
+  GtkRequisition& size(widget_->requisition);
+  return gfx::Size(size.width, size.height);
+}
+
+///////////////////////////////////////////////////////////////////////////////
 // OptionsWindowView
 //
 //  The contents of the Options dialog window.
@@ -214,9 +256,9 @@ void OptionsWindowView::Layout() {
 }
 
 gfx::Size OptionsWindowView::GetPreferredSize() {
-  return gfx::Size(views::Window::GetLocalizedContentsSize(
-      IDS_OPTIONS_DIALOG_WIDTH_CHARS,
-      IDS_OPTIONS_DIALOG_HEIGHT_LINES));
+  gfx::Size size(tabs_->GetPreferredSize());
+  size.Enlarge(2 * kDialogPadding, 2 * kDialogPadding);
+  return size;
 }
 
 void OptionsWindowView::ViewHierarchyChanged(bool is_add,
@@ -263,29 +305,23 @@ void OptionsWindowView::Init() {
                        l10n_util::GetString(IDS_OPTIONS_INTERNET_TAB_LABEL),
                        internet_page, false);
 
-  views::NativeViewHost* general_page_view = new views::NativeViewHost();
-  general_page_view->set_background(views::Background::CreateSolidBackground(
-      SK_ColorWHITE));
   tabs_->AddTabAtIndex(tab_index++,
                        l10n_util::GetString(IDS_OPTIONS_GENERAL_TAB_LABEL),
-                       general_page_view, false);
-  general_page_view->Attach(general_page_.get_page_widget());
+                       new GtkPreferencePageHost(
+                           general_page_.get_page_widget()),
+                       false);
 
-  views::NativeViewHost* content_page_view = new views::NativeViewHost();
-  content_page_view->set_background(views::Background::CreateSolidBackground(
-      SK_ColorWHITE));
   tabs_->AddTabAtIndex(tab_index++,
                        l10n_util::GetString(IDS_OPTIONS_CONTENT_TAB_LABEL),
-                       content_page_view, false);
-  content_page_view->Attach(content_page_.get_page_widget());
+                       new GtkPreferencePageHost(
+                           content_page_.get_page_widget()),
+                       false);
 
-  views::NativeViewHost* advanced_page_view = new views::NativeViewHost();
-  advanced_page_view->set_background(views::Background::CreateSolidBackground(
-      SK_ColorWHITE));
   tabs_->AddTabAtIndex(tab_index++,
                        l10n_util::GetString(IDS_OPTIONS_ADVANCED_TAB_LABEL),
-                       advanced_page_view, false);
-  advanced_page_view->Attach(advanced_page_.get_page_widget());
+                       new GtkPreferencePageHost(
+                           advanced_page_.get_page_widget()),
+                       false);
 
   DCHECK(tabs_->GetTabCount() == OPTIONS_PAGE_COUNT);
 
@@ -344,4 +380,3 @@ void ShowOptionsWindow(OptionsPage page,
 
   OptionsWindowView::instance_->ShowOptionsPage(page, highlight_group);
 }
-
diff --git a/chrome/browser/views/options/advanced_contents_view.cc b/chrome/browser/views/options/advanced_contents_view.cc
index 9ee6485e..bbe232f 100644
--- a/chrome/browser/views/options/advanced_contents_view.cc
+++ b/chrome/browser/views/options/advanced_contents_view.cc
@@ -287,7 +287,6 @@ AdvancedSection::AdvancedSection(Profile* profile,
 void AdvancedSection::DidChangeBounds(const gfx::Rect& previous,
                                       const gfx::Rect& current) {
   Layout();
-  contents_->Layout();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -434,9 +433,6 @@ class PrivacySection : public AdvancedSection,
   // Overridden from views::LinkController:
   virtual void LinkActivated(views::Link* source, int event_flags);
 
-  // Overridden from views::View:
-  virtual void Layout();
-
  protected:
   // OptionsPageView overrides:
   virtual void InitControlLayout();
@@ -549,22 +545,6 @@ void PrivacySection::LinkActivated(views::Link* source, int event_flags) {
   }
 }
 
-void PrivacySection::Layout() {
-  if (reporting_enabled_checkbox_) {
-    // We override this to try and set the width of the enable logging checkbox
-    // to the width of the parent less some fudging since the checkbox's
-    // preferred size calculation code is dependent on its width, and if we
-    // don't do this then it will return 0 as a preferred width when GridLayout
-    // (called from View::Layout) tries to access it.
-    views::View* parent = GetParent();
-    if (parent && parent->width()) {
-      const int parent_width = parent->width();
-      reporting_enabled_checkbox_->SetBounds(0, 0, parent_width - 20, 0);
-    }
-  }
-  View::Layout();
-}
-
 void PrivacySection::InitControlLayout() {
   AdvancedSection::InitControlLayout();
 
@@ -636,8 +616,8 @@ void PrivacySection::InitControlLayout() {
                          reporting_enabled_checkbox_ != NULL);
   // The "Help make Google Chrome better" checkbox.
   if (reporting_enabled_checkbox_) {
-    AddLeadingControl(layout, reporting_enabled_checkbox_, indented_view_set_id,
-                      false);
+    AddWrappingCheckboxRow(layout, reporting_enabled_checkbox_,
+                           indented_view_set_id, false);
   }
 
   // Init member prefs so we can update the controls if prefs change.
@@ -1421,5 +1401,4 @@ void AdvancedScrollViewContainer::Layout() {
       gfx::NativeTheme::LIST);
   lb.Inset(border.width(), border.height());
   scroll_view_->SetBounds(lb);
-  scroll_view_->Layout();
 }
diff --git a/chrome/browser/views/options/content_page_view.cc b/chrome/browser/views/options/content_page_view.cc
index add146e..04ccf65 100644
--- a/chrome/browser/views/options/content_page_view.cc
+++ b/chrome/browser/views/options/content_page_view.cc
@@ -41,8 +41,10 @@
 
 namespace {
 
-const int kPasswordSavingRadioGroup = 1;
-const int kFormAutofillRadioGroup = 2;
+// All the options pages are in the same view hierarchy. This means we need to
+// make sure group identifiers don't collide across different pages.
+const int kPasswordSavingRadioGroup = 201;
+const int kFormAutofillRadioGroup = 202;
 
 // Background color for the status label when it's showing an error.
 static const SkColor kSyncLabelErrorBgColor = SkColorSetRGB(0xff, 0x9a, 0x9a);
@@ -252,17 +254,6 @@ void ContentPageView::NotifyPrefChanged(const std::wstring* pref_name) {
 void ContentPageView::Layout() {
   if (is_initialized())
     UpdateSyncControls();
-  // We need to Layout twice - once to get the width of the contents box...
-  View::Layout();
-  passwords_asktosave_radio_->SetBounds(
-      0, 0, passwords_group_->GetContentsWidth(), 0);
-  passwords_neversave_radio_->SetBounds(
-      0, 0, passwords_group_->GetContentsWidth(), 0);
-  if (is_initialized()) {
-    sync_status_label_->SetBounds(
-        0, 0, sync_group_->GetContentsWidth(), 0);
-  }
-  // ... and twice to get the height of multi-line items correct.
   View::Layout();
 }
 
@@ -308,10 +299,12 @@ void ContentPageView::InitPasswordSavingGroup() {
                         GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(passwords_asktosave_radio_);
+  layout->AddView(passwords_asktosave_radio_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(passwords_neversave_radio_);
+  layout->AddView(passwords_neversave_radio_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kUnrelatedControlVerticalSpacing);
   layout->StartRow(0, single_column_view_set_id);
   layout->AddView(show_passwords_button_);
@@ -356,10 +349,12 @@ void ContentPageView::InitFormAutofillGroup() {
                         GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, fill_column_view_set_id);
-  layout->AddView(form_autofill_enable_radio_);
+  layout->AddView(form_autofill_enable_radio_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, fill_column_view_set_id);
-  layout->AddView(form_autofill_disable_radio_);
+  layout->AddView(form_autofill_disable_radio_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, leading_column_view_set_id);
   layout->AddView(change_autofill_settings_button_);
@@ -457,7 +452,8 @@ void ContentPageView::InitSyncGroup() {
                         GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(sync_status_label_, 3, 1);
+  layout->AddView(sync_status_label_, 3, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->StartRow(0, single_column_view_set_id);
   layout->AddView(sync_action_link_, 3, 1);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
diff --git a/chrome/browser/views/options/general_page_view.cc b/chrome/browser/views/options/general_page_view.cc
index 20504c9..f836ea4 100644
--- a/chrome/browser/views/options/general_page_view.cc
+++ b/chrome/browser/views/options/general_page_view.cc
@@ -35,10 +35,13 @@
 
 namespace {
 
-const int kStartupRadioGroup = 1;
-const int kHomePageRadioGroup = 2;
+// All the options pages are in the same view hierarchy. This means we need to
+// make sure group identifiers don't collide across different pages.
+const int kStartupRadioGroup = 101;
+const int kHomePageRadioGroup = 102;
 const SkColor kDefaultBrowserLabelColor = SkColorSetRGB(0, 135, 0);
 const SkColor kNotDefaultBrowserLabelColor = SkColorSetRGB(135, 0, 0);
+const int kHomePageTextfieldWidthChars = 40;
 
 }  // namespace
 
@@ -402,24 +405,6 @@ void GeneralPageView::HighlightGroup(OptionsGroup highlight_group) {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-// GeneralPageView, views::View overrides:
-
-void GeneralPageView::Layout() {
-  // We need to Layout twice - once to get the width of the contents box...
-  View::Layout();
-  startup_last_session_radio_->SetBounds(
-      0, 0, startup_group_->GetContentsWidth(), 0);
-  homepage_use_newtab_radio_->SetBounds(
-      0, 0, homepage_group_->GetContentsWidth(), 0);
-  homepage_show_home_button_checkbox_->SetBounds(
-      0, 0, homepage_group_->GetContentsWidth(), 0);
-  default_browser_status_label_->SetBounds(
-      0, 0, default_browser_group_->GetContentsWidth(), 0);
-  // ... and twice to get the height of multi-line items correct.
-  View::Layout();
-}
-
-///////////////////////////////////////////////////////////////////////////////
 // GeneralPageView, private:
 
 void GeneralPageView::SetDefaultBrowserUIState(
@@ -512,14 +497,16 @@ void GeneralPageView::InitStartupGroup() {
   layout->AddView(startup_homepage_radio_);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(startup_last_session_radio_);
+  layout->AddView(startup_last_session_radio_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, single_column_view_set_id);
   layout->AddView(startup_custom_radio_);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
 
   layout->StartRow(0, double_column_view_set_id);
-  layout->AddView(startup_custom_pages_table_);
+  layout->AddView(startup_custom_pages_table_, 1, 1,
+                  GridLayout::FILL, GridLayout::FILL);
 
   views::View* button_stack = new views::View;
   GridLayout* button_stack_layout = new GridLayout(button_stack);
@@ -558,6 +545,8 @@ void GeneralPageView::InitHomepageGroup() {
   homepage_use_url_radio_->set_listener(this);
   homepage_use_url_textfield_ = new views::Textfield;
   homepage_use_url_textfield_->SetController(this);
+  homepage_use_url_textfield_->set_default_width_in_chars(
+      kHomePageTextfieldWidthChars);
   homepage_show_home_button_checkbox_ = new views::Checkbox(
       l10n_util::GetString(IDS_OPTIONS_HOMEPAGE_SHOW_BUTTON));
   homepage_show_home_button_checkbox_->set_listener(this);
@@ -584,14 +573,16 @@ void GeneralPageView::InitHomepageGroup() {
                         GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(homepage_use_newtab_radio_);
+  layout->AddView(homepage_use_newtab_radio_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, double_column_view_set_id);
   layout->AddView(homepage_use_url_radio_);
   layout->AddView(homepage_use_url_textfield_);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(homepage_show_home_button_checkbox_);
+  layout->AddView(homepage_show_home_button_checkbox_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
 
   homepage_group_ = new OptionsGroupView(
       contents, l10n_util::GetString(IDS_OPTIONS_HOMEPAGE_GROUP_NAME),
@@ -657,7 +648,8 @@ void GeneralPageView::InitDefaultBrowserGroup() {
                         GridLayout::USE_PREF, 0, 0);
 
   layout->StartRow(0, single_column_view_set_id);
-  layout->AddView(default_browser_status_label_);
+  layout->AddView(default_browser_status_label_, 1, 1,
+                  GridLayout::FILL, GridLayout::LEADING);
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
   layout->StartRow(0, single_column_view_set_id);
   layout->AddView(default_browser_use_as_default_button_);
diff --git a/chrome/browser/views/options/general_page_view.h b/chrome/browser/views/options/general_page_view.h
index 7838fbe..6cc4911 100644
--- a/chrome/browser/views/options/general_page_view.h
+++ b/chrome/browser/views/options/general_page_view.h
@@ -62,9 +62,6 @@ class GeneralPageView : public OptionsPageView,
   virtual void NotifyPrefChanged(const std::wstring* pref_name);
   virtual void HighlightGroup(OptionsGroup highlight_group);
 
-  // views::View overrides:
-  virtual void Layout();
-
  private:
   // ShellIntegration::DefaultBrowserObserver implementation:
   // Updates the UI state to reflect the current default browser state.
diff --git a/chrome/browser/views/options/options_window_view.cc b/chrome/browser/views/options/options_window_view.cc
index 76c219b..70a043d 100644
--- a/chrome/browser/views/options/options_window_view.cc
+++ b/chrome/browser/views/options/options_window_view.cc
@@ -183,9 +183,9 @@ void OptionsWindowView::Layout() {
 }
 
 gfx::Size OptionsWindowView::GetPreferredSize() {
-  return gfx::Size(views::Window::GetLocalizedContentsSize(
-      IDS_OPTIONS_DIALOG_WIDTH_CHARS,
-      IDS_OPTIONS_DIALOG_HEIGHT_LINES));
+  gfx::Size size(tabs_->GetPreferredSize());
+  size.Enlarge(2 * kDialogPadding, 2 * kDialogPadding);
+  return size;
 }
 
 void OptionsWindowView::ViewHierarchyChanged(bool is_add,
diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc
index b3e8194d..b7915b1 100644
--- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc
+++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc
@@ -217,8 +217,10 @@ void TabContentsViewGtk::GetContainerBounds(gfx::Rect* out) const {
 
   // Callers expect the requested bounds not the actual bounds. For example,
   // during init callers expect 0x0, but Gtk layout enforces a min size of 1x1.
-  out->set_width(GetNativeView()->requisition.width);
-  out->set_height(GetNativeView()->requisition.height);
+  GtkRequisition requisition;
+  gtk_widget_get_child_requisition(GetNativeView(), &requisition);
+  out->set_width(requisition.width);
+  out->set_height(requisition.height);
 }
 
 void TabContentsViewGtk::StartDragging(const WebDropData& drop_data,