Customize user details in ash system bubble for public account mode

ash/system/user/tray_user.cc

Index: ash/system/user/tray_user.cc
diff --git a/ash/system/user/tray_user.cc b/ash/system/user/tray_user.cc
index 78ca43b0b2e2b8d520ff8364b73ac719e9bb76a8..59cc2d0da5db9dba1856a1a326c4c2778ba40e8b 100644
--- a/ash/system/user/tray_user.cc
+++ b/ash/system/user/tray_user.cc
@@ -4,37 +4,61 @@
 
 #include "ash/system/user/tray_user.h"
 
+#include <algorithm>
+#include <climits>
+#include <queue>
+#include <vector>
+
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/system_tray_delegate.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_item_view.h"
 #include "ash/system/tray/tray_views.h"
+#include "base/i18n/rtl.h"
+#include "base/logging.h"
+#include "base/memory/scoped_vector.h"
+#include "base/string16.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "grit/ash_strings.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkPath.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/text/text_elider.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/font.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia_operations.h"
+#include "ui/gfx/insets.h"
+#include "ui/gfx/rect.h"
+#include "ui/gfx/render_text.h"
 #include "ui/gfx/size.h"
 #include "ui/gfx/skia_util.h"
+#include "ui/views/border.h"
 #include "ui/views/controls/button/button.h"
-#include "ui/views/controls/button/text_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
+#include "ui/views/controls/link.h"
+#include "ui/views/controls/link_listener.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
-const int kUserInfoVerticalPadding = 10;
-const int kUserIconSize = 27;
+const int kUserDetailsVerticalPadding = 5;
+const int kUserCardVerticalPadding = 10;
 const int kProfileRoundedCornerRadius = 2;
+const int kUserIconSize = 27;
+
+// The invisible word joiner character, used as a marker to indicate the start
+// and end of the user's display name in the public account user card's text.
+const char16 kDisplayNameMark[] = { 0x2060 };
 
 }  // namespace
 
@@ -99,148 +123,357 @@ class RoundedImageView : public views::View {
   DISALLOW_COPY_AND_ASSIGN(RoundedImageView);
 };
 
