Redesign of the avatar menu button.

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.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 "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"
 
 #if defined(OS_WIN)
 #include "win8/util/win8_util.h"
 #endif  // OS_WIN
 
 namespace {
 
@@ skipping 25 matching lines @@
 // way the tabstrip draws its bottom edge, will appear like a 1 px gap to the
 // user).
 const int kAvatarBottomSpacing = 2;
 
 // Space between the frame border and the left edge of the avatar.
 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.
 const int kNewTabCaptionRestoredSpacing = 5;
 
 // In maximized mode, where the New Tab button and the caption buttons are at
 // similar vertical coordinates, we need to reserve a larger, 16 px gap to avoid
 // looking too cluttered.
 const int kNewTabCaptionMaximizedSpacing = 16;
 
@@ skipping 32 matching lines @@
       has_trailing_buttons_(false),
       extra_caption_y_(kExtraCaption),
       window_caption_spacing_(kCaptionButtonSpacing),
       minimize_button_(NULL),
       maximize_button_(NULL),
       restore_button_(NULL),
       close_button_(NULL),
       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);
 }
 
 OpaqueBrowserFrameViewLayout::~OpaqueBrowserFrameViewLayout() {}
 
 // static
 bool OpaqueBrowserFrameViewLayout::ShouldAddDefaultCaptionButtons() {
 #if defined(OS_WIN)
   return !win8::IsSingleWindowMetroMode();
 #endif  // OS_WIN
   return true;
 }
 
 void OpaqueBrowserFrameViewLayout::SetButtonOrdering(
     const std::vector<views::FrameButton>& leading_buttons,
     const std::vector<views::FrameButton>& trailing_buttons) {
   leading_buttons_ = leading_buttons;
   trailing_buttons_ = trailing_buttons;
 }
 
 gfx::Rect OpaqueBrowserFrameViewLayout::GetBoundsForTabStrip(
     const gfx::Size& tabstrip_preferred_size,
     int available_width) const {
   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();
 
   const int caption_spacing = delegate_->IsMaximized() ?
       kNewTabCaptionMaximizedSpacing : kNewTabCaptionRestoredSpacing;
   const int tabstrip_width = available_width - caption_spacing;
   gfx::Rect bounds(leading_button_start_, GetTabStripInsetsTop(false),
                    std::max(0, tabstrip_width),
                    tabstrip_preferred_size.height());
 
@@ skipping 205 matching lines @@
       window_icon_bounds_ = gfx::Rect();
     } else {
       // We set the icon bounds to a small rectangle in the top leading corner
       // if there are no icons on the leading side.
       window_icon_bounds_ = gfx::Rect(
           frame_thickness + kIconLeftSpacing, frame_thickness, size, size);
     }
   }
 }
 
-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.
   gfx::ImageSkia incognito_icon = delegate_->GetOTRAvatarIcon();
 
   int avatar_bottom = GetTabStripInsetsTop(false) +
       delegate_->GetTabStripHeight() - kAvatarBottomSpacing;
   int avatar_restored_y = avatar_bottom - incognito_icon.height();
   int avatar_y = delegate_->IsMaximized() ?
       (NonClientTopBorderHeight(false) + kTabstripTopShadowThickness) :
       avatar_restored_y;
   avatar_bounds_.SetRect(leading_button_start_ + kAvatarLeftSpacing,
       avatar_y, incognito_icon.width(),
       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();
-    }
-
-    // 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::LayoutAvatar() {
+  LayoutIncognitoAvatar();
+  gfx::ImageSkia incognito_icon = delegate_->GetOTRAvatarIcon();
+
+  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(
     views::View* host,
     views::FrameButton button_id,
     ButtonAlignment alignment,
     int caption_y) {
   switch (button_id) {
     case views::FRAME_BUTTON_MINIMIZE: {
       minimize_button_->SetVisible(true);
       SetBoundsForButton(host, minimize_button_, alignment, caption_y);
       break;
@@ skipping 142 matching lines @@
                   view->GetClassName());
       }
       window_title_ = static_cast<views::Label*>(view);
       break;
     case VIEW_ID_AVATAR_LABEL:
       avatar_label_ = view;
       break;
     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;
   }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueBrowserFrameView, views::LayoutManager:
 
 void OpaqueBrowserFrameViewLayout::Layout(views::View* host) {
   // Reset all our data so that everything is invisible.
   int thickness = FrameBorderThickness(false);
   leading_button_start_ = thickness;
   trailing_button_start_ = thickness;
   minimum_size_for_buttons_ = leading_button_start_ + trailing_button_start_;
   has_leading_buttons_ = false;
   has_trailing_buttons_ = false;
 
   LayoutWindowControls(host);
   LayoutTitleBar(host);
 
   // We now add a single pixel to the leading spacing. We do this because the
   // avatar and tab strip start one pixel inward compared to where things start
   // 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());
 }
 
 gfx::Size OpaqueBrowserFrameViewLayout::GetPreferredSize(views::View* host) {
   // This is never used; NonClientView::GetPreferredSize() will be called
   // instead.
   NOTREACHED();
   return gfx::Size();
 }
 
 void OpaqueBrowserFrameViewLayout::ViewAdded(views::View* host,
                                              views::View* view) {
   SetView(view->id(), view);
 }
 
 void OpaqueBrowserFrameViewLayout::ViewRemoved(views::View* host,
                                                views::View* view) {
   SetView(view->id(), NULL);
 }