Construct infobar child views in constructor.

This lets us avoid some ViewHierarchyChanged() overrides.

BUG=none
TEST=none

Change-Id: I7896460f65cedf2351f5acd3e16893674faa72ea
Reviewed-on: https://chromium-review.googlesource.com/996908
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548846}

chrome/browser/ui/views/infobars/alternate_nav_infobar_view.cc

@@ -36,7 +36,23 @@ std::unique_ptr<infobars::InfoBar> AlternateNavInfoBarDelegate::CreateInfoBar(
 
 AlternateNavInfoBarView::AlternateNavInfoBarView(
     std::unique_ptr<AlternateNavInfoBarDelegate> delegate)
-    : InfoBarView(std::move(delegate)) {}
+    : InfoBarView(std::move(delegate)) {
+  auto* delegate_ptr = GetDelegate();
+  size_t offset;
+  base::string16 message_text = delegate_ptr->GetMessageTextWithOffset(&offset);
+  DCHECK_NE(base::string16::npos, offset);
+  label_1_text_ = message_text.substr(0, offset);
+  label_1_ = CreateLabel(label_1_text_);
+  AddChildView(label_1_);
+
+  link_text_ = delegate_ptr->GetLinkText();
+  link_ = CreateLink(link_text_, this);
+  AddChildView(link_);
+
+  label_2_text_ = message_text.substr(offset);
+  label_2_ = CreateLabel(label_2_text_);
+  AddChildView(label_2_);
+}
 
 AlternateNavInfoBarView::~AlternateNavInfoBarView() {
 }
@@ -77,31 +93,6 @@ void AlternateNavInfoBarView::Layout() {
   label_2_->SetPosition(gfx::Point(link_->bounds().right(), OffsetY(label_2_)));
 }
 
-void AlternateNavInfoBarView::ViewHierarchyChanged(
-    const ViewHierarchyChangedDetails& details) {
-  if (details.is_add && (details.child == this) && (label_1_ == NULL)) {
-    AlternateNavInfoBarDelegate* delegate = GetDelegate();
-    size_t offset;
-    base::string16 message_text = delegate->GetMessageTextWithOffset(&offset);
-    DCHECK_NE(base::string16::npos, offset);
-    label_1_text_ = message_text.substr(0, offset);
-    label_1_ = CreateLabel(label_1_text_);
-    AddChildView(label_1_);
-
-    link_text_ = delegate->GetLinkText();
-    link_ = CreateLink(link_text_, this);
-    AddChildView(link_);
-
-    label_2_text_ = message_text.substr(offset);
-    label_2_ = CreateLabel(label_2_text_);
-    AddChildView(label_2_);
-  }
-
-  // This must happen after adding all other children so InfoBarView can ensure
-  // the close button is the last child.
-  InfoBarView::ViewHierarchyChanged(details);
-}
-
 int AlternateNavInfoBarView::ContentMinimumWidth() const {
   int label_1_width = label_1_->GetMinimumSize().width();
   return label_1_width ? label_1_width : link_->GetMinimumSize().width();

chrome/browser/ui/views/infobars/alternate_nav_infobar_view.h

@@ -33,8 +33,6 @@ class AlternateNavInfoBarView : public InfoBarView,
 
   // InfoBarView:
   void Layout() override;
-  void ViewHierarchyChanged(
-      const ViewHierarchyChangedDetails& details) override;
   int ContentMinimumWidth() const override;
 
   // views::LinkListener:

chrome/browser/ui/views/infobars/confirm_infobar.cc

@@ -36,7 +36,31 @@ std::unique_ptr<infobars::InfoBar> InfoBarService::CreateConfirmInfoBar(
 // ConfirmInfoBar -------------------------------------------------------------
 
 ConfirmInfoBar::ConfirmInfoBar(std::unique_ptr<ConfirmInfoBarDelegate> delegate)
-    : InfoBarView(std::move(delegate)) {}
+    : InfoBarView(std::move(delegate)) {
+  auto* delegate_ptr = GetDelegate();
+  label_ = CreateLabel(delegate_ptr->GetMessageText());
+  AddChildView(label_);
+
+  const auto buttons = delegate_ptr->GetButtons();
+  if (buttons & ConfirmInfoBarDelegate::BUTTON_OK) {
+    ok_button_ = CreateButton(ConfirmInfoBarDelegate::BUTTON_OK);
+    ok_button_->SetProminent(true);
+    if (delegate_ptr->OKButtonTriggersUACPrompt()) {
+      elevation_icon_setter_.reset(new ElevationIconSetter(
+          ok_button_,
+          base::BindOnce(&ConfirmInfoBar::Layout, base::Unretained(this))));
+    }
+  }
+
+  if (buttons & ConfirmInfoBarDelegate::BUTTON_CANCEL) {
+    cancel_button_ = CreateButton(ConfirmInfoBarDelegate::BUTTON_CANCEL);
+    if (buttons == ConfirmInfoBarDelegate::BUTTON_CANCEL)
+      cancel_button_->SetProminent(true);
+  }
+
+  link_ = CreateLink(delegate_ptr->GetLinkText(), this);
+  AddChildView(link_);
+}
 
 ConfirmInfoBar::~ConfirmInfoBar() {
   // Ensure |elevation_icon_setter_| is destroyed before |ok_button_|.
@@ -73,39 +97,6 @@ void ConfirmInfoBar::Layout() {
   link_->SetPosition(gfx::Point(EndX() - link_->width(), OffsetY(link_)));
 }
 
-void ConfirmInfoBar::ViewHierarchyChanged(
-    const ViewHierarchyChangedDetails& details) {
-  if (details.is_add && details.child == this && (label_ == nullptr)) {
-    ConfirmInfoBarDelegate* delegate = GetDelegate();
-    label_ = CreateLabel(delegate->GetMessageText());
-    AddChildView(label_);
-
-    if (delegate->GetButtons() & ConfirmInfoBarDelegate::BUTTON_OK) {
-      ok_button_ = CreateButton(ConfirmInfoBarDelegate::BUTTON_OK);
-      ok_button_->SetProminent(true);
-      if (delegate->OKButtonTriggersUACPrompt()) {
-        elevation_icon_setter_.reset(new ElevationIconSetter(
-            ok_button_,
-            base::BindOnce(&ConfirmInfoBar::Layout, base::Unretained(this))));
-      }
-    }
-
-    if (delegate->GetButtons() & ConfirmInfoBarDelegate::BUTTON_CANCEL) {
-      cancel_button_ = CreateButton(ConfirmInfoBarDelegate::BUTTON_CANCEL);
-      if (delegate->GetButtons() == ConfirmInfoBarDelegate::BUTTON_CANCEL)
-        cancel_button_->SetProminent(true);
-    }
-
-    base::string16 link_text(delegate->GetLinkText());
-    link_ = CreateLink(link_text, this);
-    AddChildView(link_);
-  }
-
-  // This must happen after adding all other children so InfoBarView can ensure
-  // the close button is the last child.
-  InfoBarView::ViewHierarchyChanged(details);
-}
-
 void ConfirmInfoBar::ButtonPressed(views::Button* sender,
                                    const ui::Event& event) {
   if (!owner())

chrome/browser/ui/views/infobars/confirm_infobar.h

@@ -31,8 +31,6 @@ class ConfirmInfoBar : public InfoBarView,
  private:
   // InfoBarView:
   void Layout() override;
-  void ViewHierarchyChanged(
-      const ViewHierarchyChangedDetails& details) override;
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   int ContentMinimumWidth() const override;
 

chrome/browser/ui/views/infobars/infobar_view.cc

@@ -97,6 +97,33 @@ InfoBarView::InfoBarView(std::unique_ptr<infobars::InfoBarDelegate> delegate)
   // animation.
   SetPaintToLayer();
   layer()->SetMasksToBounds(true);
+
+  gfx::Image image = this->delegate()->GetIcon();
+  if (!image.IsEmpty()) {
+    icon_ = new views::ImageView;
+    icon_->SetImage(image.ToImageSkia());
+    icon_->SizeToPreferredSize();
+    icon_->SetProperty(
+        views::kMarginsKey,
+        new gfx::Insets(ChromeLayoutProvider::Get()->GetDistanceMetric(
+                            DISTANCE_TOAST_LABEL_VERTICAL),
+                        0));
+    AddChildView(icon_);
+  }
+
+  close_button_ = views::CreateVectorImageButton(this);
+  // This is the wrong color, but allows the button's size to be computed
+  // correctly.  We'll reset this with the correct color in OnThemeChanged().
+  views::SetImageFromVectorIcon(close_button_, vector_icons::kClose16Icon,
+                                gfx::kPlaceholderColor);
+  close_button_->SetAccessibleName(
+      l10n_util::GetStringUTF16(IDS_ACCNAME_CLOSE));
+  close_button_->SetFocusForPlatform();
+  gfx::Insets close_button_spacing = GetCloseButtonSpacing();
+  close_button_->SetProperty(views::kMarginsKey,
+                             new gfx::Insets(close_button_spacing.top(), 0,
+                                             close_button_spacing.bottom(), 0));
+  AddChildView(close_button_);
 }
 
 InfoBarView::~InfoBarView() {
@@ -157,44 +184,20 @@ void InfoBarView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   View::ViewHierarchyChanged(details);
 
-  if (details.is_add && (details.child == this) && (close_button_ == NULL)) {
-    gfx::Image image = delegate()->GetIcon();
-    if (!image.IsEmpty()) {
-      icon_ = new views::ImageView;
-      icon_->SetImage(image.ToImageSkia());
-      icon_->SizeToPreferredSize();
-      icon_->SetProperty(
-          views::kMarginsKey,
-          new gfx::Insets(ChromeLayoutProvider::Get()->GetDistanceMetric(
-                              DISTANCE_TOAST_LABEL_VERTICAL),
-                          0));
-      AddChildView(icon_);
+  // Anything that needs to happen once after all subclasses add their children.
+  if (details.is_add && (details.child == this)) {
+    ReorderChildView(close_button_, -1);
+
+    // Ensure the infobar is tall enough to display its contents.
+    int height = 0;
+    for (int i = 0; i < child_count(); ++i) {
+      View* child = child_at(i);
+      const gfx::Insets* const margins = child->GetProperty(views::kMarginsKey);
+      const int margin_height = margins ? margins->height() : 0;
+      height = std::max(height, child->height() + margin_height);
     }
-
-    close_button_ = views::CreateVectorImageButton(this);
-    views::SetImageFromVectorIcon(close_button_, vector_icons::kClose16Icon,
-                                  GetColor(kTextColor));
-    close_button_->SetAccessibleName(
-        l10n_util::GetStringUTF16(IDS_ACCNAME_CLOSE));
-    close_button_->SetFocusForPlatform();
-    gfx::Insets close_button_spacing = GetCloseButtonSpacing();
-    close_button_->SetProperty(
-        views::kMarginsKey, new gfx::Insets(close_button_spacing.top(), 0,
-                                            close_button_spacing.bottom(), 0));
-    // Subclasses should already be done adding child views by this point (see
-    // related DCHECK in Layout()).
-    AddChildView(close_button_);
+    SetTargetHeight(height + InfoBarContainerDelegate::kSeparatorLineHeight);
   }
-
-  // Ensure the infobar is tall enough to display its contents.
-  int height = 0;
-  for (int i = 0; i < child_count(); ++i) {
-    View* child = child_at(i);
-    const gfx::Insets* const margins = child->GetProperty(views::kMarginsKey);
-    const int margin_height = margins ? margins->height() : 0;
-    height = std::max(height, child->height() + margin_height);
-  }
-  SetTargetHeight(height + InfoBarContainerDelegate::kSeparatorLineHeight);
 }
 
 void InfoBarView::OnThemeChanged() {
@@ -202,10 +205,8 @@ void InfoBarView::OnThemeChanged() {
   background()->SetNativeControlColor(background_color);
 
   const SkColor text_color = GetColor(kTextColor);
-  if (close_button_) {
-    views::SetImageFromVectorIcon(close_button_, vector_icons::kClose16Icon,
-                                  text_color);
-  }
+  views::SetImageFromVectorIcon(close_button_, vector_icons::kClose16Icon,
+                                text_color);
 
   for (int i = 0; i < child_count(); ++i) {
     View* child = child_at(i);
@@ -220,6 +221,9 @@ void InfoBarView::OnThemeChanged() {
 }
 
 void InfoBarView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
+  // The constructor could not set initial colors correctly, since the
+  // ThemeProvider wasn't available yet.  When this function is called, the view
+  // has been added to a Widget, so that ThemeProvider is now present.
   OnThemeChanged();
 }