Make Web Intents picker in Views conform to latest mocks

chrome/browser/ui/views/constrained_window_frame_simple.cc

Index: chrome/browser/ui/views/constrained_window_frame_simple.cc
diff --git a/chrome/browser/ui/views/constrained_window_frame_simple.cc b/chrome/browser/ui/views/constrained_window_frame_simple.cc
index 4b480cdef40434c217bb491d18282b1995b731be..8baa21446a3c504b6ee4f9e5c6237428fb1c665f 100644
--- a/chrome/browser/ui/views/constrained_window_frame_simple.cc
+++ b/chrome/browser/ui/views/constrained_window_frame_simple.cc
@@ -27,140 +27,73 @@
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 
-namespace {
-
-typedef ConstrainedWindowFrameSimple::HeaderViews HeaderViews;
-
-// A layout manager that lays out the header view with proper padding,
-// and sized to the widget's client view.
-class HeaderLayout : public views::LayoutManager {
- public:
-  explicit HeaderLayout() {}
-  virtual ~HeaderLayout() {}
-
-  // Overridden from LayoutManager
-  virtual void Layout(views::View* host);
-  virtual gfx::Size GetPreferredSize(views::View* host);
-
-  DISALLOW_COPY_AND_ASSIGN(HeaderLayout);
-};
-
-void HeaderLayout::Layout(views::View* host) {
-  if (!host->has_children())
-    return;
-
-  int top_padding = ConstrainedWindowConstants::kCloseButtonPadding;
-  int left_padding = ConstrainedWindowConstants::kHorizontalPadding;
-  int right_padding = ConstrainedWindowConstants::kCloseButtonPadding;
-
-  views::View* header = host->child_at(0);
-  gfx::Size preferred_size = GetPreferredSize(host);
-  int width = preferred_size.width() - left_padding - right_padding;
-  int height = preferred_size.height() - top_padding;
-
-  header->SetBounds(left_padding, top_padding, width, height);
-}
-
-gfx::Size HeaderLayout::GetPreferredSize(views::View* host) {
-  int top_padding = ConstrainedWindowConstants::kCloseButtonPadding;
-  int left_padding = ConstrainedWindowConstants::kHorizontalPadding;
-  int right_padding = ConstrainedWindowConstants::kCloseButtonPadding;
-
-  views::View* header = host->child_at(0);
-  gfx::Size header_size = header ? header->GetPreferredSize() : gfx::Size();
-  int width = std::max(host->GetPreferredSize().width(),
-      left_padding + header_size.width() + right_padding);
-  int height = header_size.height() + top_padding;
-
-  return gfx::Size(width, height);
-}
-
-}  // namespace
-
-ConstrainedWindowFrameSimple::HeaderViews::HeaderViews(
-    views::View* header,
-    views::Label* title_label,
-    views::Button* close_button)
-    : header(header),
-      title_label(title_label),
-      close_button(close_button) {
-  DCHECK(header);
-}
+using namespace views;

[Peter Kasting, 2012/10/16 02:26:53]

This is banned by the Google style guide.

[please use gerrit instead, 2012/10/16 16:29:38]

Changed to explicit statements:

using views::ImageButton;
using views::Label;
using views::View;

Done.

 
 ConstrainedWindowFrameSimple::ConstrainedWindowFrameSimple(
     ConstrainedWindowViews* container)
