From 54bccee689127f1c88afba973e12a45f14842435 Mon Sep 17 00:00:00 2001
From: "akuegel@chromium.org"
 <akuegel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 17 Sep 2013 22:10:34 +0000
Subject: [PATCH] There was a regression regarding the position of the avatar
 label. This CL fixes the regression and adds a unit test to avoid further
 regressions.

BUG=290699
TEST=unit_test

Review URL: https://chromiumcodereview.appspot.com/23820007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223712 0039d316-1c4b-4281-b951-d872f2087c98
---
 .../frame/opaque_browser_frame_view_layout.cc | 39 +++++++++++--------
 ...aque_browser_frame_view_layout_unittest.cc | 25 ++++++++++++
 2 files changed, 47 insertions(+), 17 deletions(-)

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 ae81d18cda927a..839b14c86d651c 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
@@ -372,24 +372,29 @@ void OpaqueBrowserFrameViewLayout::LayoutAvatar() {
       delegate_->ShouldShowAvatar() ? (avatar_bottom - avatar_y) : 0);
   if (avatar_button_) {
     avatar_button_->SetBoundsRect(avatar_bounds_);
-    leading_button_start_ += kAvatarLeftSpacing + incognito_icon.width();
-    minimum_size_for_buttons_ += kAvatarLeftSpacing + incognito_icon.width();
-  }
 
-  if (avatar_label_) {
-    // Space between the bottom of the avatar and the bottom of the avatar
-    // label.
-    const int kAvatarLabelBottomSpacing = 3;
-    // Space between the frame border and the left edge of the avatar label.
-    const int kAvatarLabelLeftSpacing = -1;
-    gfx::Size label_size = avatar_label_->GetPreferredSize();
-    gfx::Rect label_bounds(
-        leading_button_start_ + kAvatarLabelLeftSpacing,
-        avatar_bottom - kAvatarLabelBottomSpacing - label_size.height(),
-        label_size.width(),
-        delegate_->ShouldShowAvatar() ? label_size.height() : 0);
-    avatar_label_->SetBoundsRect(label_bounds);
-    leading_button_start_ += kAvatarLabelLeftSpacing + label_size.width();
+    if (avatar_label_) {
+      // 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.
+      gfx::Rect label_bounds(
+          leading_button_start_,
+          avatar_bottom - kAvatarLabelBottomSpacing - label_size.height(),
+          label_size.width(),
+          delegate_->ShouldShowAvatar() ? label_size.height() : 0);
+      avatar_label_->SetBoundsRect(label_bounds);
+      leading_button_start_ += label_size.width();
+    } else {
+      leading_button_start_ += kAvatarLeftSpacing + incognito_icon.width();
+    }
+
+    // We just add the avatar button size to the minimum size because clicking
+    // the avatar label does the same thing as clicking the avatar button.
+    minimum_size_for_buttons_ += kAvatarLeftSpacing + incognito_icon.width();
   }
 }
 
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 e0bd0461467875..2916c5fd9b5c0e 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
@@ -204,6 +204,15 @@ class OpaqueBrowserFrameViewLayoutTest : public views::ViewsTestBase {
     root_view_->AddChildView(menu_button_);
   }
 
+  void AddAvatarLabel() {
+    avatar_label_ = new views::MenuButton(NULL, string16(), NULL, false);
+    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 ExpectBasicWindowBounds() {
     EXPECT_EQ("428,1 25x18", maximize_button_->bounds().ToString());
     EXPECT_EQ("402,1 26x18", minimize_button_->bounds().ToString());
@@ -228,6 +237,7 @@ class OpaqueBrowserFrameViewLayoutTest : public views::ViewsTestBase {
   views::Label* window_title_;
 
   views::MenuButton* menu_button_;
+  views::MenuButton* avatar_label_;
 
   DISALLOW_COPY_AND_ASSIGN(OpaqueBrowserFrameViewLayoutTest);
 };
@@ -303,3 +313,18 @@ TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatar) {
                 delegate_->GetTabstripPreferredSize(), kWidth).ToString());
   EXPECT_EQ("261x73", layout_manager_->GetMinimumSize(kWidth).ToString());
 }
+
+TEST_F(OpaqueBrowserFrameViewLayoutTest, WindowWithAvatarLabelAndButton) {
+  AddAvatarLabel();
+  root_view_->Layout();
+
+  ExpectBasicWindowBounds();
+
+  // Check the location of the avatar label relative to the avatar button.
+  // 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());
+}