Minor cleanup in BookmarkBarView

Nukes LayoutItems and folds into Layout as that is the only place
calling into it. Removes early out if parent() is NULL in Layout. No
point in doing that.

BUG=416641
TEST=none
R=msw@chromium.org

Review URL: https://codereview.chromium.org/656033007

Cr-Commit-Position: refs/heads/master@{#299920}

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -783,7 +783,109 @@ gfx::Size BookmarkBarView::GetMinimumSize() const {
 }
 
 void BookmarkBarView::Layout() {
-  LayoutItems();
+  int x = kLeftMargin;
+  int top_margin = IsDetached() ? kDetachedTopMargin : 0;
+  int y = top_margin;
+  int width = View::width() - kRightMargin - kLeftMargin;
+  int height = chrome::kBookmarkBarHeight - kBottomMargin;
+  int separator_margin = kSeparatorMargin;
+
+  if (IsDetached()) {
+    double current_state = 1 - size_animation_->GetCurrentValue();
+    x += static_cast<int>(kNewtabHorizontalPadding * current_state);
+    y += (View::height() - chrome::kBookmarkBarHeight) / 2;
+    width -= static_cast<int>(kNewtabHorizontalPadding * current_state);
+    separator_margin -= static_cast<int>(kSeparatorMargin * current_state);
+  } else {
+    // For the attached appearance, pin the content to the bottom of the bar
+    // when animating in/out, as shrinking its height instead looks weird.  This
+    // also matches how we layout infobars.
+    y += View::height() - chrome::kBookmarkBarHeight;
+  }
+
+  gfx::Size other_bookmarked_pref = other_bookmarked_button_->visible() ?
+      other_bookmarked_button_->GetPreferredSize() : gfx::Size();
+  gfx::Size overflow_pref = overflow_button_->GetPreferredSize();
+  gfx::Size bookmarks_separator_pref =
+      bookmarks_separator_view_->GetPreferredSize();
+  gfx::Size apps_page_shortcut_pref = apps_page_shortcut_->visible() ?
+      apps_page_shortcut_->GetPreferredSize() : gfx::Size();
+
+  int max_x = width - overflow_pref.width() - kButtonPadding -
+      bookmarks_separator_pref.width();
+  if (other_bookmarked_button_->visible())
+    max_x -= other_bookmarked_pref.width() + kButtonPadding;
+
+  // Next, layout out the buttons. Any buttons that are placed beyond the
+  // visible region are made invisible.
+
+  // Start with the apps page shortcut button.
+  if (apps_page_shortcut_->visible()) {
+    apps_page_shortcut_->SetBounds(x, y, apps_page_shortcut_pref.width(),
+                                   height);
+    x += apps_page_shortcut_pref.width() + kButtonPadding;
+  }
+
+  // Then comes the managed bookmarks folder, if visible.
+  if (managed_bookmarks_button_->visible()) {
+    gfx::Size managed_bookmarks_pref = managed_bookmarks_button_->visible() ?
+        managed_bookmarks_button_->GetPreferredSize() : gfx::Size();
+    managed_bookmarks_button_->SetBounds(x, y, managed_bookmarks_pref.width(),
+                                         height);
+    x += managed_bookmarks_pref.width() + kButtonPadding;
+  }
+
+  // Then go through the bookmark buttons.
+  if (GetBookmarkButtonCount() == 0 && model_ && model_->loaded()) {
+    gfx::Size pref = instructions_->GetPreferredSize();
+    instructions_->SetBounds(
+        x + kInstructionsPadding, y,
+        std::min(static_cast<int>(pref.width()),
+                 max_x - x),
+        height);
+    instructions_->SetVisible(true);
+  } else {
+    instructions_->SetVisible(false);
+
+    for (int i = 0; i < GetBookmarkButtonCount(); ++i) {
+      views::View* child = child_at(i);
+      gfx::Size pref = child->GetPreferredSize();
+      int next_x = x + pref.width() + kButtonPadding;
+      child->SetVisible(next_x < max_x);
+      child->SetBounds(x, y, pref.width(), height);
+      x = next_x;
+    }
+  }
+
+  // Layout the right side of the bar.
+  const bool all_visible = (GetBookmarkButtonCount() == 0 ||
+                            child_at(GetBookmarkButtonCount() - 1)->visible());
+
+  // Layout the right side buttons.
+  x = max_x + kButtonPadding;
+
+  // The overflow button.
+  overflow_button_->SetBounds(x, y, overflow_pref.width(), height);
+  overflow_button_->SetVisible(!all_visible);
+  x += overflow_pref.width();
+
+  // Separator.
+  if (bookmarks_separator_view_->visible()) {
+    bookmarks_separator_view_->SetBounds(x,
+                                         y - top_margin,
+                                         bookmarks_separator_pref.width(),
+                                         height + top_margin + kBottomMargin -
+                                         separator_margin);
+
+    x += bookmarks_separator_pref.width();
+  }
+
+  // The other bookmarks button.
+  if (other_bookmarked_button_->visible()) {
+    other_bookmarked_button_->SetBounds(x, y, other_bookmarked_pref.width(),
+                                        height);
+    x += other_bookmarked_pref.width() + kButtonPadding;
+  }
 }
 
 void BookmarkBarView::ViewHierarchyChanged(
@@ -1861,115 +1963,6 @@ void BookmarkBarView::UpdateBookmarksSeparatorVisibility() {
       other_bookmarked_button_->visible());
 }
 
-void BookmarkBarView::LayoutItems() {
-  if (!parent())
-    return;
-
-  int x = kLeftMargin;
-  int top_margin = IsDetached() ? kDetachedTopMargin : 0;
-  int y = top_margin;
-  int width = View::width() - kRightMargin - kLeftMargin;
-  int height = chrome::kBookmarkBarHeight - kBottomMargin;
-  int separator_margin = kSeparatorMargin;
-
-  if (IsDetached()) {
-    double current_state = 1 - size_animation_->GetCurrentValue();
-    x += static_cast<int>(kNewtabHorizontalPadding * current_state);
-    y += (View::height() - chrome::kBookmarkBarHeight) / 2;
-    width -= static_cast<int>(kNewtabHorizontalPadding * current_state);
-    separator_margin -= static_cast<int>(kSeparatorMargin * current_state);
-  } else {
-    // For the attached appearance, pin the content to the bottom of the bar
-    // when animating in/out, as shrinking its height instead looks weird.  This
-    // also matches how we layout infobars.
-    y += View::height() - chrome::kBookmarkBarHeight;
-  }
-
-  gfx::Size other_bookmarked_pref = other_bookmarked_button_->visible() ?
-      other_bookmarked_button_->GetPreferredSize() : gfx::Size();
-  gfx::Size overflow_pref = overflow_button_->GetPreferredSize();
-  gfx::Size bookmarks_separator_pref =
-      bookmarks_separator_view_->GetPreferredSize();
-  gfx::Size apps_page_shortcut_pref = apps_page_shortcut_->visible() ?
-      apps_page_shortcut_->GetPreferredSize() : gfx::Size();
-
-  int max_x = width - overflow_pref.width() - kButtonPadding -
-      bookmarks_separator_pref.width();
-  if (other_bookmarked_button_->visible())
-    max_x -= other_bookmarked_pref.width() + kButtonPadding;
-
-  // Next, layout out the buttons. Any buttons that are placed beyond the
-  // visible region are made invisible.
-
-  // Start with the apps page shortcut button.
-  if (apps_page_shortcut_->visible()) {
-    apps_page_shortcut_->SetBounds(x, y, apps_page_shortcut_pref.width(),
-                                   height);
-    x += apps_page_shortcut_pref.width() + kButtonPadding;
-  }
-
-  // Then comes the managed bookmarks folder, if visible.
-  if (managed_bookmarks_button_->visible()) {
-    gfx::Size managed_bookmarks_pref = managed_bookmarks_button_->visible() ?
-        managed_bookmarks_button_->GetPreferredSize() : gfx::Size();
-    managed_bookmarks_button_->SetBounds(x, y, managed_bookmarks_pref.width(),
-                                         height);
-    x += managed_bookmarks_pref.width() + kButtonPadding;
-  }
-
-  // Then go through the bookmark buttons.
-  if (GetBookmarkButtonCount() == 0 && model_ && model_->loaded()) {
-    gfx::Size pref = instructions_->GetPreferredSize();
-    instructions_->SetBounds(
-        x + kInstructionsPadding, y,
-        std::min(static_cast<int>(pref.width()),
-                 max_x - x),
-        height);
-    instructions_->SetVisible(true);
-  } else {
-    instructions_->SetVisible(false);
-
-    for (int i = 0; i < GetBookmarkButtonCount(); ++i) {
-      views::View* child = child_at(i);
-      gfx::Size pref = child->GetPreferredSize();
-      int next_x = x + pref.width() + kButtonPadding;
-      child->SetVisible(next_x < max_x);
-      child->SetBounds(x, y, pref.width(), height);
-      x = next_x;
-    }
-  }
-
-  // Layout the right side of the bar.
-  const bool all_visible = (GetBookmarkButtonCount() == 0 ||
-                            child_at(GetBookmarkButtonCount() - 1)->visible());
-
-  // Layout the right side buttons.
-  x = max_x + kButtonPadding;
-
-  // The overflow button.
-  overflow_button_->SetBounds(x, y, overflow_pref.width(), height);
-  overflow_button_->SetVisible(!all_visible);
-  x += overflow_pref.width();
-
-  // Separator.
-  if (bookmarks_separator_view_->visible()) {
-    bookmarks_separator_view_->SetBounds(x,
-                                         y - top_margin,
-                                         bookmarks_separator_pref.width(),
-                                         height + top_margin + kBottomMargin -
-                                         separator_margin);
-
-    x += bookmarks_separator_pref.width();
-  }
-
-  // The other bookmarks button.
-  if (other_bookmarked_button_->visible()) {
-    other_bookmarked_button_->SetBounds(x, y, other_bookmarked_pref.width(),
-                                        height);
-    x += other_bookmarked_pref.width() + kButtonPadding;
-  }
-}
-
 void BookmarkBarView::OnAppsPageShortcutVisibilityPrefChanged() {
   DCHECK(apps_page_shortcut_);
   // Only perform layout if required.

chrome/browser/ui/views/bookmarks/bookmark_bar_view.h

@@ -379,9 +379,6 @@ class BookmarkBarView : public DetachableToolbarView,
   // Updates the visibility of |bookmarks_separator_view_|.
   void UpdateBookmarksSeparatorVisibility();
 
-  // This method computes the bounds for the bookmark bar items.
-  void LayoutItems();
-
   // Updates the visibility of the apps shortcut based on the pref value.
   void OnAppsPageShortcutVisibilityPrefChanged();
 

chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc

@@ -283,29 +283,20 @@ class BookmarkBarViewEventTestBase : public ViewEventTestBase {
     AddTestData(CreateBigMenu());
 
     // Calculate the preferred size so that one button doesn't fit, which
-    // triggers the overflow button to appear.
-    //
-    // BookmarkBarView::Layout does nothing if the parent is NULL and
-    // GetPreferredSize hard codes a width of 1. For that reason we add the
-    // BookmarkBarView to a dumby view as the parent.
+    // triggers the overflow button to appear. We have to do this incrementally
+    // as there isn't a good way to determine the point at which the overflow
+    // button is shown.
     //
     // This code looks a bit hacky, but I've written it so that it shouldn't
     // be dependant upon any of the layout code in BookmarkBarView. Instead
     // we brute force search for a size that triggers the overflow button.
-    views::View tmp_parent;
-
-    tmp_parent.AddChildView(bb_view_.get());
-
     bb_view_pref_ = bb_view_->GetPreferredSize();
     bb_view_pref_.set_width(1000);
-    views::LabelButton* button = GetBookmarkButton(6);
-    while (button->visible()) {
+    do {
       bb_view_pref_.set_width(bb_view_pref_.width() - 25);
       bb_view_->SetBounds(0, 0, bb_view_pref_.width(), bb_view_pref_.height());
       bb_view_->Layout();
-    }
-
-    tmp_parent.RemoveChildView(bb_view_.get());
+    } while (GetBookmarkButton(6)->visible());
 
     ViewEventTestBase::SetUp();
   }