Fixes bug in clear browsing data dialog where throbber would overlap

flash link.

BUG=35009
TEST=bring up clear browsing data, click clear browsing data and make
     sure throbber doesn't overlap flash link.

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

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

5 files changed, 54 insertions, 56 deletions

diff --git a/chrome/browser/views/clear_browsing_data.cc b/chrome/browser/views/clear_browsing_data.cc
index 1d0450d..4ad0e93 100644
--- a/chrome/browser/views/clear_browsing_data.cc
+++ b/chrome/browser/views/clear_browsing_data.cc
@@ -25,6 +25,8 @@
 #include "views/window/dialog_client_view.h"
 #include "views/window/window.h"
 
+using views::GridLayout;
+
 // The combo box is vertically aligned to the 'time-period' label, which makes
 // the combo box look a little too close to the check box above it when we use
 // standard layout to separate them. We therefore add a little extra margin to
@@ -73,13 +75,12 @@ ClearBrowsingDataView::~ClearBrowsingDataView(void) {
 void ClearBrowsingDataView::Init() {
   // Views we will add to the *parent* of this dialog, since it will display
   // next to the buttons which we don't draw ourselves.
-  throbber_.reset(new views::Throbber(50, true));
-  throbber_->set_parent_owned(false);
+  throbber_ = new views::Throbber(50, true);
   throbber_->SetVisible(false);
 
-  status_label_.SetText(l10n_util::GetString(IDS_CLEAR_DATA_DELETING));
-  status_label_.SetVisible(false);
-  status_label_.set_parent_owned(false);
+  status_label_ = new views::Label(
+      l10n_util::GetString(IDS_CLEAR_DATA_DELETING));
+  status_label_->SetVisible(false);
 
   // Regular view controls we draw by ourself. First, we add the dialog label.
   delete_all_label_ = new views::Label(
@@ -122,6 +123,28 @@ void ClearBrowsingDataView::Init() {
                                          prefs::kDeleteTimePeriod));
   time_period_combobox_->set_listener(this);
   AddChildView(time_period_combobox_);
+
+  // Create the throbber and related views. The throbber and status link are
+  // contained in throbber_view_, which is positioned by DialogClientView right
+  // next to the buttons.
+  throbber_view_ = new views::View();
+
+  GridLayout* layout = new GridLayout(throbber_view_);
+  throbber_view_->SetLayoutManager(layout);
+  views::ColumnSet* column_set = layout->AddColumnSet(0);
+  // DialogClientView positions the extra view at kButtonHEdgeMargin, but we
+  // put all our controls at kPanelHorizMargin. Add a padding column so things
+  // line up nicely.
+  if (kPanelHorizMargin - kButtonHEdgeMargin > 0)
+    column_set->AddPaddingColumn(0, kPanelHorizMargin - kButtonHEdgeMargin);
+  column_set->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
+                        GridLayout::USE_PREF, 0, 0);
+  column_set->AddPaddingColumn(0, kRelatedControlHorizontalSpacing);
+  column_set->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
+                        GridLayout::USE_PREF, 0, 0);
+  layout->StartRow(1, 0);
+  layout->AddView(throbber_);
+  layout->AddView(status_label_);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -203,47 +226,6 @@ void ClearBrowsingDataView::Layout() {
                                    time_period_label_->y() -
                                        ((sz.height() - label_y_size) / 2),
                                    sz.width(), sz.height());