-    : container_(container) {
-  container_->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM);
-
-  layout_ = new HeaderLayout();
-  SetLayoutManager(layout_);
-
-  SetHeaderView(CreateDefaultHeaderView());
-
-  set_background(views::Background::CreateSolidBackground(
-      ConstrainedWindow::GetBackgroundColor()));
-
-  set_border(views::Border::CreateEmptyBorder(
-      ConstrainedWindowConstants::kClientTopPadding,
-      ConstrainedWindowConstants::kHorizontalPadding,
-      ConstrainedWindowConstants::kClientBottomPadding,
-      ConstrainedWindowConstants::kHorizontalPadding));
-}
-
-ConstrainedWindowFrameSimple::~ConstrainedWindowFrameSimple() {
-}
-
-void ConstrainedWindowFrameSimple::SetHeaderView(HeaderViews* header_views)
-{
-  RemoveAllChildViews(true);
-
-  header_views_.reset(header_views);
-
-  AddChildView(header_views_->header);
-}
-
-HeaderViews* ConstrainedWindowFrameSimple::CreateDefaultHeaderView() {
-  const int kTitleTopPadding = ConstrainedWindowConstants::kTitleTopPadding -
+    : container_(container),
+      title_label_(new Label(container->widget_delegate()->GetWindowTitle())),
+      ALLOW_THIS_IN_INITIALIZER_LIST(close_button_(new ImageButton(this))),
+      header_(new View()) {
+  const int kHeaderTopPadding = ConstrainedWindowConstants::kCloseButtonPadding;

[Peter Kasting, 2012/10/16 02:26:53]

Nit: It'd be technically more correct to do:

  const int kHeaderTopPadding = std::min(
      ConstrainedWindowConstants::kCloseButtonPadding,
      ConstrainedWindowConstants::kTitleTopPadding);

...and then subtract kHeaderTopPadding from the top padding for both the title
and close button.  The practical effect is the same but this would continue to
work correctly if for some reason we reversed those two padding values.

[please use gerrit instead, 2012/10/16 16:29:38]

Done.

+  const int kHeaderLeftPadding = ConstrainedWindowConstants::kHorizontalPadding;
+  const int kHeaderRightPadding =
       ConstrainedWindowConstants::kCloseButtonPadding;

[Peter Kasting, 2012/10/16 02:26:53]

Nit: I'd eliminate these two constants and just inline their values into the
SetInsets() call, since the values are already named constants.

[please use gerrit instead, 2012/10/16 16:29:38]

Done.

-  const int kTitleLeftPadding = 0;
-  const int kTitleBottomPadding = 0;
-  const int kTitleRightPadding = 0;
 
-  views::View* header_view = new views::View;
-
-  views::GridLayout* grid_layout = new views::GridLayout(header_view);
-  header_view->SetLayoutManager(grid_layout);
-
-  views::ColumnSet* header_cs = grid_layout->AddColumnSet(0);
-  header_cs->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
-                       views::GridLayout::USE_PREF, 0, 0);  // Title.
-  header_cs->AddPaddingColumn(1, views::kUnrelatedControlHorizontalSpacing);
-  header_cs->AddColumn(views::GridLayout::TRAILING, views::GridLayout::LEADING,
-                       0, views::GridLayout::USE_PREF, 0, 0);  // Close Button.
+  // Need to remove header padding from the title padding to achieve the desired
+  // result.
+  const int kTitleTopPadding = ConstrainedWindowConstants::kTitleTopPadding -
+      kHeaderTopPadding;

[Peter Kasting, 2012/10/16 02:26:53]

Nit: I'd remove the comment and inline this calculation into the
CreateEmptyBorder() call.  I don't think the comment adds anything to the code.

[please use gerrit instead, 2012/10/16 16:29:38]

Done.

 
-  // Header row.
+  container_->set_frame_type(Widget::FRAME_TYPE_FORCE_CUSTOM);
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  grid_layout->StartRow(0, 0);
 
-  views::Label* title_label = new views::Label();
-  title_label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-  title_label->SetFont(rb.GetFont(ConstrainedWindowConstants::kTitleFontStyle));
-  title_label->SetEnabledColor(ConstrainedWindow::GetTextColor());
-  title_label->SetText(container_->widget_delegate()->GetWindowTitle());
-  title_label->set_border(views::Border::CreateEmptyBorder(kTitleTopPadding,
-      kTitleLeftPadding, kTitleBottomPadding, kTitleRightPadding));
-  grid_layout->AddView(title_label);
+  title_label_->SetHorizontalAlignment(Label::ALIGN_LEFT);
+  title_label_->SetFont(rb.GetFont(
+      ConstrainedWindowConstants::kTitleFontStyle));
+  title_label_->SetEnabledColor(ConstrainedWindow::GetTextColor());
+  title_label_->set_border(Border::CreateEmptyBorder(
+      kTitleTopPadding, 0, 0, 0));
 
-  views::Button* close_button = CreateCloseButton();
-  grid_layout->AddView(close_button);
+  close_button_->SetImage(CustomButton::BS_NORMAL,
+                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X));
+  close_button_->SetImage(CustomButton::BS_HOT,
+                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_HOVER));
+  close_button_->SetImage(CustomButton::BS_PUSHED,
+                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_PRESSED));
 
