Make Web Intents picker in Views conform to latest mocks

chrome/browser/ui/views/web_intent_picker_views.cc

Index: chrome/browser/ui/views/web_intent_picker_views.cc
diff --git a/chrome/browser/ui/views/web_intent_picker_views.cc b/chrome/browser/ui/views/web_intent_picker_views.cc
index 7c85fdf14d388a791e62e61d6d17e7d6e846050c..db0a800a4c3bc66c7e4a70afefc0f421b0500439 100644
--- a/chrome/browser/ui/views/web_intent_picker_views.cc
+++ b/chrome/browser/ui/views/web_intent_picker_views.cc
@@ -8,7 +8,9 @@
 #include "base/command_line.h"
 #include "base/memory/scoped_vector.h"
 #include "base/time.h"
+#include "base/timer.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_navigator.h"
@@ -47,6 +49,7 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 #include "ui/views/controls/link_listener.h"
+#include "ui/views/controls/separator.h"
 #include "ui/views/controls/throbber.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/layout/box_layout.h"
@@ -66,6 +69,12 @@ namespace {
 // The color used to dim disabled elements.
 const SkColor kHalfOpacityWhite = SkColorSetARGB(128, 255, 255, 255);
 
+// The color used to display an enabled label.
+const SkColor kEnabledLabelColor = SkColorSetRGB(51, 51, 51);
+
+// The color used to display an enabled link.
+const SkColor kEnabledLinkColor = SkColorSetRGB(17, 85, 204);
+
 // The color used to display a disabled link.
 const SkColor kDisabledLinkColor = SkColorSetRGB(128, 128, 128);
 
@@ -81,6 +90,12 @@ const int kMinRowCount = 4;
 // Maximum number of action buttons - do not add suggestions to reach.
 const int kMaxRowCount = 8;
 
+// Custom client insets for the constrained window.
+const int kConstrainedWindowClientTopInset = 10;
+const int kConstrainedWindowClientLeftInset = 0;
+const int kConstrainedWindowClientBottomInset = 0;
+const int kConstrainedWindowClientRightInset = 0;
+
 // Enables or disables all child views of |view|.
 void EnableChildViews(views::View* view, bool enabled) {
   for (int i = 0; i < view->child_count(); ++i) {
@@ -89,18 +104,32 @@ void EnableChildViews(views::View* view, bool enabled) {
   }
 }
 
-// Create a "close" button.
-views::ImageButton* CreateCloseButton(views::ButtonListener* listener) {
+// Creates a label.
+views::Label* CreateLabel() {
+  views::Label* label = new views::Label();
+  label->SetEnabledColor(kEnabledLabelColor);
+  label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);

[Peter Kasting, 2012/10/12 03:23:32]

Wow, I'm amazed this isn't the default.  I don't know where on earth we actually
want centered labels.

Consider filing a bug that someone should clean up Label to make its default
alignment "ALIGN_LEFT" and then fix any cases that actually want ALIGN_CENTER to
specify that.

[please use gerrit instead, 2012/10/14 18:40:45]

Filed http://crbug.com/155526.

+  return label;
+}
+
+// Creates a title-style label.
+views::Label* CreateTitleLabel() {
+  views::Label* label = CreateLabel();
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();

[Peter Kasting, 2012/10/12 03:23:32]

Nit: Or just inline into next line (I know this is a common pattern for resource
bundle usage but I never like using non-const refs)

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

-  views::ImageButton* close_button = new views::ImageButton(listener);
-  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_HOVER));
-  return close_button;
+  label->SetFont(rb.GetFont(ui::ResourceBundle::MediumFont));
+  return label;
+}
+
+// Creates a link.
+views::Link* CreateLink() {
+  views::Link* link = new views::Link();
+  link->SetEnabledColor(kEnabledLinkColor);
+  link->SetDisabledColor(kDisabledLinkColor);
+  link->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+  link->SetUnderlineOnHover(true);
+  return link;
 }
+
 // SarsView -------------------------------------------------------------------

[Peter Kasting, 2012/10/12 03:23:32]

Nit: While here, can you fix Sars -> Stars?  "SarsView" sounds scary :)

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

 
 // A view that displays 5 stars: empty, full or half full, given a rating in
@@ -269,140 +298,138 @@ void ThrobberNativeTextButton::Run() {
   SchedulePaint();
 }
 
