Redesign of the avatar menu button.

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

Index: chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc
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 839b14c86d651c4f6d46bb5b5138266fd3cdb18c..0a396cfbb02a1728341065e752f1249f37191512 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
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h"
 
+#include "chrome/browser/profiles/profiles_state.h"
 #include "ui/gfx/font.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/label.h"
@@ -49,6 +50,9 @@ const int kAvatarLeftSpacing = 2;
 // Space between the right edge of the avatar and the tabstrip.
 const int kAvatarRightSpacing = -4;
 
+// How far the new avatar button is from the left of the minimize button.
+const int kNewAvatarButtonOffset = 5;
+
 // In restored mode, the New Tab button isn't at the same height as the caption
 // buttons, but the space will look cluttered if it actually slides under them,
 // so we stop it when the gap between the two is down to 5 px.
@@ -101,7 +105,8 @@ OpaqueBrowserFrameViewLayout::OpaqueBrowserFrameViewLayout(
       window_icon_(NULL),
       window_title_(NULL),
       avatar_label_(NULL),
-      avatar_button_(NULL) {
+      avatar_button_(NULL),
+      new_avatar_button_(NULL) {
   trailing_buttons_.push_back(views::FRAME_BUTTON_MINIMIZE);
   trailing_buttons_.push_back(views::FRAME_BUTTON_MAXIMIZE);
   trailing_buttons_.push_back(views::FRAME_BUTTON_CLOSE);
@@ -130,6 +135,10 @@ gfx::Rect OpaqueBrowserFrameViewLayout::GetBoundsForTabStrip(
   available_width -= trailing_button_start_;
   available_width -= leading_button_start_;
 
+  // The new avatar button is displayed to the left of the caption area buttons.
+  if (delegate_->ShouldShowAvatar() && new_avatar_button_)
+    available_width -= new_avatar_button_->width();

[Elliot Glaysher, 2013/09/26 21:05:44]

This shouldn't be calculated here; rather, it should indirectly be calculated by
working with either the existing minimum size calculations, or the existing
leading/trailing calculations.

[noms (inactive), 2013/10/01 17:42:21]

Done.

+
   if (delegate_->GetAdditionalReservedSpaceInTabStrip())
     available_width -= delegate_->GetAdditionalReservedSpaceInTabStrip();
 
@@ -355,7 +364,17 @@ void OpaqueBrowserFrameViewLayout::LayoutTitleBar(views::View* host) {
   }
 }
 
-void OpaqueBrowserFrameViewLayout::LayoutAvatar() {
+void OpaqueBrowserFrameViewLayout::LayoutNewStyleAvatar() {

[Elliot Glaysher, 2013/09/26 21:05:44]

Since the new avatar menu looks like a caption button, please plumb
"ButtonAlignment alignment" here and act on it.

[noms (inactive), 2013/10/01 17:42:21]

I've used the has_trailing_buttons_ boolean that gets set after
LayoutWindowControls runs. I don't know what should happen if both that and
has_leading_buttons_ are set to true -- could this realistically ever happen?

On 2013/09/26 21:05:44, Elliot Glaysher wrote:

[Elliot Glaysher, 2013/10/01 18:12:56]

Yeap! "close:minimize" is the default button layout on some window managers (no
maximize button!) and we deal with that in the gtk port today. (Also, since
button layout is under user control, we have to handle it anyway...)

In such cases, the GTK port defaults to putting the avatar selector on whichever
side has the least buttons, biased to the right in case of tie, which is the
policy I'd prefer to see going forward. (Currently, the views code here just
*always* puts things to the left, but that's only because I'm swamped and have
much higher priority things to do right now to get linux_aura out the door.)

[noms (inactive), 2013/10/03 19:30:14]

After offline conversation, I've made it so that the button always gets added to
the trailing buttons. For now this is only supported on Windows, where the
button should go to the right, with the trailing buttons. On Linux/Mac we don't
have UI specs yet, so it's unclear where the button _should_ go, but it's likely
it will go as a trailing button as not to clutter the area before the first tab
too much. 

This will get revisited as soon as there are Linux UI specs, I promise :)

On 2013/10/01 18:12:56, Elliot Glaysher wrote:

+  gfx::Size label_size = new_avatar_button_->GetPreferredSize();
+  int caption_y = CaptionButtonY(false);
+  new_avatar_button_->SetBounds(
+      minimize_button_->x() - label_size.width() - kNewAvatarButtonOffset,
+      caption_y,
+      label_size.width(),
+      caption_y + kCaptionButtonHeightWithPadding);

[Elliot Glaysher, 2013/09/26 21:05:44]

This position calculation is wrong. You can't position things in terms of the
minimize button because it's not always on the right side of the screen. (And
when it is, it's not guaranteed to be the leftmost button on the right side.
"close:maximize,minimize" is valid button layout here.)

[noms (inactive), 2013/10/01 17:42:21]

Done.

+}
+
+void OpaqueBrowserFrameViewLayout::LayoutIncognitoAvatar() {
   // Even though the avatar is used for both incognito and profiles we always
   // use the incognito icon to layout the avatar button. The profile icon
   // can be customized so we can't depend on its size to perform layout.
@@ -372,30 +391,38 @@ void OpaqueBrowserFrameViewLayout::LayoutAvatar() {
       delegate_->ShouldShowAvatar() ? (avatar_bottom - avatar_y) : 0);
   if (avatar_button_) {
     avatar_button_->SetBoundsRect(avatar_bounds_);

[Elliot Glaysher, 2013/09/26 21:05:44]

If you're pulling out the code block after this (not adjusting
leading_button_start_ or minimum_size_for_buttons_), the incognito avatar
probably doesn't take up any space in the tabstrip; I suspect the tabs can be
placed on top of the avatar.

[noms (inactive), 2013/10/01 17:42:21]

I don't actually know what I was thinking when I split this code up. :| 
Reverted.
On 2013/09/26 21:05:44, Elliot Glaysher wrote:

+  }
+}
 
-    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();
-    }
+void OpaqueBrowserFrameViewLayout::LayoutAvatar() {
+  LayoutIncognitoAvatar();
+  gfx::ImageSkia incognito_icon = delegate_->GetOTRAvatarIcon();
 
-    // 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();
+  int avatar_bottom = GetTabStripInsetsTop(false) +
+      delegate_->GetTabStripHeight() - kAvatarBottomSpacing;
+
+  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();
 }
 
 void OpaqueBrowserFrameViewLayout::ConfigureButton(
@@ -560,6 +587,9 @@ void OpaqueBrowserFrameViewLayout::SetView(int id, views::View* view) {
     case VIEW_ID_AVATAR_BUTTON:
       avatar_button_ = view;
       break;
+    case VIEW_ID_NEW_AVATAR_BUTTON:
+      new_avatar_button_ = view;
+      break;
     default:
       NOTIMPLEMENTED() << "Unknown view id " << id;
       break;
@@ -586,7 +616,14 @@ void OpaqueBrowserFrameViewLayout::Layout(views::View* host) {
   // on the trailing side.
   leading_button_start_++;
 
-  LayoutAvatar();
+  if (profiles::IsNewProfileManagementEnabled()) {
+    if (delegate_->IsOffTheRecord() && !delegate_->IsGuestSession())
+      LayoutIncognitoAvatar();
+    else
+      LayoutNewStyleAvatar();
+  } else {
+    LayoutAvatar();
+  }
 
   client_view_bounds_ = CalculateClientAreaBounds(
       host->width(), host->height());