Just set borders once when creating a views::LabelButton

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

Index: chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
index 3262df593f0a94ce6001257517e0d9e28ec48769..7a8319d84e5bf70f0a1133c7c65bb546fd2b6604 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc
@@ -287,23 +287,15 @@ class BookmarkBarViewEventTestBase : public ViewEventTestBase {
 
     AddTestData(CreateBigMenu());
 
-    // Calculate the preferred size so that one button doesn't fit, which
-    // 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.
-    bb_view_pref_ = bb_view_->GetPreferredSize();
-    bb_view_pref_.set_width(1000);
-    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();
-    } while (GetBookmarkButton(6)->visible());
-
+    // Create the Widget. Note the initial size is given by GetPreferredSize()
+    // during initialization. This occurs after the WidgetDelegate provides
+    // |bb_view_| as the contents view and adds it to the hierarchy.
     ViewEventTestBase::SetUp();
+
+    // Verify the layout triggered by the initial size preserves the overflow
+    // state calculated in GetPreferredSize().
+    EXPECT_TRUE(GetBookmarkButton(5)->visible());
+    EXPECT_FALSE(GetBookmarkButton(6)->visible());
   }
 
   void TearDown() override {
@@ -333,7 +325,25 @@ class BookmarkBarViewEventTestBase : public ViewEventTestBase {
  protected:
   views::View* CreateContentsView() override { return bb_view_.get(); }
 
-  gfx::Size GetPreferredSize() const override { return bb_view_pref_; }
+  gfx::Size GetPreferredSize() const override {
+    // Calculate the preferred size so that one button doesn't fit, which
+    // 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 it is written so that it shouldn't

[msw, 2015/07/13 17:54:03]

nit: This code *does* look hacky; it's fine to leave as-is for now, since it's
been working (right?), but why not layout at full size, then get the bounds of
bb_view_->GetBookmarkButton(6), and return a size slightly inset from its right
edge? (or at least remove the size.set_width(1000), since
bb_view_->GetPreferredSize() should give a sufficient width to show the button)

[tapted, 2015/07/14 05:36:12]

I actually tried this while investigating one of the test failures. The problem
is that there are other things to the right of the last button -- the "Other
Bookmarks" button, and potentially other things (ManagedBookmarksButton,
SupervisedBookmarksButton) if a test were to enable those. So the current
approach is more robust to those things, and avoids coupling the test to a
specific layout. And, although I did suspect if for being fragile, it turned out
to be something else. So, it does seem to work fine.

There are, however, some tests disabled, and the whole file is skipped for
official CrOS builds :/ http://crbug.com/431450 http://crbug.com/431427 . I
wouldn't rule this logic out as a culprit, but most of the tests are still
enabled elsewhere, and the trybots are giving pretty good coverage (e.g. lots of
red when I screwed it up :p)

+    // depend on any of the layout code in BookmarkBarView, or extra buttons
+    // added to the right of the bookmarks. Instead, brute force search for a
+    // size that triggers the overflow button.
+    gfx::Size size = bb_view_->GetPreferredSize();
+    size.set_width(1000);
+    do {
+      size.set_width(size.width() - 25);
+      bb_view_->SetBounds(0, 0, size.width(), size.height());
+      bb_view_->Layout();
+    } while (bb_view_->GetBookmarkButton(6)->visible());
+    return size;
+  }
 
   views::LabelButton* GetBookmarkButton(int view_index) {
     return bb_view_->GetBookmarkButton(view_index);
@@ -379,7 +389,6 @@ class BookmarkBarViewEventTestBase : public ViewEventTestBase {
     model_->AddURL(of2, 1, ASCIIToUTF16("of2b"), GURL(test_base + "of2b"));
   }
 
-  gfx::Size bb_view_pref_;
   scoped_ptr<ChromeContentClient> content_client_;
   scoped_ptr<chrome::ChromeContentBrowserClient> browser_content_client_;
   scoped_ptr<TestingProfile> profile_;