+// SpinnerProgressIndicator ----------------------------------------------------
+class SpinnerProgressIndicator : public views::View {

[Peter Kasting, 2012/10/12 03:23:32]

How does this differ from the similar-looking progress indicator in the download
shelf/downloads tab?  Please let's not define Yet Another Progress Indicator. 
We need them all to be the same.  If this means changing the way that indicator
looks, so be it.

[please use gerrit instead, 2012/10/14 18:40:45]

The downloads tab is Javascript:

this.canvasProgress_.arc(Download.Progress.centerX,
                               Download.Progress.centerY,
                               Download.Progress.radius,
                               Download.Progress.base,
                               Download.Progress.base + Math.PI * 0.02 *
                               Number(this.percent_),
                               false);

The download shelf is a DownloadItemView object that paints the download
progress bar on a small part of itself:

.-------------------------------+-.
| [prog] Filename               |v|
| [ress] Status                 | |
`-------------------------------+-'

Good thing is both DownloadItemView and SpinnerProgressIndicator here use the
same utility function to paint the progress on canvas:
download_util::PaintCustomDownloadProgress().

Perhaps a better solution is to move SpinnerProgressIndicator to
chrome/browser/ui/views/ and make the DownloadItemView use it?

[Peter Kasting, 2012/10/15 18:24:23]

Yeah, having the code here call download_util seems wrong.  Moving things to a
common location makes sense.  That can be done as a separate change if you want.

[please use gerrit instead, 2012/10/15 20:24:04]

Filed for follow up work for myself: http://crbug.com/155913

+ public:
+  SpinnerProgressIndicator();
+  void SetPercentDone(int percent);
+  void SetIndeterminate(bool indetereminate);
+
+  // Overridden from views::View.
+  virtual void Paint(gfx::Canvas* canvas) OVERRIDE;
+  virtual gfx::Size GetPreferredSize() OVERRIDE;
+
+ private:
+  void UpdateTimer();
+  int GetProgressAngle();
+
+  static const int kTimerIntervalMs = 1000 / 30;
+  static const int kSpinRateDegreesPerSecond = 270;
+
+  int percentDone_;

[Peter Kasting, 2012/10/12 03:23:32]

Never use camelCase member variables please

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

+  int isIndeterminate_;
+
+  base::TimeTicks startTime_;
+  base::RepeatingTimer<SpinnerProgressIndicator> timer_;
+};
+
+SpinnerProgressIndicator::SpinnerProgressIndicator()
+  : percentDone_(0),
+    isIndeterminate_(true) {}
+
+void SpinnerProgressIndicator::SetPercentDone(int percent) {
+  percentDone_ = percent;
+  SchedulePaint();
+  UpdateTimer();
+}
+
+void SpinnerProgressIndicator::SetIndeterminate(bool indetereminate) {
+  isIndeterminate_ = indetereminate;
+  SchedulePaint();
+  UpdateTimer();
+}
+
+void SpinnerProgressIndicator::Paint(gfx::Canvas* canvas) {
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  gfx::ImageSkia* fg = rb.GetImageSkiaNamed(IDR_WEB_INTENT_PROGRESS_FOREGROUND);
+  gfx::ImageSkia* bg = rb.GetImageSkiaNamed(IDR_WEB_INTENT_PROGRESS_BACKGROUND);
+  download_util::PaintCustomDownloadProgress(
+      canvas,
+      *bg,
+      *fg,
+      fg->width(),
+      bounds(),
+      GetProgressAngle(),
+      isIndeterminate_ ? -1 : percentDone_);
+}
+
+gfx::Size SpinnerProgressIndicator::GetPreferredSize() {
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  gfx::ImageSkia* fg = rb.GetImageSkiaNamed(IDR_WEB_INTENT_PROGRESS_FOREGROUND);
+  return fg->size();
+}
+
+void SpinnerProgressIndicator::UpdateTimer() {
+  if (!parent() || !isIndeterminate_) {
+    timer_.Stop();
+    return;
+  }
+
+  if (!timer_.IsRunning()) {
+    startTime_ = base::TimeTicks::Now();
+    timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kTimerIntervalMs),
+        this, &SpinnerProgressIndicator::SchedulePaint);
+  }
+}
+
+int SpinnerProgressIndicator::GetProgressAngle() {
+  if (!isIndeterminate_)
+    return download_util::kStartAngleDegrees;
+  base::TimeDelta delta = base::TimeTicks::Now() - startTime_;
+  int angle = delta.InSecondsF() * kSpinRateDegreesPerSecond;
+  return angle % 360;
+}
+
 // WaitingView ----------------------------------------------------------
 class WaitingView : public views::View {
  public:
-  WaitingView(views::ButtonListener* listener, bool use_close_button);
+  WaitingView();
 
  private:
+  static const int kTopBorder = 16;
+  static const int kBottomBorder = 37;
+  static const int kLeftBorder = 0;
+  static const int kRightBorder = 0;
+  static const int kThrobberTextPadding = 15;
+
   DISALLOW_COPY_AND_ASSIGN(WaitingView);
 };
 
-WaitingView::WaitingView(views::ButtonListener* listener,
-                         bool use_close_button) {
+WaitingView::WaitingView() {
+  enum {
+    kContentRowColumnSet, // Column set for the main content.
+  };
+
   views::GridLayout* layout = new views::GridLayout(this);
   layout->set_minimum_size(gfx::Size(WebIntentPicker::kWindowMinWidth, 0));
-  layout->SetInsets(WebIntentPicker::kContentAreaBorder,
-                    WebIntentPicker::kContentAreaBorder,
-                    WebIntentPicker::kContentAreaBorder,
-                    WebIntentPicker::kContentAreaBorder);
+  layout->SetInsets(kTopBorder, kLeftBorder, kBottomBorder, kRightBorder);
   SetLayoutManager(layout);
 
-  views::ColumnSet* cs = layout->AddColumnSet(0);
-  views::ColumnSet* header_cs = NULL;
-  if (use_close_button) {
-    header_cs = layout->AddColumnSet(1);
-    header_cs->AddPaddingColumn(1, views::kUnrelatedControlHorizontalSpacing);
-    header_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
-                         GridLayout::USE_PREF, 0, 0);  // Close Button.
-  }
+  views::ColumnSet* cs = layout->AddColumnSet(kContentRowColumnSet);
   cs->AddPaddingColumn(0, views::kPanelHorizIndentation);
-  cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER,
-                1, GridLayout::USE_PREF, 0, 0);
+  cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 1,
+                GridLayout::USE_PREF, 0, 0);
   cs->AddPaddingColumn(0, views::kPanelHorizIndentation);
 
   // Create throbber.
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  const gfx::ImageSkia* frames =
-      rb.GetImageNamed(IDR_SPEECH_INPUT_SPINNER).ToImageSkia();
-  views::Throbber* throbber = new views::Throbber(kThrobberFrameTimeMs, true);
-  throbber->SetFrames(frames);
-  throbber->Start();
+  SpinnerProgressIndicator* throbber = new SpinnerProgressIndicator();
 
   // Create text.
-  views::Label* label = new views::Label();
+  views::Label* label = CreateLabel();
   label->SetHorizontalAlignment(views::Label::ALIGN_CENTER);
-  label->SetFont(rb.GetFont(ui::ResourceBundle::MediumBoldFont));
   label->SetText(l10n_util::GetStringUTF16(IDS_INTENT_PICKER_WAIT_FOR_CWS));
 
   // Layout the view.
-  if (use_close_button) {
-    layout->StartRow(0, 1);
-    layout->AddView(CreateCloseButton(listener));
-  }
-
-  layout->AddPaddingRow(0, views::kUnrelatedControlLargeVerticalSpacing);
-  layout->StartRow(0, 0);
+  layout->StartRow(0, kContentRowColumnSet);
   layout->AddView(throbber);
-  layout->AddPaddingRow(0, views::kUnrelatedControlLargeVerticalSpacing);
-  layout->StartRow(0, 0);
-  layout->AddView(label);
-  layout->AddPaddingRow(0, views::kUnrelatedControlLargeVerticalSpacing);
-}
-
-// SuggestedExtensionsLayout ---------------------------------------------------
-
-// TODO(groby): Extremely fragile code, relies on order and number of fields.
-// Would probably be better off as GridLayout or similar. Also see review
-// comments on http://codereview.chromium.org/10909183
-
-// A LayoutManager used by a row of the IntentsView. It is similar
-// to a BoxLayout, but it right aligns the rightmost view (which is the install
-// button). It also uses the maximum height of all views in the row as a
-// preferred height so it doesn't change when the install button is hidden.
-class SuggestedExtensionsLayout : public views::LayoutManager {
- public:
-  SuggestedExtensionsLayout();
-  virtual ~SuggestedExtensionsLayout();
 
-  // Implementation of views::LayoutManager.
-  virtual void Layout(views::View* host) OVERRIDE;
-  virtual gfx::Size GetPreferredSize(views::View* host) OVERRIDE;
+  layout->AddPaddingRow(0, kThrobberTextPadding);
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(SuggestedExtensionsLayout);
-};
-
-SuggestedExtensionsLayout::SuggestedExtensionsLayout() {
-}
-
-SuggestedExtensionsLayout::~SuggestedExtensionsLayout() {
-}
-
-void SuggestedExtensionsLayout::Layout(views::View* host) {
-  gfx::Rect child_area(host->GetLocalBounds());
-  child_area.Inset(host->GetInsets());
-  int x = child_area.x();
-  int y = child_area.y();
-
-  for (int i = 0; i < host->child_count(); ++i) {
-    views::View* child = host->child_at(i);
-    if (!child->visible())
-      continue;
-    gfx::Size size(child->GetPreferredSize());
-    gfx::Rect child_bounds(x, y, size.width(), child_area.height());
-    if (i == host->child_count() - 1) {
-      // Last child (the install button) should be right aligned.
-      child_bounds.set_x(std::max(child_area.width() - size.width(), x));
-    } else if (i == 1) {
-      // Label is considered fixed width, to align ratings widget.
-      DCHECK_LE(size.width(), WebIntentPicker::kTitleLinkMaxWidth);
-      x += WebIntentPicker::kTitleLinkMaxWidth +
-          views::kRelatedControlHorizontalSpacing;
-    } else {
-      x += size.width() + views::kRelatedControlHorizontalSpacing;
-    }
-    // Clamp child view bounds to |child_area|.
-    child->SetBoundsRect(child_bounds.Intersect(child_area));
-  }
-}
+  layout->StartRow(0, kContentRowColumnSet);
+  layout->AddView(label);
 
-gfx::Size SuggestedExtensionsLayout::GetPreferredSize(views::View* host) {
-  int width = 0;
-  int height = 0;
-  for (int i = 0; i < host->child_count(); ++i) {
-    views::View* child = host->child_at(i);
-    gfx::Size size(child->GetPreferredSize());
-    // The preferred height includes visible and invisible children. This
-    // prevents jank when a child is hidden.
-    height = std::max(height, size.height());
-    if (!child->visible())
-      continue;
-    if (i != 0)
-      width += views::kRelatedControlHorizontalSpacing;
-  }
-  gfx::Insets insets(host->GetInsets());
-  return gfx::Size(width + insets.width(), height + insets.height());
+  // Start the throbber.
+  throbber->SetIndeterminate(true);
 }
 
 // IntentRowView --------------------------------------------------
@@ -472,6 +499,8 @@ class IntentRowView : public views::View,
   virtual void PaintChildren(gfx::Canvas* canvas) OVERRIDE;
 
  private:
+  static const int kStarRatingHorizontalSpacing = 20;
+
   IntentRowView(ActionType type, size_t tag);
 
   // Gets the proper message string associated with |type_|.
@@ -547,11 +576,14 @@ IntentRowView* IntentRowView::CreateHandlerRow(
     view = new IntentRowView(ACTION_INVOKE, tag);
     icon = service->favicon.ToImageSkia();
     label = new views::Label(elided_title);
+    label->SetEnabledColor(kEnabledLabelColor);
   } else {
     view = new IntentRowView(ACTION_INSTALL, tag);
     view->extension_id_ = extension->id;
     icon = extension->icon.ToImageSkia();
     views::Link* link = new views::Link(elided_title);
+    link->SetUnderlineOnHover(true);
+    link->SetEnabledColor(kEnabledLinkColor);
     link->set_listener(view);
     label = link;
     stars = new StarsView(extension->average_rating);
@@ -559,25 +591,42 @@ IntentRowView* IntentRowView::CreateHandlerRow(
 
   view->delegate_ = delegate;
 
-  view->SetLayoutManager(new SuggestedExtensionsLayout);
+  views::GridLayout* grid_layout = new views::GridLayout(view);
+  view->SetLayoutManager(grid_layout);
 
-  view->icon_ = new views::ImageView();
+  views::ColumnSet* columns = grid_layout->AddColumnSet(0);
+  columns->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 0,
+                     GridLayout::USE_PREF, 0, 0); // Icon.
+  columns->AddPaddingColumn(0, WebIntentPicker::kIconLabelHorizontalSpacing);
+  columns->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 1,
+                     GridLayout::FIXED, WebIntentPicker::kTitleLinkMaxWidth, 0);
+  columns->AddPaddingColumn(0, kStarRatingHorizontalSpacing);
+  if (stars != NULL) {
+    columns->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
+                       GridLayout::USE_PREF, 0, 0); // Star rating.
+    columns->AddPaddingColumn(0, kStarRatingHorizontalSpacing);
+  }
+  columns->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
+                     GridLayout::FIXED, preferred_width, 0); // Button.
 
+  grid_layout->StartRow(0, 0);
+
+  view->icon_ = new views::ImageView();
   view->icon_->SetImage(icon);
-  view->AddChildView(view->icon_);
+  grid_layout->AddView(view->icon_);
 
   view->title_link_ = label;
-  view->AddChildView(view->title_link_);
+  grid_layout->AddView(view->title_link_);
 
   if (stars != NULL) {
     view->stars_ = stars;
-    view->AddChildView(view->stars_);
+    grid_layout->AddView(view->stars_);
   }
 
-  view->install_button_ = new ThrobberNativeTextButton(
-      view, view->GetActionButtonMessage());
+  view->install_button_ = new ThrobberNativeTextButton(view,
+      view->GetActionButtonMessage());
   view->install_button_->set_preferred_width(preferred_width);
-  view->AddChildView(view->install_button_);
+  grid_layout->AddView(view->install_button_);
 
   return view;
 }
@@ -671,6 +720,8 @@ class IntentsView : public views::View {
   virtual void OnEnabledChanged() OVERRIDE;
 
  private:
+  static const int kAppRowVerticalSpacing = 10;

[Peter Kasting, 2012/10/12 03:23:32]

Nit: Just declare this in the function it's used in, avoid members except when
necessary

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

+
   const WebIntentPickerModel* model_;
   IntentRowView::Delegate* delegate_;
 
@@ -703,8 +754,8 @@ void IntentsView::Update() {
   button_width_ = std::max(
       kButtonWidth, size_helper.GetPreferredSize().width());
 
-  views::BoxLayout* layout = new views::BoxLayout(
-      views::BoxLayout::kVertical, 0, 0, views::kRelatedControlVerticalSpacing);
+  views::BoxLayout* layout = new views::BoxLayout(views::BoxLayout::kVertical,
+      0, 0, kAppRowVerticalSpacing);

[Peter Kasting, 2012/10/12 03:23:32]

Nit: Align with first arg in line above or else move that arg onto the beginning
of this line

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

   SetLayoutManager(layout);
 
   int available_rows = kMaxRowCount;
@@ -782,6 +833,7 @@ class WebIntentPickerViews : public views::ButtonListener,
   virtual views::Widget* GetWidget() OVERRIDE;
   virtual const views::Widget* GetWidget() const OVERRIDE;
   virtual views::View* GetContentsView() OVERRIDE;
+  virtual views::View* GetHeaderView() OVERRIDE;

[Peter Kasting, 2012/10/12 03:23:32]

I don't think we should add this concept of a "header view" to dialog delegate. 
We can just lay things out appropriately in the contents, see below.

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

   virtual int GetDialogButtons() const OVERRIDE;
   virtual bool Cancel() OVERRIDE;
 
@@ -821,6 +873,10 @@ class WebIntentPickerViews : public views::ButtonListener,
   // Update picker contents to reflect the current state of the model.
   void UpdateContents();
 
+  // Shows a spinner and notifies the user that we are waiting for information
+  // from the Chrome Web Store.
+  void ShowWaitingForSuggestions();
+
   // Updates the dialog with the list of available services, suggestions,
   // and a nice link to CWS to find more suggestions. This is the "Main"
   // view of the picker.
@@ -854,6 +910,9 @@ class WebIntentPickerViews : public views::ButtonListener,
   // Created locally, owned by Views.
   views::Label* action_label_;
 
+  // Label for the no-services dialog.
+  views::Label* no_services_title_;
+
   // A weak pointer to the header label for the extension suggestions.
   // Created locally, owned by Views.
   views::Label* suggestions_label_;
@@ -876,6 +935,9 @@ class WebIntentPickerViews : public views::ButtonListener,
   // Created locally, owned by Views.
   views::View* contents_;
 
+  // The view that contains the header for the picker.
+  views::View* header_;
+
   // A weak pointer to the constrained window.
   // Created locally, owned by Views.
   ConstrainedWindowViews* window_;
@@ -888,6 +950,12 @@ class WebIntentPickerViews : public views::ButtonListener,
   // Created locally, owned by Views.
   views::Link* choose_another_service_link_;
 
+  // The icon for the inline intent.
+  views::ImageView* inline_intent_icon_;
+
+  // The label for the inline intent.
+  views::Label* inline_intent_label_;
+
   // Weak pointer to "Waiting for CWS" display. Owned by parent view.
   WaitingView* waiting_view_;
 
@@ -901,11 +969,6 @@ class WebIntentPickerViews : public views::ButtonListener,
   // Ownership of the WebContents we are displaying in the inline disposition.
   scoped_ptr<WebContents> inline_web_contents_;
 
-  // Indicate if dialog should display its own close button.
-  // TODO(groby): Only relevant until new ConstrainedWindow is implemented,
-  // from then on always true.
-  bool use_close_button_;
-
   // Signals if the picker can be closed. False during extension install.
   bool can_close_;
 
@@ -925,7 +988,6 @@ WebIntentPickerViews::WebIntentPickerViews(TabContents* tab_contents,
                                            WebIntentPickerModel* model)
     : delegate_(delegate),
       model_(model),
-      action_label_(NULL),
       suggestions_label_(NULL),
       extensions_(NULL),
       tab_contents_(tab_contents),
@@ -936,17 +998,24 @@ WebIntentPickerViews::WebIntentPickerViews(TabContents* tab_contents,
       waiting_view_(NULL),
       displaying_web_contents_(false),
       can_close_(true) {
-  bool enable_chrome_style = CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnableFramelessConstrainedDialogs);
-  use_close_button_ = enable_chrome_style;
-
   model_->set_observer(this);
   contents_ = new views::View();
+  header_ = new views::View();
+
+  action_label_ = CreateTitleLabel();
+  action_label_->SetText(GetActionTitle());
+  no_services_title_ = CreateTitleLabel();
+  inline_intent_icon_ = new views::ImageView();
+  inline_intent_label_ = CreateLabel();
+  choose_another_service_link_ = CreateLink();
+
+  gfx::Insets client_insets(kConstrainedWindowClientTopInset,
+                            kConstrainedWindowClientLeftInset,
+                            kConstrainedWindowClientBottomInset,
+                            kConstrainedWindowClientRightInset);
   // Show the dialog.
-  window_ = new ConstrainedWindowViews(tab_contents->web_contents(),
-                                       this,
-                                       enable_chrome_style);
-
+  window_ = new ConstrainedWindowViews(tab_contents->web_contents(), this, true,
+                                       client_insets);
   UpdateContents();
 }
 
@@ -979,6 +1048,10 @@ views::View* WebIntentPickerViews::GetContentsView() {
   return contents_;
 }
 
+views::View* WebIntentPickerViews::GetHeaderView() {
+  return header_;
+}
+
 int WebIntentPickerViews::GetDialogButtons() const {
   return ui::DIALOG_BUTTON_NONE;
 }
@@ -1007,10 +1080,10 @@ void WebIntentPickerViews::Close() {
 void WebIntentPickerViews::SetActionString(const string16& action) {
   action_text_ = action;
 
-  if (action_label_)
-    action_label_->SetText(GetActionTitle());
-    contents_->Layout();
-    SizeToContents();
+  action_label_->SetText(GetActionTitle());
+
+  contents_->Layout();
+  SizeToContents();
 }
 
 void WebIntentPickerViews::OnExtensionInstallSuccess(const std::string& id) {
@@ -1039,102 +1112,144 @@ void WebIntentPickerViews::OnPendingAsyncCompleted() {
 }
 
 void WebIntentPickerViews::ShowNoServicesMessage() {
-  ClearContents();
+  static const int kTopPadding = 20;

[Peter Kasting, 2012/10/12 03:23:32]

Nit: I'd just have:

  const int kPadding = 20;
  const int kHeaderTopPadding = 11;

...and drop the other constants.

Also, regarding constants in general.  It's really odd to have 1, 7, 11, 13,
etc. all over.  It's normally OK to adjust padding values given in the mocks by
a few pixels; more important is to use a small number of consistent values. 
Look at the layout constants header file and wherever possible pull from there,
otherwise try and choose a small number of constants that are used across
multiple dialogs.  If the constants differ because we're aligning different-size
objects to have e.g. a common baseline or something, calculate the precise
constants needed on the fly based on the font size or image size or whatever. 
This will make the dialog layout robust if we start changing the assets instead
of incredibly brittle and needing tons of small tweaks all the time, which sucks
since we _will_ change these dialogs' appearance over time.

[please use gerrit instead, 2012/10/14 18:40:45]

Partially done. (1) I am now using as many global constants from
layout_constants.h, web_intent_picker.h, and constrained_window_constants.h as
possible. (2) I've moved as many as possible of the remaining layout constants
into the unnamed namespace at the top of web_intent_picker_views.cc to be
defined in one place and shared among methods in this file.

UX wants some very specific paddings for Web Intents views. I don't think we can
convince them otherwise...

[Peter Kasting, 2012/10/15 18:24:23]

Normally weird paddings happen because we want some particular consistent visual
appearance and then we need to adjust different elements by different amounts in
order to achieve it.  Like if we want to align two objects but one has some
built-in padding, we need different external padding for them.

In cases like this, even if we need two padding values, we can do something like
"desired indent = x, built in padding of object 2 = y" and then pad by x and (x
- y) instead of just having two different padding values.  That's easier to
understand and maintain later because for example if we want to change the
overall padding we can just change x and not y.  Added bonus if after this sort
of transformation x becomes one of our common layout constants and then we just
need to specially define y.

The point of all that was, even if we do need some custom values here, just make
sure they're as few in number and as maintainable as possible.

[please use gerrit instead, 2012/10/15 20:24:04]

Done. I am now using mostly paddings from ConstarinedWindowConstants with
offsets for builtin paddings of icons and labels.

+  static const int kLeftPadding = 20;
+  static const int kBottomPadding = 20;
+  static const int kRightPadding = 20;
 
-  views::GridLayout* grid_layout = new views::GridLayout(contents_);
-  contents_->SetLayoutManager(grid_layout);
-
-  grid_layout->SetInsets(kContentAreaBorder, kContentAreaBorder,
-                         kContentAreaBorder, kContentAreaBorder);
-  views::ColumnSet* main_cs = grid_layout->AddColumnSet(0);
-  main_cs->AddColumn(GridLayout::FILL, GridLayout::LEADING, 1,
-                     GridLayout::USE_PREF, 0, 0);
+  static const int kHeaderTopPadding = 11;
+  static const int kHeaderLeftPadding = 0;
+  static const int kHeaderBottomPadding = 0;
+  static const int kHeaderRightPadding = 0;
 
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  ClearContents();
 
-  grid_layout->StartRow(0, 0);
-  views::Label* header = new views::Label();
-  header->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-  header->SetFont(rb.GetFont(ui::ResourceBundle::MediumFont));
-  header->SetText(l10n_util::GetStringUTF16(
+  // Header.
+  no_services_title_->SetText(l10n_util::GetStringUTF16(
       IDS_INTENT_PICKER_NO_SERVICES_TITLE));
-  grid_layout->AddView(header);
+  views::GridLayout* header_layout = new views::GridLayout(header_);
+  header_->SetLayoutManager(header_layout);
+  header_layout->SetInsets(kHeaderTopPadding, kHeaderLeftPadding,
+      kHeaderBottomPadding, kHeaderRightPadding);
+  header_layout->AddColumnSet(0)->AddColumn(GridLayout::LEADING,
+      GridLayout::CENTER, 1, GridLayout::USE_PREF, 0, 0);
+  header_layout->StartRow(0, 0);
+  header_layout->AddView(no_services_title_);
+
+  // Contents.
+  views::GridLayout* layout = new views::GridLayout(contents_);

[Peter Kasting, 2012/10/12 03:23:32]

Let's kill the header view entirely and avoid having multiple grid layouts. 
That makes it harder to refactor to use DialogDelegateView later anyway. 
Instead of using two sets of insets we can use multiple column sets, some of
which contain padding columns/rows.

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

+  layout->set_minimum_size(gfx::Size(WebIntentPicker::kWindowMinWidth, 0));
+  layout->SetInsets(kTopPadding, kLeftPadding, kBottomPadding, kRightPadding);
+  contents_->SetLayoutManager(layout);
 
-  grid_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  layout->AddColumnSet(0)->AddColumn(GridLayout::FILL, GridLayout::CENTER, 1,
+      GridLayout::USE_PREF, 0, 0);
 
-  grid_layout->StartRow(0, 0);
-  views::Label* body = new views::Label();
+  // Content row.
+  layout->StartRow(0, 0);
+  views::Label* body = CreateLabel();
   body->SetMultiLine(true);
-  body->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   body->SetText(l10n_util::GetStringUTF16(IDS_INTENT_PICKER_NO_SERVICES));
-  grid_layout->AddView(body);
+  layout->AddView(body);
 
-  int height = contents_->GetHeightForWidth(kWindowMinWidth);
-  contents_->SetSize(gfx::Size(kWindowMinWidth, height));
+  int height = contents_->GetHeightForWidth(WebIntentPicker::kWindowMinWidth);
+  contents_->SetSize(gfx::Size(WebIntentPicker::kWindowMinWidth, height));
+  contents_->Layout();
 }
 
 void WebIntentPickerViews::OnInlineDispositionWebContentsLoaded(
     content::WebContents* web_contents) {
+  enum {
+    kSeparatorRowColumnSet, // Column set for separator layout.
+    kWebContentsRowColumnSet, // Column set for main content layout.
+  };
+
   if (displaying_web_contents_)
     return;
 
-  // Replace the picker with the inline disposition.
-  ClearContents();
+  const int kHeaderTopPadding = 7;
+  const int kHeaderLeftPadding = 0;
+  const int kHeaderBottomPadding = 0;
+  const int kHeaderRightPadding = 1;
 
-  views::GridLayout* grid_layout = new views::GridLayout(contents_);
-  contents_->SetLayoutManager(grid_layout);
+  const int kTopPadding = 13;
+  const int kLeftPadding = 0;
+  const int kBottomPadding = 20;
+  const int kRightPadding = 0;
 
-  grid_layout->SetInsets(kContentAreaBorder, kContentAreaBorder,
-                         kContentAreaBorder, kContentAreaBorder);
-  views::ColumnSet* header_cs = grid_layout->AddColumnSet(0);
-  header_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
-                       GridLayout::USE_PREF, 0, 0);  // Icon.
-  header_cs->AddPaddingColumn(0, 4);
-  header_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
-                       GridLayout::USE_PREF, 0, 0);  // Title.
-  header_cs->AddPaddingColumn(0, 4);
-  header_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
-                       GridLayout::USE_PREF, 0, 0);  // Link.
-  header_cs->AddPaddingColumn(1, views::kUnrelatedControlHorizontalSpacing);
-  if (use_close_button_) {
-    header_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
-                         GridLayout::USE_PREF, 0, 0);  // Close Button.
-  }
+  const int kWebContentsTopPadding = 3;
+  const int kWebContentsLeftPadding = 1;
+  const int kWebContentsRightPadding = 1;
 
-  views::ColumnSet* full_cs = grid_layout->AddColumnSet(1);
-  full_cs->AddColumn(GridLayout::FILL, GridLayout::FILL, 1.0,
-                     GridLayout::USE_PREF, 0, 0);
+  bool show_use_another = (model_->GetInstalledServiceCount() > 1 ||
+                           model_->GetSuggestedExtensionCount());

[Peter Kasting, 2012/10/12 03:23:32]

Nit: Strange indenting, only indent even for comma-separated arg lists, indent 4
otherwise

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

+
+  // Replace the picker with the inline disposition.
+  ClearContents();
 
   const WebIntentPickerModel::InstalledService* service =
       model_->GetInstalledServiceWithURL(model_->inline_disposition_url());
 
-  // Header row.
-  grid_layout->StartRow(0, 0);
-  views::ImageView* icon = new views::ImageView();
-  icon->SetImage(service->favicon.ToImageSkia());
-  grid_layout->AddView(icon);
+  // Header.
+  views::GridLayout* header_layout = new views::GridLayout(header_);
+  header_layout->SetInsets(kHeaderTopPadding, kHeaderLeftPadding,
+                           kHeaderBottomPadding, kHeaderRightPadding);
+  header_->SetLayoutManager(header_layout);
+  views::ColumnSet* header_view_cs = header_layout->AddColumnSet(0);
+  header_view_cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 0,
+                       GridLayout::USE_PREF, 0, 0);  // Icon.
+  header_view_cs->AddPaddingColumn(0, kIconLabelHorizontalSpacing);
+  header_view_cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 1,
+                            GridLayout::USE_PREF, 0, 0);  // Title.
+  if (show_use_another) {
+    header_view_cs->AddPaddingColumn(0,
+        views::kUnrelatedControlHorizontalSpacing);
+    header_view_cs->AddColumn(GridLayout::TRAILING, GridLayout::CENTER, 0,
+                              GridLayout::USE_PREF, 0, 0);  // Use another app.
+  }
 
-  string16 elided_title = ui::ElideText(
-      service->title, gfx::Font(), kTitleLinkMaxWidth, ui::ELIDE_AT_END);
-  views::Label* title = new views::Label(elided_title);
-  grid_layout->AddView(title);
+  header_layout->StartRow(0, 0);
+  inline_intent_icon_->SetImage(service->favicon.ToImageSkia());
+  header_layout->AddView(inline_intent_icon_);
+  inline_intent_label_->SetText(ui::ElideText(service->title,
+      inline_intent_label_->font(), kTitleLinkMaxWidth, ui::ELIDE_AT_END));
+  header_layout->AddView(inline_intent_label_);
   // Add link for "choose another service" if other suggestions are available
   // or if more than one (the current) service is installed.
-  if (model_->GetInstalledServiceCount() > 1 ||
-       model_->GetSuggestedExtensionCount()) {
-    choose_another_service_link_ = new views::Link(
+  if (show_use_another) {
+    choose_another_service_link_->SetText(
         l10n_util::GetStringUTF16(IDS_INTENT_PICKER_USE_ALTERNATE_SERVICE));
-    grid_layout->AddView(choose_another_service_link_);
     choose_another_service_link_->set_listener(this);
+    header_layout->AddView(choose_another_service_link_);
   }
 
-  if (use_close_button_)
-    grid_layout->AddView(CreateCloseButton(this));
+  // Content.
+  views::GridLayout* grid_layout = new views::GridLayout(contents_);
+  grid_layout->SetInsets(kTopPadding, kLeftPadding, kBottomPadding,
+                         kRightPadding);
+  contents_->SetLayoutManager(grid_layout);
+
+  grid_layout->AddColumnSet(kSeparatorRowColumnSet)->AddColumn(
+      GridLayout::FILL, GridLayout::CENTER, 1, GridLayout::USE_PREF, 0, 0);
+
+  views::ColumnSet* full_cs =
+      grid_layout->AddColumnSet(kWebContentsRowColumnSet);
+  full_cs->AddPaddingColumn(0, kWebContentsLeftPadding);
+  full_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 1,
+                     GridLayout::USE_PREF, 0, 0);
+  full_cs->AddPaddingColumn(0, kWebContentsRightPadding);
+
+  // Separator row.
+  grid_layout->StartRow(0, kSeparatorRowColumnSet);
+  grid_layout->AddView(new views::Separator());
+
+  // Padding row.
+  grid_layout->AddPaddingRow(0, kWebContentsTopPadding);
 
   // Inline web contents row.
-  grid_layout->StartRow(0, 1);
-  grid_layout->AddView(webview_, 1, 1, GridLayout::CENTER,
-                       GridLayout::CENTER, 0, 0);
+  grid_layout->StartRow(0, kWebContentsRowColumnSet);
+  grid_layout->AddView(webview_);
+
   contents_->Layout();
   SizeToContents();
   displaying_web_contents_ = true;
@@ -1144,10 +1259,10 @@ void WebIntentPickerViews::OnModelChanged(WebIntentPickerModel* model) {
   if (waiting_view_ && !model->IsWaitingForSuggestions())
     UpdateContents();
 
+  string16 suggestions_label_text = model->GetSuggestionsLinkText();
   if (suggestions_label_) {
-    string16 label_text = model->GetSuggestionsLinkText();
-    suggestions_label_->SetText(label_text);
-    suggestions_label_->SetVisible(!label_text.empty());
+    suggestions_label_->SetText(suggestions_label_text);
+    suggestions_label_->SetVisible(!suggestions_label_text.empty());

[Peter Kasting, 2012/10/12 03:23:32]

Nit: It's really weird to have a view that is both optionally NULL _and_
optionally visible.  I suggest eliminating one of those two optionals.

[please use gerrit instead, 2012/10/14 18:40:45]

Done.

   }
 
   if (extensions_)
@@ -1227,13 +1342,7 @@ void WebIntentPickerViews::OnActionButtonClicked(
 
 void WebIntentPickerViews::UpdateContents() {
   if (model_ && model_->IsWaitingForSuggestions()) {
-    ClearContents();
-    contents_->SetLayoutManager(new views::FillLayout());
-    waiting_view_ = new WaitingView(this, use_close_button_);
-    contents_->AddChildView(waiting_view_);
-    int height = contents_->GetHeightForWidth(kWindowMinWidth);
-    contents_->SetSize(gfx::Size(kWindowMinWidth, height));
-    contents_->Layout();
+    ShowWaitingForSuggestions();
   } else if (model_->GetInstalledServiceCount() ||
       model_->GetSuggestedExtensionCount()) {
     ShowAvailableServices();
@@ -1243,99 +1352,87 @@ void WebIntentPickerViews::UpdateContents() {
   SizeToContents();
 }
 
+void WebIntentPickerViews::ShowWaitingForSuggestions() {
+  ClearContents();
+  contents_->SetLayoutManager(new views::FillLayout());
+  waiting_view_ = new WaitingView();
+  contents_->AddChildView(waiting_view_);
+  int height = contents_->GetHeightForWidth(kWindowMinWidth);
+  contents_->SetSize(gfx::Size(kWindowMinWidth, height));
+  contents_->Layout();
+}
+
 const string16 WebIntentPickerViews::GetActionTitle() {
-  return (!action_text_.empty()) ?
-      action_text_ :
+  return (!action_text_.empty()) ? action_text_ :
       l10n_util::GetStringUTF16(IDS_INTENT_PICKER_CHOOSE_SERVICE);
 }
 
 void WebIntentPickerViews::ShowAvailableServices() {
-  enum {
-    kHeaderRowColumnSet,  // Column set for header layout.
-    kFullWidthColumnSet,  // Column set with a single full-width column.
-    kIndentedFullWidthColumnSet,  // Single full-width column, indented.
-  };
+  const int kHeaderTopBorder = 8;
+  const int kHeaderLeftBorder = 0;
+  const int kHeaderBottomBorder = 0;
+  const int kHeaderRightBorder = 0;
+
+  const int kTopPadding = 13;
+  const int kLeftPadding = 20;
+  const int kBottomPadding = 16;
+  const int kRightPadding = 20;
+
+  const int kAppsSuggestionsSpacing = 14;
 
   ClearContents();
   displaying_web_contents_ = false;
-
   extensions_ = new IntentsView(model_, this);
 
-  views::GridLayout* grid_layout = new views::GridLayout(contents_);
-  contents_->SetLayoutManager(grid_layout);
+  // Header view.
+  action_label_->SetText(GetActionTitle());
+
+  views::GridLayout* header_layout = new views::GridLayout(header_);
+  header_->SetLayoutManager(header_layout);
+  header_layout->SetInsets(kHeaderTopBorder, kHeaderLeftBorder,
+                           kHeaderBottomBorder, kHeaderRightBorder);
+  header_layout->AddColumnSet(0)->AddColumn(GridLayout::LEADING,
+      GridLayout::CENTER, 1, GridLayout::USE_PREF, 0, 0); // Action label
+  header_layout->StartRow(0, 0);
+  header_layout->AddView(action_label_);
 
+  // Content view.
+  views::GridLayout* grid_layout = new views::GridLayout(contents_);
   grid_layout->set_minimum_size(
       gfx::Size(extensions_->AdjustWidth(kWindowMinWidth), 0));
-  grid_layout->SetInsets(kContentAreaBorder, kContentAreaBorder,
-                         kContentAreaBorder, kContentAreaBorder);
-  views::ColumnSet* header_cs = grid_layout->AddColumnSet(kHeaderRowColumnSet);
-  header_cs->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 1,
-                       GridLayout::USE_PREF, 0, 0);  // Title.
-  if (use_close_button_) {
-    header_cs->AddPaddingColumn(0, views::kUnrelatedControlHorizontalSpacing);
-    header_cs->AddColumn(GridLayout::CENTER, GridLayout::CENTER, 0,
-                         GridLayout::USE_PREF, 0, 0);  // Close Button.
-  }
-
-  views::ColumnSet* full_cs = grid_layout->AddColumnSet(kFullWidthColumnSet);
-  full_cs->AddColumn(GridLayout::FILL, GridLayout::CENTER, 1,
-                     GridLayout::USE_PREF, 0, 0);
-
-  views::ColumnSet* indent_cs =
-      grid_layout->AddColumnSet(kIndentedFullWidthColumnSet);
-  indent_cs->AddPaddingColumn(0, views::kUnrelatedControlHorizontalSpacing);
-  indent_cs->AddColumn(GridLayout::FILL, GridLayout::CENTER, 1,
-                       GridLayout::USE_PREF, 0, 0);
-
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-
-  // Header row.
-  grid_layout->StartRow(0, kHeaderRowColumnSet);
-  action_label_ = new views::Label(GetActionTitle());
-  action_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-  action_label_->SetFont(rb.GetFont(ui::ResourceBundle::MediumFont));
-  grid_layout->AddView(action_label_);
-
-  if (use_close_button_)
-    grid_layout->AddView(CreateCloseButton(this));
-
-  // Padding row.
-  grid_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  grid_layout->SetInsets(kTopPadding, kLeftPadding, kBottomPadding,
+                         kRightPadding);
+  contents_->SetLayoutManager(grid_layout);
 
-  // Row with app suggestions label.
-  grid_layout->StartRow(0, kIndentedFullWidthColumnSet);
-  suggestions_label_ = new views::Label();
-  suggestions_label_->SetVisible(false);
-  suggestions_label_->SetMultiLine(true);
-  suggestions_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-  grid_layout->AddView(suggestions_label_);
-
-  // Padding row.
-  grid_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  grid_layout->AddColumnSet(0)->AddColumn(GridLayout::FILL, GridLayout::CENTER,
+      1, GridLayout::USE_PREF, 0, 0); // Content.
 
   // Row with extension suggestions.
-  grid_layout->StartRow(0, kIndentedFullWidthColumnSet);
+  grid_layout->StartRow(0, 0);
   grid_layout->AddView(extensions_);
 
   // Padding row.
-  grid_layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
+  grid_layout->AddPaddingRow(0, kAppsSuggestionsSpacing);
+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
 
   // Row with "more suggestions" link.
-  grid_layout->StartRow(0, kFullWidthColumnSet);
+  grid_layout->StartRow(0, 0);
   views::View* more_view = new views::View();
   more_view->SetLayoutManager(
       new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0,
-                           views::kRelatedControlHorizontalSpacing));
+                           kIconLabelHorizontalSpacing));
   views::ImageView* icon = new views::ImageView();
   icon->SetImage(rb.GetImageNamed(IDR_WEBSTORE_ICON_16).ToImageSkia());
   more_view->AddChildView(icon);
-  more_suggestions_link_ = new views::Link(
-    l10n_util::GetStringUTF16(IDS_FIND_MORE_INTENT_HANDLER_MESSAGE));
-  more_suggestions_link_->SetDisabledColor(kDisabledLinkColor);
+  more_suggestions_link_ = CreateLink();
+  more_suggestions_link_->SetText(
+      l10n_util::GetStringUTF16(IDS_FIND_MORE_INTENT_HANDLER_MESSAGE));
   more_suggestions_link_->set_listener(this);
   more_view->AddChildView(more_suggestions_link_);
   grid_layout->AddView(more_view, 1, 1,
                        GridLayout::LEADING, GridLayout::CENTER);
+
   contents_->Layout();
 }
 
@@ -1365,14 +1462,14 @@ void WebIntentPickerViews::SizeToContents() {
 
 void WebIntentPickerViews::ClearContents() {
   DCHECK(contents_);
+  DCHECK(header_);
   // The call RemoveAllChildViews(true) deletes all children of |contents|. If
   // we do not set our weak pointers to NULL, then they will continue to point
   // to where the deleted objects used to be, i.e. unitialized memory. This
   // would cause hard-to-explain crashes.
   contents_->RemoveAllChildViews(true);
-  action_label_ = NULL;
+  header_->RemoveAllChildViews(false);
   suggestions_label_ = NULL;
   extensions_ = NULL;
   more_suggestions_link_ = NULL;
-  choose_another_service_link_ = NULL;
 }