-
-  // Get the y-coordinate of our parent so we can position the throbber and
-  // status message at the bottom of the panel.
-  gfx::Rect parent_bounds = GetParent()->GetLocalBounds(false);
-
-  sz = throbber_->GetPreferredSize();
-  int throbber_topleft_x = kPanelHorizMargin;
-  int throbber_topleft_y = parent_bounds.bottom() - sz.height() -
-                           kButtonVEdgeMargin - 3;
-  throbber_->SetBounds(throbber_topleft_x, throbber_topleft_y, sz.width(),
-                       sz.height());
-
-  // The status label should be at the bottom of the screen, to the right of
-  // the throbber.
-  sz = status_label_.GetPreferredSize();
-  int status_label_x = throbber_->x() + throbber_->width() +
-                       kRelatedControlHorizontalSpacing;
-  status_label_.SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-  status_label_.SetBounds(status_label_x,
-                          throbber_topleft_y + 1,
-                          sz.width(),
-                          sz.height());
-}
-
-void ClearBrowsingDataView::ViewHierarchyChanged(bool is_add,
-                                                 views::View* parent,
-                                                 views::View* child) {
-  // Since we want the some of the controls to show up in the same visual row
-  // as the buttons, which are provided by the framework, we must add the
-  // buttons to the non-client view, which is the parent of this view.
-  // Similarly, when we're removed from the view hierarchy, we must take care
-  // to remove these items as well.
-  if (child == this) {
-    if (is_add) {
-      parent->AddChildView(&status_label_);
-      parent->AddChildView(throbber_.get());
-    } else {
-      parent->RemoveChildView(&status_label_);
-      parent->RemoveChildView(throbber_.get());
-    }
-  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -320,8 +302,6 @@ views::View* ClearBrowsingDataView::GetContentsView() {
 
 views::ClientView* ClearBrowsingDataView::CreateClientView(
     views::Window* window) {
-  using views::GridLayout;
-
   views::Link* flash_link =
       new views::Link(l10n_util::GetString(IDS_FLASH_STORAGE_SETTINGS));
   flash_link->SetController(this);
@@ -430,7 +410,7 @@ void ClearBrowsingDataView::UpdateControlEnabledState() {
   del_form_data_checkbox_->SetEnabled(!delete_in_progress_);
   time_period_combobox_->SetEnabled(!delete_in_progress_);
 
-  status_label_.SetVisible(delete_in_progress_);
+  status_label_->SetVisible(delete_in_progress_);
   throbber_->SetVisible(delete_in_progress_);
   if (delete_in_progress_)
     throbber_->Start();
@@ -479,5 +459,5 @@ void ClearBrowsingDataView::OnBrowsingDataRemoverDone() {
   // No need to remove ourselves as an observer as BrowsingDataRemover deletes
   // itself after we return.
   remover_ = NULL;
-  window()->Close();
+  //  window()->Close();
 }
diff --git a/chrome/browser/views/clear_browsing_data.h b/chrome/browser/views/clear_browsing_data.h
index 1510f95..0030b28 100644
--- a/chrome/browser/views/clear_browsing_data.h
+++ b/chrome/browser/views/clear_browsing_data.h
@@ -48,9 +48,6 @@ class ClearBrowsingDataView : public views::View,
   // Overridden from views::View:
   virtual gfx::Size GetPreferredSize();
   virtual void Layout();
-  void ViewHierarchyChanged(bool is_add,
-                            views::View* parent,
-                            views::View* child);
 
   // Overridden from views::DialogDelegate:
   virtual int GetDefaultDialogButton() const;
@@ -67,6 +64,8 @@ class ClearBrowsingDataView : public views::View,
   virtual bool Accept();
   virtual views::View* GetContentsView();
   views::ClientView* CreateClientView(views::Window* window);
+  virtual views::View* GetExtraView() { return throbber_view_; }
+  virtual bool GetSizeExtraViewHeightToButtons() { return true; }
 
   // Overridden from ComboboxModel:
   virtual int GetItemCount();
@@ -98,8 +97,9 @@ class ClearBrowsingDataView : public views::View,
   virtual void OnBrowsingDataRemoverDone();
 
   // UI elements we add to the parent view.
-  scoped_ptr<views::Throbber> throbber_;
-  views::Label status_label_;
+  views::View* throbber_view_;
+  views::Throbber* throbber_;
+  views::Label* status_label_;
   // Other UI elements.
   views::Label* delete_all_label_;
   views::Checkbox* del_history_checkbox_;
diff --git a/views/window/dialog_client_view.cc b/views/window/dialog_client_view.cc
index 7ea51f1..3eac93c 100644
--- a/views/window/dialog_client_view.cc
+++ b/views/window/dialog_client_view.cc
@@ -110,6 +110,7 @@ DialogClientView::DialogClientView(Window* owner, View* contents_view)
       cancel_button_(NULL),
       default_button_(NULL),
       extra_view_(NULL),
+      size_extra_view_height_to_buttons_(false),
       accepted_(false),
       listening_to_focus_(false),
       saved_focus_manager_(NULL),
@@ -462,6 +463,7 @@ void DialogClientView::LayoutDialogButtons() {
   gfx::Rect lb = GetLocalBounds(false);
   gfx::Rect extra_bounds;
   int bottom_y = lb.bottom() - kButtonVEdgeMargin;
+  int button_height = 0;
   if (bottom_view_) {
     gfx::Size bottom_pref = bottom_view_->GetPreferredSize();
     bottom_y -= bottom_pref.height() + kButtonVEdgeMargin + kButtonVEdgeMargin;
@@ -476,6 +478,7 @@ void DialogClientView::LayoutDialogButtons() {
     // The extra view bounds are dependent on this button.
     extra_bounds.set_width(std::max(0, cancel_button_->x()));
     extra_bounds.set_y(cancel_button_->y());
+    button_height = std::max(button_height, ps.height());
   }
   if (ok_button_) {
     gfx::Size ps = ok_button_->GetPreferredSize();
@@ -491,11 +494,14 @@ void DialogClientView::LayoutDialogButtons() {
     // The extra view bounds are dependent on this button.
     extra_bounds.set_width(std::max(0, ok_button_->x()));
     extra_bounds.set_y(ok_button_->y());
+    button_height = std::max(button_height, ps.height());
   }
   if (extra_view_) {
     gfx::Size ps = extra_view_->GetPreferredSize();
     extra_bounds.set_x(lb.x() + kButtonHEdgeMargin);
-    extra_bounds.set_height(ps.height());
+    int height = size_extra_view_height_to_buttons_ ?
+        std::max(ps.height(), button_height) : ps.height();
+    extra_bounds.set_height(height);
     extra_view_->SetBounds(extra_bounds);
   }
 }
@@ -513,6 +519,8 @@ void DialogClientView::CreateExtraView() {
     extra_view_ = extra_view;
     extra_view_->SetGroup(kButtonGroup);
     AddChildView(extra_view_);
+    size_extra_view_height_to_buttons_ =
+        GetDialogDelegate()->GetSizeExtraViewHeightToButtons();
   }
 }
 
diff --git a/views/window/dialog_client_view.h b/views/window/dialog_client_view.h
index e23e346..193ce1f 100644
--- a/views/window/dialog_client_view.h
+++ b/views/window/dialog_client_view.h
@@ -126,6 +126,10 @@ class DialogClientView : public ClientView,
   // The button-level extra view, NULL unless the dialog delegate supplies one.
   View* extra_view_;
 
+  // See description of DialogDelegate::GetSizeExtraViewHeightToButtons for
+  // details on this.
+  bool size_extra_view_height_to_buttons_;
+
   // The layout rect of the size box, when visible.
   gfx::Rect size_box_bounds_;
 
diff --git a/views/window/dialog_delegate.h b/views/window/dialog_delegate.h
index 19c2a89..ebfde24 100644
--- a/views/window/dialog_delegate.h
+++ b/views/window/dialog_delegate.h
@@ -59,6 +59,12 @@ class DialogDelegate : public WindowDelegate {
   // up to the buttons.
   virtual View* GetExtraView() { return NULL; }
 
+  // Returns whether the height of the extra view should be at least as tall as
+  // the buttons. The default (false) is to give the extra view it's preferred
+  // height. By returning true the height becomes
+  // max(extra_view preferred height, buttons preferred height).
+  virtual bool GetSizeExtraViewHeightToButtons() { return false; }
+
   // Returns the default dialog button. This should not be a mask as only
   // one button should ever be the default button.  Return
   // MessageBoxFlags::DIALOGBUTTON_NONE if there is no default.  Default