-class UserView : public views::View,
-                 public views::ButtonListener {
+// The user details shown in public account mode. This is essentially a label
+// but with custom painting code as the text is styled with multiple colors and
+// contains a link.
+class PublicAccountUserDetails : public views::View,

[msw, 2012/11/16 22:29:32]

nit: separate declaration and definition. 

[bartfab (slow), 2012/11/19 17:34:06]

Done, although I was just following the style used in this file already.

On 2012/11/16 22:29:32, msw wrote:

+                                 public views::LinkListener {
  public:
-  explicit UserView(ash::user::LoginStatus login)
-      : login_(login),
-        container_(NULL),
-        user_info_(NULL),
-        username_(NULL),
-        email_(NULL),
-        signout_(NULL) {
-    CHECK(login_ != ash::user::LOGGED_IN_NONE);
-
-    bool public_account = login_ == ash::user::LOGGED_IN_PUBLIC;
-    bool guest = login_ == ash::user::LOGGED_IN_GUEST;
-    bool locked = login_ == ash::user::LOGGED_IN_LOCKED;
+  PublicAccountUserDetails(SystemTrayItem* owner, int used_width)
+      : learn_more_(NULL),
+        font_(ResourceBundle::GetSharedInstance().GetFont(
+            ResourceBundle::BaseFont)) {
+    set_border(views::Border::CreateEmptyBorder(
+        kUserDetailsVerticalPadding,
+        kTrayPopupPaddingHorizontal - kTrayPopupPaddingBetweenItems,
+        kUserDetailsVerticalPadding, 0));
+
+    ash::SystemTrayDelegate* delegate =
+        ash::Shell::GetInstance()->tray_delegate();
+    // Retrieve the user's display name and wrap it with markers.
+    string16 display_name = delegate->GetUserDisplayName();
+    ReplaceChars(display_name, kDisplayNameMark, string16(), &display_name);
+    display_name.insert(0U, 1U, kDisplayNameMark[0]);
+    display_name.push_back(kDisplayNameMark[0]);
+    // Retrieve the domain managing the device and wrap it with markers.
+    string16 domain = UTF8ToUTF16(delegate->GetEnterpriseDomain());
+    ReplaceChars(domain, kDisplayNameMark, string16(), &domain);
+    base::i18n::WrapStringWithLTRFormatting(&domain);
+    // Retrieve the label text, inserting the display name and domain.
+    text_ = l10n_util::GetStringFUTF16(IDS_ASH_STATUS_TRAY_PUBLIC_LABEL,
+                                       display_name, domain);
+
+    learn_more_ = new views::Link(
+        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_PUBLIC_LEARN_MORE));
+    learn_more_->SetUnderline(false);
+    learn_more_->set_listener(this);
+    AddChildView(learn_more_);
+
+    CalculatePreferredSize(owner, used_width);
+  }
 
-    set_background(views::Background::CreateSolidBackground(
-        public_account ? kPublicAccountBackgroundColor : kBackgroundColor));
+  // Overridden from views::LinkListener.
+  virtual void LinkClicked(views::Link* source, int event_flags) OVERRIDE {
+    DCHECK_EQ(source, learn_more_);
+    ash::Shell::GetInstance()->tray_delegate()->ShowPublicAccountInfo();
+  }
 
-    container_ = new TrayPopupLabelButtonContainer;
-    container_->layout()->set_spread_blank_space(false);
-    AddChildView(container_);
+  // Overridden from views::View.
+  void Layout() OVERRIDE {
+    lines_.clear();
+    const gfx::Rect& contents_area = GetContentsBounds();
+    if (contents_area.IsEmpty())

[msw, 2012/11/16 22:29:32]

nit: does this happen?

[bartfab (slow), 2012/11/19 17:34:06]

Yes, it happens during bubble construction:

CreateBubbleWidget() ->
 Widget::Init() ->
  Widget::SetContentsView() ->
   RootView::SetContentsView() ->
    forces a Layout() call without any bounds set yet

On 2012/11/16 22:29:32, msw wrote:

+      return;
 
-    if (!guest)
-      AddUserInfo();
+    // Word-wrap the label text.
+    std::vector<string16> lines;
+    ui::ElideRectangleText(text_, font_, contents_area.width(),

[msw, 2012/11/16 22:29:32]

Does ElideRectangleText do the right thing if the UI directionality (say LTR for
the "is a shared account...") differs from the domain/user names (which could be
RTL)? Does your text styling also get this right? (I think that probably should
be okay, as it's all done in logical character indicies).

[bartfab (slow), 2012/11/19 17:34:06]

Yes, I verified that all conceivable LTR/RTL combinations work correctly,
including corner cases such as an RTL UI, an LTR user name with a line break
occurring inside the LTR bit.

On 2012/11/16 22:29:32, msw wrote:

+                           contents_area.height(), ui::TRUNCATE_LONG_WORDS,
+                           &lines);
+    // Loop through the lines, creating a renderer for each.
+    gfx::Point position = contents_area.origin();
+    std::queue<size_t> display_name_markers;
+    for (std::vector<string16>::const_iterator it = lines.begin();
+        it != lines.end(); ++it) {

[msw, 2012/11/16 22:29:32]

nit: indent one more space (just my preference to wrap within parens here,
etc.).

[bartfab (slow), 2012/11/19 17:34:06]

Yes, this is my preference as well. The missing space was an oversight. Thanks
for pointing it out.

On 2012/11/16 22:29:32, msw wrote:

+      gfx::RenderText* line = gfx::RenderText::CreateInstance();
+      line->SetDirectionalityMode(gfx::DIRECTIONALITY_FROM_UI);
+      line->SetText(*it);
+      const gfx::Size size(contents_area.width(),
+                           line->GetStringSize().height());
+      line->SetDisplayRect(gfx::Rect(position, size));
+      position.set_y(position.y() + size.height());
+
+      // Set the default text color for the line.
+      gfx::StyleRange default_style(line->default_style());
+      default_style.foreground = kPublicAccountUserCardTextColor;
+      line->set_default_style(default_style);
+      line->ApplyDefaultStyle();
+
+      // Locate any markers indicating the start and end of the user's display

[msw, 2012/11/16 22:29:32]

nit: you should be able to safely assume that there will only be a single name
in the block of text, so there will be two markers to find. Use a ui::Range to
track their locations per line, and a flag to persist the outside-/inside-
flag-pair state across lines. Then you should only need to apply a single style
range if a marker was found or the flag is on.

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+      // name.
+      size_t pos = 0;
+      while ((pos = it->find(kDisplayNameMark, pos)) != string16::npos) {
+        display_name_markers.push(pos);
+        ++pos;
+      }
+      // If the display name extends past the end of this line, synthesize a
+      // marker at the end of the line and another at the start of the next
+      // line.
+      if (display_name_markers.size() % 2) {
+        display_name_markers.push(it->size());
+        display_name_markers.push(0);

[stevenjb, 2012/11/16 21:13:52]

This is confusing. It looks like this means that we may end up with an odd
number of entries in the queue, so the 0 entry will persist for the next line? I
think this might be more clear if we set a bool here then pushed an entry at 0
into the queue at the beginning of this section if the bool is true. At minimum
the comment needs to be more clear.

[bartfab (slow), 2012/11/19 17:34:06]

I reworked the code as per Mike's suggestion. I am using a ui::Range now to
track the markers on a line and also to persist the
inside-display-name/outside-display-name state across lines.

On 2012/11/16 21:13:52, stevenjb (chromium) wrote:

+      }
+      // Loop through the intervals enclosed by markers and apply a custom text
+      // color to the display name.
+      if (display_name_markers.size() >= 2) {
+        gfx::StyleRange display_name_style(line->default_style());
+        display_name_style.foreground = kPublicAccountUserCardNameColor;
+        while (display_name_markers.size() >= 2) {

[msw, 2012/11/16 22:29:32]

there should only be one non-default style range per line, afaict.

[bartfab (slow), 2012/11/19 17:34:06]

Correct. I simplified the code, removing the loop.

On 2012/11/16 22:29:32, msw wrote:

+          display_name_style.range.set_start(display_name_markers.front());
+          display_name_markers.pop();
+          display_name_style.range.set_end(display_name_markers.front());
+          display_name_markers.pop();
+          line->ApplyStyleRange(display_name_style);
+        }
+      }
+      lines_.push_back(line);
+    }
 
-    // A user should not be able to modify logged in state when screen is
-    // locked.
-    if (!locked)
-      AddButtonContainer();
+    // Position link after the label text, separated by a space. If it does not

[msw, 2012/11/16 22:29:32]

This should be tested for RTL UI; I suspect that it may be incorrect.

[bartfab (slow), 2012/11/19 17:34:06]

I heavily tested this for RTL and did not encounter any bugs.

On 2012/11/16 22:29:32, msw wrote:

+    // fit onto the last line of the text, wrap the link onto its own line.
+    const gfx::Size last_line_size = lines_.back()->GetStringSize();
+    const int space_width = font_.GetStringWidth(ASCIIToUTF16(" "));
+    const gfx::Size link_size = learn_more_->GetPreferredSize();
+    if (contents_area.width() - last_line_size.width() >=
+        space_width + link_size.width()) {
+      position.set_x(position.x() + last_line_size.width() + space_width);
+      position.set_y(position.y() - last_line_size.height());
+    }
+    position.set_y(position.y() - learn_more_->GetInsets().top());
+    gfx::Rect learn_more_bounds(position, link_size);
+    learn_more_bounds.Intersect(contents_area);
+    learn_more_->SetBoundsRect(learn_more_bounds);
   }
 
-  virtual ~UserView() {}
-
-  // Create container for buttons.
-  void AddButtonContainer() {
-    TrayPopupLabelButton* button = new TrayPopupLabelButton(this,
-        ash::user::GetLocalizedSignOutStringForStatus(login_, true));
-    container_->AddLabelButton(button);
-    signout_ = button;
+  virtual gfx::Size GetPreferredSize() OVERRIDE {
+    return preferred_size_;
   }
 
  private:
-  void AddUserInfo() {
-    user_info_ = new views::View;
-    user_info_->SetLayoutManager(new views::BoxLayout(
-        views::BoxLayout::kHorizontal, kTrayPopupPaddingHorizontal,
-        kUserInfoVerticalPadding, kTrayPopupPaddingBetweenItems));
-    container_->AddChildView(user_info_);
-
-    if (login_ == ash::user::LOGGED_IN_KIOSK) {
-      views::Label* label = new views::Label;
-      ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-      label->SetText(
-          bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_KIOSK_LABEL));
-      label->set_border(views::Border::CreateEmptyBorder(
-            0, 4, 0, 1));
-      label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-      user_info_->AddChildView(label);
-      return;
-    }
+  virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE {
+    for (ScopedVector<gfx::RenderText>::const_iterator it = lines_.begin();
+         it != lines_.end(); ++it)
+      (*it)->Draw(canvas);
+    views::View::OnPaint(canvas);
+  }
 
-    RoundedImageView* image = new RoundedImageView(kProfileRoundedCornerRadius);
-    image->SetImage(ash::Shell::GetInstance()->tray_delegate()->GetUserImage(),
-        gfx::Size(kUserIconSize, kUserIconSize));
-    user_info_->AddChildView(image);
+  // Calculate a preferred size that ensures the label text and the following
+ // link do not wrap over more than three lines in total for esthetic reasons.

[stevenjb, 2012/11/16 21:13:52]

nit: align, aesthetic (more common)

[msw, 2012/11/16 22:29:32]

What's done for arbitrarily long doman/user names? You can't expect them to
always fit on three lines if there's a hard maximum width (which we should have,
as Steven suggested).

[bartfab (slow), 2012/11/19 17:34:06]

Done.

[bartfab (slow), 2012/11/19 17:34:06]

I added a maximum width. The text can now wrap to > 3 lines in extreme cases
(very, very, very long user name) and/or may be elided (very, very, very long
domain name).

On 2012/11/16 22:29:32, msw wrote:

+  void CalculatePreferredSize(SystemTrayItem* owner, int used_width) {
+    const gfx::Size link_size = learn_more_->GetPreferredSize();
+    const int space_width = font_.GetStringWidth(ASCIIToUTF16(" "));
+    int min_width = std::max(1, link_size.width());

[msw, 2012/11/16 22:29:32]

nit: it should be safe to assume the link width is larger.

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+    int max_width = font_.GetStringWidth(text_) +
+                    space_width + link_size.width();

[stevenjb, 2012/11/16 21:13:52]

We should probably limit max width here to a reasonable percentage of the screen
width (or a reasonable fixed size like 400px; on a large display a bubble half
its width would look odd. The SystemBubble should probably define the maximum
width. In practice this shouldn't happen of course...

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+    // Do a binary search for the minimum width that ensures no more than three
+    // lines are needed. The lower bound is the width of the link (as no
+    // wrapping is permitted inside the link). The upper bound is the width of
+    // the label text and link when put on a single line.
+    while (min_width < max_width) {

[msw, 2012/11/16 22:29:32]

I fear this will be the performance bottleneck for showing the system tray menu,
although I lack data to substantiate that concern and don't have great
alternative suggestions beyond picking a reasonable width and letting the height
increase while potentially eliding/truncating obscenely long user/domain names.

[bartfab (slow), 2012/11/19 17:34:06]

Since this is a binary search, it only takes a handful of iterations, even if
the search range happens to be very long. I very much want to use this form of
wrapping as a fixed width produces ugly results for many cases I tested,
including the simplest thing that came to my mind first, "Internet kiosk" and
"google.com".

Note also that this entire code runs *only* in public account mode. Regular
users will not be affected by it in any way.

On 2012/11/16 22:29:32, msw wrote:

+      const int width = (min_width + max_width) / 2;
+      std::vector<string16> lines;

[msw, 2012/11/16 22:29:32]

nit: declare |lines| outside this loop and use it below too (line 297).

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+      const bool truncated = ui::ElideRectangleText(
+          text_, font_, width, INT_MAX, ui::TRUNCATE_LONG_WORDS, &lines);
+      const int line_count = lines.size() +
+          (width - font_.GetStringWidth(lines.back()) <=

[stevenjb, 2012/11/16 21:13:52]

Avoid this (expensive) calculation if truncated?

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+           space_width + link_size.width());

[msw, 2012/11/16 22:29:32]

nit: make this a ternary * ? 0 : 1; to explicitly specify the value added.

[bartfab (slow), 2012/11/19 17:34:06]

I reordered the code to avoid this implicit bool-to-int conversion and avoid any
unnecessary computations.

On 2012/11/16 22:29:32, msw wrote:

+      if (truncated || line_count > 3)
+        min_width = width + 1;
+      else
+        max_width = width;
+    }
 
-    views::View* user = new views::View;
-    user->SetLayoutManager(new views::BoxLayout(
-        views::BoxLayout::kVertical, 0, 5, 0));
-    ash::SystemTrayDelegate* tray =
-        ash::Shell::GetInstance()->tray_delegate();
-    username_ = new views::Label(tray->GetUserDisplayName());
-    username_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    user->AddChildView(username_);
+    // If the space available for the user details in the system tray bubble
+    // is larger than the minimum width calculated above, use that width
+    // instead, potentially wrapping onto less than three lines.
+    const gfx::Insets insets = GetInsets();
+    views::TrayBubbleView* bubble_view =
+        owner->system_tray()->GetSystemBubble()->bubble_view();
+    const int available_width =

[msw, 2012/11/16 22:29:32]

The initial min_width value above (line 265) should be the maximum of this
allocated width and the width of "Learn More".

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+        bubble_view->GetPreferredSize().width() - (used_width + insets.width());
+    min_width = std::max(min_width, available_width);
+
+    // Calculate the corresponding height and set the preferred size.
+    std::vector<string16> lines;
+    ui::ElideRectangleText(
+        text_, font_, min_width, INT_MAX, ui::TRUNCATE_LONG_WORDS, &lines);
+    const int line_count = lines.size() +

[msw, 2012/11/16 22:29:32]

ditto nit: make this a ternary * ? 0 : 1; to explicitly specify the value added.

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+        (min_width - font_.GetStringWidth(lines.back()) <=
+         space_width + link_size.width());
+    const int line_height = font_.GetHeight();
+    const int link_extra_height = std::max(
+        link_size.height() - learn_more_->GetInsets().top() - line_height, 0);
+    preferred_size_ = gfx::Size(
+        min_width + insets.width(),
+        line_count * line_height + link_extra_height + insets.height());
+
+    // If the system tray bubble currently is too narrow to provide the

[msw, 2012/11/16 22:29:32]

nit: "is currently"

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+    // calculated minimum width, resize the bubble.
+    if (min_width > available_width)

[msw, 2012/11/16 22:29:32]

nit: can't you just call bubble_view->SizeToContents()?

[bartfab (slow), 2012/11/19 17:34:06]

Unfortunately, no:

SizeToContents() takes all child Views' preferred sizes into account. I would
have to sabotage all other childrens' GetPreferredSize() to ensure that they
never request a width larger than the current bubble width (because the bubble
is supposed to expand only for the user card and only in public account mode).

On 2012/11/16 22:29:32, msw wrote:

+      bubble_view->SetWidth(preferred_size_.width() + used_width);
+  }
 
-    email_ = new views::Label(UTF8ToUTF16(tray->GetUserEmail()));
-    email_->SetFont(username_->font().DeriveFont(-1));
-    email_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    email_->SetEnabled(false);
-    user->AddChildView(email_);
+  string16 text_;
+  views::Link* learn_more_;
+  gfx::Font font_;
+  gfx::Size preferred_size_;
+  ScopedVector<gfx::RenderText> lines_;
 
-    user_info_->AddChildView(user);
-  }
+  DISALLOW_COPY_AND_ASSIGN(PublicAccountUserDetails);
+};
 
-  // Overridden from views::ButtonListener.
-  virtual void ButtonPressed(views::Button* sender,
-                             const ui::Event& event) OVERRIDE {
-    CHECK(sender == signout_);
-    ash::SystemTrayDelegate* tray = ash::Shell::GetInstance()->tray_delegate();
-    tray->SignOut();
+class UserView : public views::View,

[msw, 2012/11/16 22:29:32]

nit: separate declaration and definition. 

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+                 public views::ButtonListener {
+ public:
+  explicit UserView(SystemTrayItem* owner,
+                    ash::user::LoginStatus login)
+      : user_card_(NULL),
+        logout_button_(NULL) {
+    CHECK(login != ash::user::LOGGED_IN_NONE);
+    set_background(views::Background::CreateSolidBackground(
+        login == ash::user::LOGGED_IN_PUBLIC ? kPublicAccountBackgroundColor :
+                                               kBackgroundColor));
+    SetLayoutManager(new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0,
+                                          kTrayPopupPaddingBetweenItems));
+    AddLogoutButton(login);
+    AddUserCard(owner, login);
   }
 
   // Overridden from views::View.
   virtual gfx::Size GetPreferredSize() OVERRIDE {
-    gfx::Size size;
-    if (user_info_)
-      size = user_info_->GetPreferredSize();
-    if (signout_) {
-      gfx::Size signout_size = signout_->GetPreferredSize();
-      // Make sure the user default view item at least as tall as the other
-      // tray popup items.
-      if (size.height() == 0)
-        size.set_height(kTrayPopupItemHeight);
-      size.set_height(std::max(size.height(), signout_size.height()));
-      size.set_width(size.width() + signout_size.width() +
-          kTrayPopupPaddingHorizontal * 2 + kTrayPopupPaddingBetweenItems);
+    gfx::Size size = views::View::GetPreferredSize();
+    if (!user_card_) {
+      // Make sure the default user default view item is at least as tall as the
+      // other items.
+      size.set_height(std::max(size.height(),
+                               kTrayPopupItemHeight + GetInsets().height()));
     }
     return size;
   }
 
   virtual void Layout() OVERRIDE {
-    views::View::Layout();
-    if (bounds().IsEmpty())
+    gfx::Rect contents_area(GetContentsBounds());
+    if (user_card_ && logout_button_) {
+      // Give the logout button the space it requests.
+      gfx::Rect logout_area = contents_area;
+      logout_area.ClampToCenteredSize(logout_button_->GetPreferredSize());
+      logout_area.set_x(contents_area.right() - logout_area.width());
+      logout_button_->SetBoundsRect(logout_area);
+
+      // Give the remaining space to the user card.
+      gfx::Rect user_card_area = contents_area;
+      user_card_area.set_width(contents_area.width() -
+          (logout_area.width() + kTrayPopupPaddingBetweenItems));
+      user_card_->SetBoundsRect(user_card_area);
+    } else if (user_card_) {
+      user_card_->SetBoundsRect(contents_area);
+    } else if (logout_button_) {
+      logout_button_->SetBoundsRect(contents_area);
+    }
+  }
+
+ private:
+  void AddLogoutButton(ash::user::LoginStatus login) {
+    // The logout button must be added before the user card.
+    DCHECK(!user_card_);
+
+    // A user should not be able to modify logged-in state when screen is
+    // locked.
+    if (login == ash::user::LOGGED_IN_LOCKED)
       return;
 
-    container_->SetBoundsRect(gfx::Rect(size()));
-    if (signout_ && user_info_) {
-      gfx::Rect signout_bounds(bounds());
-      signout_bounds.ClampToCenteredSize(signout_->GetPreferredSize());
-      signout_bounds.set_x(width() - signout_bounds.width() -
-          kTrayPopupPaddingHorizontal);
-      signout_->SetBoundsRect(signout_bounds);
-
-      gfx::Rect usercard_bounds(user_info_->GetPreferredSize());
-      usercard_bounds.set_width(signout_bounds.x());
-      user_info_->SetBoundsRect(usercard_bounds);
-    } else if (signout_) {
-      signout_->SetBoundsRect(gfx::Rect(size()));
-    } else if (user_info_) {
-      user_info_->SetBoundsRect(gfx::Rect(size()));
+    TrayPopupLabelButton* logout_button = new TrayPopupLabelButton(
+        this, ash::user::GetLocalizedSignOutStringForStatus(login, true));
+    // In public account mode, the logout button has custom padding.
+    if (login == ash::user::LOGGED_IN_PUBLIC) {
+      gfx::Insets extra_padding = logout_button->GetExtraPadding();
+      extra_padding.Set(kTrayPopupPublicAccountLogoutButtonExtraPaddingVertical,
+                        extra_padding.left(),
+                        kTrayPopupPublicAccountLogoutButtonExtraPaddingVertical,
+                        extra_padding.right());
+      logout_button->SetExtraPadding(extra_padding);
     }
+    logout_button_ = logout_button;
+    AddChildView(logout_button);
   }
 
-  user::LoginStatus login_;
+  void AddUserCard(SystemTrayItem* owner, ash::user::LoginStatus login) {
+    if (login == ash::user::LOGGED_IN_GUEST)
+      return;
+
+    set_border(views::Border::CreateEmptyBorder(
+        0, kTrayPopupPaddingHorizontal, 0, kTrayPopupPaddingHorizontal));
+
+    // Add a vertical separator between the logout button and the user card.
+    if (logout_button_) {
+      int border_width = 1;
+      SkColor border_color = kButtonStrokeColor;
+      // In public account mode, the vertical separator has a custom style.
+      if (login == ash::user::LOGGED_IN_PUBLIC) {
+        border_width = 2;
+        border_color = kPublicAccountLogoutButtonStrokeColor;
+      }
+      const bool rtl = base::i18n::IsRTL();
+      logout_button_->set_border(views::Border::CreateSolidSidedBorder(
+          0, rtl ? 0 : border_width, 0, rtl ? border_width : 0, border_color));
+    }
+
+    user_card_ = new views::View();
+    user_card_->SetLayoutManager(new views::BoxLayout(
+        views::BoxLayout::kHorizontal, 0, kUserCardVerticalPadding,
+        kTrayPopupPaddingBetweenItems));
+    AddChildViewAt(user_card_, 0);
 
-  TrayPopupLabelButtonContainer* container_;
-  views::View* user_info_;
-  views::Label* username_;
-  views::Label* email_;
+    if (login == ash::user::LOGGED_IN_KIOSK) {
+      views::Label* details = new views::Label;
+      ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+      details->SetText(
+          bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_KIOSK_LABEL));
+      details->set_border(views::Border::CreateEmptyBorder(0, 4, 0, 1));
+      details->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+      user_card_->AddChildView(details);
+      return;
+    }
+
+    RoundedImageView* avatar =
+        new RoundedImageView(kProfileRoundedCornerRadius);
+    avatar->SetImage(ash::Shell::GetInstance()->tray_delegate()->GetUserImage(),
+                     gfx::Size(kUserIconSize, kUserIconSize));
+    user_card_->AddChildView(avatar);
+
+    if (login == ash::user::LOGGED_IN_PUBLIC) {
+      user_card_->AddChildView(new PublicAccountUserDetails(
+          owner, GetPreferredSize().width() + kTrayPopupPaddingBetweenItems));
+      return;
+    }
+
+    ash::SystemTrayDelegate* delegate =
+        ash::Shell::GetInstance()->tray_delegate();
+    views::View* details = new views::View;
+    details->SetLayoutManager(new views::BoxLayout(
+        views::BoxLayout::kVertical, 0, kUserDetailsVerticalPadding, 0));
+    views::Label* username = new views::Label(delegate->GetUserDisplayName());
+    username->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    details->AddChildView(username);
+
+    views::Label* email =

[msw, 2012/11/16 22:29:32]

nit: you might want to SetElideBehavior for this label to ELIDE_AS_EMAIL.

[bartfab (slow), 2012/11/19 17:34:06]

Done.

+        new views::Label(UTF8ToUTF16(delegate->GetUserEmail()));
+    email->SetFont(username->font().DeriveFont(-1));
+    email->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    email->SetEnabled(false);
+    details->AddChildView(email);
+    user_card_->AddChildView(details);
+  }
+
+  // Overridden from views::ButtonListener.
+  virtual void ButtonPressed(views::Button* sender,
+                             const ui::Event& event) OVERRIDE {
+    CHECK(sender == logout_button_);
+    ash::Shell::GetInstance()->tray_delegate()->SignOut();
+  }
 
-  views::Button* signout_;
+  views::View* user_card_;
+  views::View* logout_button_;
 
   DISALLOW_COPY_AND_ASSIGN(UserView);
 };
@@ -278,7 +511,7 @@ views::View* TrayUser::CreateDefaultView(user::LoginStatus status) {
     return NULL;
 
   CHECK(user_ == NULL);
-  user_ = new tray::UserView(status);
+  user_ = new tray::UserView(this, status);
   return user_;
 }