Support displaying the avatar label on the right side.

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h"
 
 #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"
 #include "chrome/common/chrome_switches.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_rep.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/label.h"
@@ skipping 196 matching lines @@
   }
 
   void AddAvatarButton() {
     menu_button_ = new AvatarMenuButton(NULL, false);
     menu_button_->set_id(VIEW_ID_AVATAR_BUTTON);
     delegate_->SetShouldShowAvatar(true);
     root_view_->AddChildView(menu_button_);
   }
 
   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_);
 
     // The avatar label should only be used together with the avatar button.
     AddAvatarButton();
   }
 
   void AddNewAvatarButton() {
    new_avatar_button_ =
        new views::MenuButton(NULL, base::string16(), NULL, false);
@@ skipping 15 matching lines @@
 
   // Widgets:
   views::ImageButton* minimize_button_;
   views::ImageButton* maximize_button_;
   views::ImageButton* restore_button_;
   views::ImageButton* close_button_;
 
   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);
 };
 
 TEST_F(OpaqueBrowserFrameViewLayoutTest, BasicWindow) {
   // Tests the layout of a default chrome window with no avatars, no window
   // titles, and a tabstrip.
   root_view_->Layout();
 
@@ skipping 153 matching lines @@
   EXPECT_EQ("7,11 40x29", menu_button_->bounds().ToString());
   EXPECT_EQ("45,13 352x29",
             layout_manager_->GetBoundsForTabStrip(
                 delegate_->GetTabstripPreferredSize(), kWidth).ToString());
   EXPECT_EQ("261x73", layout_manager_->GetMinimumSize(kWidth).ToString());
 }
 
 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);
   leading_buttons.push_back(views::FRAME_BUTTON_MINIMIZE);
   leading_buttons.push_back(views::FRAME_BUTTON_MAXIMIZE);
   layout_manager_->SetButtonOrdering(leading_buttons, trailing_buttons);
   root_view_->Layout();
 
   EXPECT_EQ("73,1 25x18", maximize_button_->bounds().ToString());
   EXPECT_EQ("47,1 26x18", minimize_button_->bounds().ToString());
   EXPECT_EQ("0,0 0x0", restore_button_->bounds().ToString());
   EXPECT_EQ("4,1 43x18", close_button_->bounds().ToString());
 
   // 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());
 
   // If the buttons are on the left, there should be no hidden icon for the user
   // to double click.
   EXPECT_EQ("0,0 0x0", layout_manager_->IconBounds().ToString());
 }
 
 TEST_F(OpaqueBrowserFrameViewLayoutTest,
@@ skipping 43 matching lines @@
   EXPECT_EQ("385,1 12x20", new_avatar_button_->bounds().ToString());
   // The basic window bounds are (-1, 13 398x29). There should not be an icon
   // avatar in the left, and the new avatar button has an offset of 5 to its
   // next control.
   EXPECT_EQ("-1,13 381x29",
             layout_manager_->GetBoundsForTabStrip(
                 delegate_->GetTabstripPreferredSize(), kWidth).ToString());
   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());
   EXPECT_EQ(
       menu_button_->bounds().bottom() - 3 - avatar_label_->bounds().height(),
       avatar_label_->bounds().y());
 }