Support displaying the avatar label on the right side.

The avatar label should always be displayed on the same side
as the avatar button. Currently, the avatar label is always
displayed on the left, but the avatar icon can also be
displayed on the right. This CL moves the avatar label adds
support for displaying the avatar label on the right and moves
the avatar label to the right if the avatar icon is also on
the right.
A screenshot of the placement on the right can be found in the bug description.

BUG=332392
TEST=unit_tests

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245503 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 55 insertions, 19 deletions

diff --git a/chrome/browser/ui/views/avatar_label.cc b/chrome/browser/ui/views/avatar_label.cc
index de923db..9b35b8a 100644
--- a/chrome/browser/ui/views/avatar_label.cc
+++ b/chrome/browser/ui/views/avatar_label.cc
@@ -23,7 +23,7 @@ namespace {
 // A special text button border for the managed user avatar label.
 class AvatarLabelBorder: public views::TextButtonBorder {
  public:
-  explicit AvatarLabelBorder();
+  explicit AvatarLabelBorder(bool label_on_right);
 
   // views::TextButtonBorder:
   virtual void Paint(const views::View& view, gfx::Canvas* canvas) OVERRIDE;
@@ -35,9 +35,9 @@ class AvatarLabelBorder: public views::TextButtonBorder {
   DISALLOW_COPY_AND_ASSIGN(AvatarLabelBorder);
 };
 
-AvatarLabelBorder::AvatarLabelBorder() {
-  const int kHorizontalInsetRight = 10;
-  const int kHorizontalInsetLeft = 43;
+AvatarLabelBorder::AvatarLabelBorder(bool label_on_right) {
+  const int kHorizontalInsetRight = label_on_right ? 43 : 10;
+  const int kHorizontalInsetLeft = label_on_right ? 10 : 43;
   const int kVerticalInsetTop = 2;
   const int kVerticalInsetBottom = 3;
   // We want to align with the top of the tab. This works if the default font
@@ -96,7 +96,7 @@ AvatarLabel::AvatarLabel(BrowserView* browser_view)
                  l10n_util::GetStringUTF16(IDS_MANAGED_USER_AVATAR_LABEL)),
       browser_view_(browser_view) {
   ClearMaxTextSize();
-  set_border(new AvatarLabelBorder);
+  SetLabelOnRight(false);
   UpdateLabelStyle();
 }
 
@@ -111,6 +111,10 @@ bool AvatarLabel::OnMousePressed(const ui::MouseEvent& event) {
 }
 
 void AvatarLabel::UpdateLabelStyle() {
+  // |browser_view_| can be NULL in unit tests.
+  if (!browser_view_)
+    return;
+
   SkColor color_label = browser_view_->frame()->GetThemeProvider()->GetColor(
       ThemeProperties::COLOR_MANAGED_USER_LABEL);
   SetEnabledColor(color_label);
@@ -119,3 +123,7 @@ void AvatarLabel::UpdateLabelStyle() {
   SetDisabledColor(color_label);
   SchedulePaint();
 }
+
+void AvatarLabel::SetLabelOnRight(bool label_on_right) {
+  set_border(new AvatarLabelBorder(label_on_right));
+}
diff --git a/chrome/browser/ui/views/avatar_label.h b/chrome/browser/ui/views/avatar_label.h
index d22c179..accd282 100644
--- a/chrome/browser/ui/views/avatar_label.h
+++ b/chrome/browser/ui/views/avatar_label.h
@@ -30,6 +30,11 @@ class AvatarLabel : public views::TextButton {
   // Update the style of the label according to the provided theme.
   void UpdateLabelStyle();
 
+  // Sets whether the label should be displayed on the right or on the left. A
+  // new button border is created which has the right insets for the positioning
+  // of the button.
+  void SetLabelOnRight(bool label_on_right);
+
  private:
   BrowserView* browser_view_;
 
diff --git a/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc b/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc
index 982bdf3..c34a4b8 100644
--- a/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc
+++ b/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h"
 
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/ui/views/avatar_label.h"
 #include "chrome/browser/ui/views/avatar_menu_button.h"
 #include "chrome/common/profile_management_switches.h"
 #include "ui/gfx/font.h"
@@ -431,15 +432,18 @@ void OpaqueBrowserFrameViewLayout::LayoutAvatar(views::View* host) {
 
     int edge_offset;
     if (avatar_label_) {
+      avatar_label_->SetLabelOnRight(avatar_on_right);
       // Space between the bottom of the avatar and the bottom of the avatar
       // label.
       const int kAvatarLabelBottomSpacing = 3;
       gfx::Size label_size = avatar_label_->GetPreferredSize();
-      // The x-position of the avatar label should be slightly to the left of
-      // the avatar menu button. Therefore we use the |leading_button_start_|
-      // value directly.
+      // The outside edge of the avatar label should be just outside that of the
+      // avatar menu button.
+      int avatar_label_x = avatar_on_right ?
+          (host->width() - trailing_button_start_ - label_size.width()) :
+          leading_button_start_;
       gfx::Rect label_bounds(
-          leading_button_start_,
+          avatar_label_x,
           avatar_bottom - kAvatarLabelBottomSpacing - label_size.height(),
           label_size.width(),
           delegate_->ShouldShowAvatar() ? label_size.height() : 0);
@@ -626,7 +630,7 @@ void OpaqueBrowserFrameViewLayout::SetView(int id, views::View* view) {
       window_title_ = static_cast<views::Label*>(view);
       break;
     case VIEW_ID_AVATAR_LABEL:
-      avatar_label_ = view;
+      avatar_label_ = static_cast<AvatarLabel*>(view);
       break;
     case VIEW_ID_AVATAR_BUTTON:
       if (view) {
diff --git a/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h b/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h
index 41d6245..90d0125 100644
--- a/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h
+++ b/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h
@@ -9,6 +9,7 @@
 #include "ui/views/layout/layout_manager.h"
 #include "ui/views/window/frame_buttons.h"
 
+class AvatarLabel;
 class AvatarMenuButton;
 class NewAvatarButton;
 class OpaqueBrowserFrameViewLayoutDelegate;
@@ -178,7 +179,7 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
   views::View* window_icon_;
   views::Label* window_title_;
 
-  views::View* avatar_label_;
+  AvatarLabel* avatar_label_;
   AvatarMenuButton* avatar_button_;
   views::View* new_avatar_button_;
 
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 59cfc0b..d17bd54 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,26 @@ 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.
+  gfx::Rect tab_strip_bounds = layout_manager_->GetBoundsForTabStrip(
+      delegate_->GetTabstripPreferredSize(), kWidth);
+  EXPECT_GT(tab_strip_bounds.x(), maximize_button_->bounds().x());
+  EXPECT_GT(maximize_button_->bounds().right(), tab_strip_bounds.x());
+  EXPECT_EQ(13, tab_strip_bounds.y());
+  EXPECT_EQ(29, tab_strip_bounds.height());
+  EXPECT_GT(avatar_label_->bounds().x(), tab_strip_bounds.right());
   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_GT(avatar_label_->bounds().right(), menu_button_->bounds().right());
+  EXPECT_GT(menu_button_->bounds().x(), avatar_label_->bounds().x());
+  EXPECT_GT(menu_button_->bounds().bottom(),
+            avatar_label_->bounds().bottom());
+  EXPECT_GT(avatar_label_->bounds().y(), menu_button_->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 +535,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());