-  return new HeaderViews(header_view, title_label, close_button);
+  GridLayout* header_layout = new GridLayout(header_);
+  header_layout->SetInsets(kHeaderTopPadding, kHeaderLeftPadding, 0,
+                           kHeaderRightPadding);
+  header_->SetLayoutManager(header_layout);
+  ColumnSet* cs = header_layout->AddColumnSet(0);
+  cs->AddColumn(GridLayout::FILL, GridLayout::CENTER, 1,
+                GridLayout::USE_PREF, 0, 0);  // Title.
+  cs->AddPaddingColumn(0, ConstrainedWindowConstants::kCloseButtonPadding);
+  cs->AddColumn(GridLayout::TRAILING, GridLayout::LEADING, 0,
+                GridLayout::USE_PREF, 0, 0);  // Close Button.
+  header_layout->StartRow(0, 0);
+  header_layout->AddView(title_label_);
+  header_layout->AddView(close_button_);
+
+  GridLayout* layout = new GridLayout(this);

[Peter Kasting, 2012/10/16 02:26:53]

What does this GridLayout do for us?  It seems like we could get rid of this
entirely, couldn't we?

[please use gerrit instead, 2012/10/16 16:29:38]

Done. This dramatically simplifies the code.

+  SetLayoutManager(layout);
+  layout->AddColumnSet(0)->AddColumn(
+      GridLayout::FILL, GridLayout::LEADING, 1,
+      GridLayout::USE_PREF, 0, 0);
+  layout->StartRow(0, 0);
+  layout->AddView(header_);
+  Layout();
+
+  set_background(Background::CreateSolidBackground(
+      ConstrainedWindow::GetBackgroundColor()));
+}
+
+ConstrainedWindowFrameSimple::~ConstrainedWindowFrameSimple() {
 }
 
 gfx::Rect ConstrainedWindowFrameSimple::GetBoundsForClientView() const {
   gfx::Rect bounds(GetContentsBounds());
-  if (header_views_->header)
-    bounds.Inset(0, header_views_->header->GetPreferredSize().height(), 0, 0);
   return bounds;
 }
 
@@ -168,8 +101,8 @@ gfx::Rect ConstrainedWindowFrameSimple::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) const {
   gfx::Rect bounds(client_bounds);
   bounds.Inset(-GetInsets());
-  if (header_views_->header)
-    bounds.Inset(0, -header_views_->header->GetPreferredSize().height(), 0, 0);
+  bounds.set_width(std::max(bounds.width(),
+                            header_->GetPreferredSize().width()));
   return bounds;
 }
 
@@ -204,32 +137,30 @@ void ConstrainedWindowFrameSimple::UpdateWindowIcon() {
 }
 
 void ConstrainedWindowFrameSimple::UpdateWindowTitle() {
-  if (!header_views_->title_label)
+  if (!title_label_)
     return;
 
   string16 text = container_->widget_delegate()->GetWindowTitle();
-  header_views_->title_label->SetText(text);
+  title_label_->SetText(text);
+}
+
+void ConstrainedWindowFrameSimple::OnBoundsChanged(
+    const gfx::Rect& previous_bounds) {
+  gfx::Rect header_bounds(header_->bounds());
+  header_bounds.set_width(std::max(header_->GetPreferredSize().width(),
+                                   bounds().width()));
+  header_->SetBounds(header_bounds.x(), header_bounds.y(),
+                     header_bounds.width(), header_bounds.height());
+  NonClientFrameView::OnBoundsChanged(previous_bounds);
 }
 
 gfx::Size ConstrainedWindowFrameSimple::GetPreferredSize() {
-  return container_->non_client_view()->GetWindowBoundsForClientBounds(
+  return GetWindowBoundsForClientBounds(
       gfx::Rect(container_->client_view()->GetPreferredSize())).size();
 }
 
-void ConstrainedWindowFrameSimple::ButtonPressed(views::Button* sender,
+void ConstrainedWindowFrameSimple::ButtonPressed(Button* sender,
                                                  const ui::Event& event) {
-  if (header_views_->close_button && sender == header_views_->close_button)
+  if (sender == close_button_)
     sender->GetWidget()->Close();
 }
-
-views::ImageButton* ConstrainedWindowFrameSimple::CreateCloseButton() {
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  views::ImageButton* close_button = new views::ImageButton(this);
-  close_button->SetImage(views::CustomButton::BS_NORMAL,
-                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X));
-  close_button->SetImage(views::CustomButton::BS_HOT,
-                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_HOVER));
-  close_button->SetImage(views::CustomButton::BS_PUSHED,
-                          rb.GetImageSkiaNamed(IDR_SHARED_IMAGES_X_PRESSED));
-  return close_button;
-}