Support displaying the avatar label on the right side.

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc

Index: chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
diff --git a/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc b/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
index 59cfc0b7810c8ec5a3219995fa93646067ee140f..f94c9e86d968630fea87aa76a495b4ba571575dc 100644
--- a/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
+++ b/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/ui/views/avatar_label.h"
 #include "chrome/browser/ui/views/avatar_menu_button.h"
 #include "chrome/browser/ui/views/tab_icon_view.h"
 #include "chrome/browser/ui/views/tabs/tab.h"
@@ -223,7 +224,7 @@ class OpaqueBrowserFrameViewLayoutTest : public views::ViewsTestBase {
   }
 
   void AddAvatarLabel() {
-    avatar_label_ = new views::MenuButton(NULL, base::string16(), NULL, false);
+    avatar_label_ = new AvatarLabel(NULL);
     avatar_label_->set_id(VIEW_ID_AVATAR_LABEL);
     root_view_->AddChildView(avatar_label_);
 
@@ -259,8 +260,8 @@ class OpaqueBrowserFrameViewLayoutTest : public views::ViewsTestBase {
   TabIconView* tab_icon_view_;
   views::Label* window_title_;
 
+  AvatarLabel* avatar_label_;
   AvatarMenuButton* menu_button_;
-  views::MenuButton* avatar_label_;
   views::MenuButton* new_avatar_button_;
 
   DISALLOW_COPY_AND_ASSIGN(OpaqueBrowserFrameViewLayoutTest);
@@ -434,7 +435,8 @@ TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatar) {
 TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatarWithButtonsOnLeft) {
   // Tests the layout of a chrome window with an avatar icon and caption buttons
   // on the left. The avatar icon should therefore be on the right.
-  AddAvatarButton();
+  // AddAvatarLabel() also adds the avatar button.
+  AddAvatarLabel();
   std::vector<views::FrameButton> leading_buttons;
   std::vector<views::FrameButton> trailing_buttons;
   leading_buttons.push_back(views::FRAME_BUTTON_CLOSE);
@@ -450,11 +452,32 @@ TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatarWithButtonsOnLeft) {
 
   // Check the location of the avatar
   EXPECT_EQ("454,11 40x29", menu_button_->bounds().ToString());
-  EXPECT_EQ("93,13 356x29",
-            layout_manager_->GetBoundsForTabStrip(
-                delegate_->GetTabstripPreferredSize(), kWidth).ToString());
+
+  // Check the tab strip bounds. These bounds depend on the avatar label, which
+  // can have a different width on Linux and Windows depending on the font size
+  // used.
+  int avatar_button_width = menu_button_->bounds().width();
+  int avatar_label_width = avatar_label_->bounds().width();
+  const int kAvatarIconToLabelSpacing = 2;

[Peter Kasting, 2014/01/15 02:06:26]

There are a lot of magic numbers in here.  It would be nice to only use numbers
defined in the implementation, instead of having to define more numbers in the
test.

Furthermore, we can avoid the need for these numbers, and be more resilient
against future changes, by loosening the checks.  For example, instead of
checking that the label bounds are at some exact set of coordinates, we can just
use e.g. EXPECT_GT() and similar inequality checks to ensure things appear in
the right order.

[Adrian Kuegel, 2014/01/15 16:24:51]

I tried to rewrite my changes to do only relative checks (expect for the exact
checks that are also used elsewhere). This should now ensure that the avatar
label will not be in the region of the tab strip, and that the avatar icon/
avatar label have the correct relative order of x(), right(), y() and bottom()
positions.

+  int expected_width = 356 - (avatar_label_width - avatar_button_width) +
+                       kAvatarIconToLabelSpacing;
+  gfx::Rect tab_strip_bounds = layout_manager_->GetBoundsForTabStrip(
+      delegate_->GetTabstripPreferredSize(), kWidth);
+  EXPECT_EQ(93, tab_strip_bounds.x());
+  EXPECT_EQ(13, tab_strip_bounds.y());
+  EXPECT_EQ(expected_width, tab_strip_bounds.width());
+  EXPECT_EQ(29, tab_strip_bounds.height());
   EXPECT_EQ("261x73", layout_manager_->GetMinimumSize(kWidth).ToString());
 
+  // Check the relative location of the avatar label to the avatar. The right
+  // end of the avatar label should be slightly to the right of the right end of
+  // the avatar icon.
+  EXPECT_EQ(menu_button_->bounds().right() + kAvatarIconToLabelSpacing,
+            avatar_label_->bounds().right());
+  EXPECT_EQ(
+      menu_button_->bounds().bottom() - 3 - avatar_label_->bounds().height(),
+      avatar_label_->bounds().y());
+
   // This means that the menu will pop out facing the left (if it were to face
   // the right, it would go outside the window frame and be clipped).
   EXPECT_TRUE(menu_button_->button_on_right());
@@ -518,13 +541,14 @@ TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithNewAvatar) {
   EXPECT_EQ("261x73", layout_manager_->GetMinimumSize(kWidth).ToString());
 }
 
-TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatarLabelAndButton) {
+TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatarLabelAndButtonOnLeft) {
   AddAvatarLabel();
   root_view_->Layout();
 
   ExpectBasicWindowBounds();
 
-  // Check the location of the avatar label relative to the avatar button.
+  // Check the location of the avatar label relative to the avatar button if
+  // both are displayed on the left side.
   // The label height and width depends on the font size and the text displayed.
   // This may possibly change, so we don't test it here.
   EXPECT_EQ(menu_button_->bounds().x() - 2, avatar_label_->bounds().x());