Fixing tab-ability of multi user items in system tray menu.

BUG=364652
TEST=visual (Tab in, tab out, Tab in, activate, add user, tabbable, ..)

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

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

4 files changed, 131 insertions, 20 deletions

diff --git a/ash/system/user/button_from_view.cc b/ash/system/user/button_from_view.cc
index 64b641d..34f5099 100644
--- a/ash/system/user/button_from_view.cc
+++ b/ash/system/user/button_from_view.cc
@@ -4,7 +4,9 @@
 
 #include "ash/system/user/button_from_view.h"
 
+#include "ash/ash_constants.h"
 #include "ash/system/tray/tray_constants.h"
+#include "ui/gfx/canvas.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/layout/box_layout.h"
@@ -22,17 +24,22 @@ namespace tray {
 
 ButtonFromView::ButtonFromView(views::View* content,
                                views::ButtonListener* listener,
-                               bool highlight_on_hover)
+                               bool highlight_on_hover,
+                               const gfx::Insets& tab_frame_inset)
     : CustomButton(listener),
       content_(content),
       highlight_on_hover_(highlight_on_hover),
       button_hovered_(false),
-      show_border_(false) {
+      show_border_(false),
+      tab_frame_inset_(tab_frame_inset) {
   set_notify_enter_exit_on_child(true);
   SetLayoutManager(
-      new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0));
+      new views::BoxLayout(views::BoxLayout::kHorizontal, 1, 1, 0));
   AddChildView(content_);
   ShowActive();
+  // Only make it focusable when we are active/interested in clicks.
+  if (listener)
+    SetFocusable(true);
 }
 
 ButtonFromView::~ButtonFromView() {}
@@ -52,6 +59,27 @@ void ButtonFromView::OnMouseExited(const ui::MouseEvent& event) {
   ShowActive();
 }
 
+void ButtonFromView::OnPaint(gfx::Canvas* canvas) {
+  View::OnPaint(canvas);
+  if (HasFocus()) {
+    gfx::Rect rect(GetLocalBounds());
+    rect.Inset(tab_frame_inset_);
+    canvas->DrawSolidFocusRect(rect, kFocusBorderColor);
+  }
+}
+
+void ButtonFromView::OnFocus() {
+  View::OnFocus();
+  // Adding focus frame.
+  SchedulePaint();
+}
+
+void ButtonFromView::OnBlur() {
+  View::OnBlur();
+  // Removing focus frame.
+  SchedulePaint();
+}
+
 void ButtonFromView::ShowActive() {
   bool border_visible =
       (button_hovered_ && highlight_on_hover_) || show_border_;
diff --git a/ash/system/user/button_from_view.h b/ash/system/user/button_from_view.h
index 1d391a6..23a632b 100644
--- a/ash/system/user/button_from_view.h
+++ b/ash/system/user/button_from_view.h
@@ -6,7 +6,7 @@
 #define ASH_SYSTEM_USER_BUTTON_FROM_VIEW_H_
 
 #include "base/macros.h"
-
+#include "ui/gfx/insets.h"
 #include "ui/views/controls/button/custom_button.h"
 
 namespace ash {
@@ -15,9 +15,16 @@ namespace tray {
 // This view is used to wrap it's content and transform it into button.
 class ButtonFromView : public views::CustomButton {
  public:
+  // The |content| is the content which is shown within the button. The
+  // |button_listener| will be informed - if provided - when a button was
+  // pressed. If |highlight_on_hover| is set to true, the button will be
+  // highlighted upon hover and show the accessibility caret.
+  // The |tab_frame_inset| will be used to inset the blue tab frame inside the
+  // button.
   ButtonFromView(views::View* content,
                  views::ButtonListener* listener,
-                 bool highlight_on_hover);
+                 bool highlight_on_hover,
+                 const gfx::Insets& tab_frame_inset);
   virtual ~ButtonFromView();
 
   // Called when the border should remain even in the non highlighted state.
@@ -26,6 +33,9 @@ class ButtonFromView : public views::CustomButton {
   // Overridden from views::View
   virtual void OnMouseEntered(const ui::MouseEvent& event) OVERRIDE;
   virtual void OnMouseExited(const ui::MouseEvent& event) OVERRIDE;
+  virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
+  virtual void OnFocus() OVERRIDE;
+  virtual void OnBlur() OVERRIDE;
 
   // Check if the item is hovered.
   bool is_hovered_for_test() { return button_hovered_; }
@@ -46,6 +56,9 @@ class ButtonFromView : public views::CustomButton {
   // True if the border should be always visible.
   bool show_border_;
 
+  // The insets which get used for the drawn accessibility (tab) frame.
+  gfx::Insets tab_frame_inset_;
+
   DISALLOW_COPY_AND_ASSIGN(ButtonFromView);
 };
 
diff --git a/ash/system/user/user_view.cc b/ash/system/user/user_view.cc
index ddfae38..27fc5fd 100644
--- a/ash/system/user/user_view.cc
+++ b/ash/system/user/user_view.cc
@@ -230,12 +230,12 @@ UserView::UserView(SystemTrayItem* owner,
   AddUserCard(login);
 }
 
-UserView::~UserView() {}
+UserView::~UserView() {
+  RemoveAddUserMenuOption();
+}
 
 void UserView::MouseMovedOutOfHost() {
-  popup_message_.reset();
-  mouse_watcher_.reset();
-  add_menu_option_.reset();
+  RemoveAddUserMenuOption();
 }
 
 TrayUser::TestState UserView::GetStateForTest() const {
@@ -321,6 +321,7 @@ void UserView::ButtonPressed(views::Button* sender, const ui::Event& event) {
   if (sender == logout_button_) {
     Shell::GetInstance()->metrics()->RecordUserMetricsAction(
         ash::UMA_STATUS_AREA_SIGN_OUT);
+    RemoveAddUserMenuOption();
     Shell::GetInstance()->system_tray_delegate()->SignOut();
   } else if (sender == user_card_view_ && !multiprofile_index_ &&
              IsMultiAccountSupportedAndUserActive()) {
@@ -330,6 +331,7 @@ void UserView::ButtonPressed(views::Button* sender, const ui::Event& event) {
     if (!multiprofile_index_) {
       ToggleAddUserMenuOption();
     } else {
+      RemoveAddUserMenuOption();
       SwitchUser(multiprofile_index_);
       // Since the user list is about to change the system menu should get
       // closed.
@@ -337,6 +339,7 @@ void UserView::ButtonPressed(views::Button* sender, const ui::Event& event) {
     }
   } else if (add_menu_option_.get() &&
              sender == add_menu_option_->GetContentsView()) {
+    RemoveAddUserMenuOption();
     // Let the user add another account to the session.
     MultiProfileUMA::RecordSigninUser(MultiProfileUMA::SIGNIN_USER_BY_TRAY);
     Shell::GetInstance()->system_tray_delegate()->ShowUserLogin();
@@ -346,6 +349,15 @@ void UserView::ButtonPressed(views::Button* sender, const ui::Event& event) {
   }
 }
 
+void UserView::OnWillChangeFocus(View* focused_before, View* focused_now) {
+  if (focused_now)
+    RemoveAddUserMenuOption();
+}
+
+void UserView::OnDidChangeFocus(View* focused_before, View* focused_now) {
+  // Nothing to do here.
+}
+
 void UserView::AddLogoutButton(user::LoginStatus login) {
   const base::string16 title =
       user::GetLocalizedSignOutStringForStatus(login, true);
@@ -404,15 +416,40 @@ void UserView::AddUserCard(user::LoginStatus login) {
       user_card_view_->SetBorder(views::Border::CreateEmptyBorder(
           0, kTrayUserTileHoverBorderInset, 0, 0));
     }
+    gfx::Insets insets = gfx::Insets(1, 1, 1, 1);
+    views::View* contents_view = user_card_view_;
+    ButtonFromView* button = NULL;
     if (!for_detailed_view_) {
-      user_card_view_ =
-          new ButtonFromView(user_card_view_, this, !multiprofile_index_);
+      if (multiprofile_index_) {
+        // Since the activation border needs to be drawn around the tile, we
+        // have to put the tile into another view which fills the menu panel,
+        // but keeping the offsets of the content.
+        contents_view = new views::View();
+        contents_view->SetBorder(views::Border::CreateEmptyBorder(
+            kTrayPopupUserCardVerticalPadding,
+            kTrayPopupPaddingHorizontal,
+            kTrayPopupUserCardVerticalPadding,
+            kTrayPopupPaddingHorizontal));
+        contents_view->SetLayoutManager(new views::FillLayout());
+        SetBorder(views::Border::CreateEmptyBorder(0, 0, 0, 0));
+        contents_view->AddChildView(user_card_view_);
+        insets = gfx::Insets(1, 1, 1, 3);
+      }
+      button = new ButtonFromView(contents_view,
+                                  this,
+                                  !multiprofile_index_,
+                                  insets);
+      // TODO(skuhne): For accessibility we need to call |SetAccessibleName|
+      // with a useful name (string freeze for M37 has passed).
     } else {
       // We want user card for detailed view to have exactly the same look
       // as user card for default view. That's why we wrap it in a button
-      // without click listener and special hover behaviour.
-      user_card_view_ = new ButtonFromView(user_card_view_, NULL, false);
+      // without click listener and special hover behavior.
+      button = new ButtonFromView(contents_view, NULL, false, insets);
     }
+    // A click on the button should not trigger a focus change.
+    button->set_request_focus_on_press(false);
+    user_card_view_ = button;
     is_user_card_button_ = true;
   }
   AddChildViewAt(user_card_view_, 0);
@@ -424,9 +461,7 @@ void UserView::AddUserCard(user::LoginStatus login) {
 
 void UserView::ToggleAddUserMenuOption() {
   if (add_menu_option_.get()) {
-    popup_message_.reset();
-    mouse_watcher_.reset();
-    add_menu_option_.reset();
+    RemoveAddUserMenuOption();
     return;
   }
 
@@ -462,9 +497,14 @@ void UserView::ToggleAddUserMenuOption() {
       Shell::GetInstance()->session_state_delegate();
   add_user_disabled_ = delegate->NumberOfLoggedInUsers() >=
                        delegate->GetMaximumNumberOfLoggedInUsers();
-  ButtonFromView* button = add_user_disabled_
-                               ? new ButtonFromView(add_user_view, NULL, false)
-                               : new ButtonFromView(add_user_view, this, true);
+  ButtonFromView* button = new ButtonFromView(
+      add_user_view,
+      add_user_disabled_ ? NULL : this,
+      !add_user_disabled_,
+      gfx::Insets(1, 1, 1, 1));
+  button->set_request_focus_on_press(false);
+  button->SetAccessibleName(
+      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SIGN_IN_ANOTHER_ACCOUNT));
   button->ForceBorderVisible(true);
   add_menu_option_->SetContentsView(button);
 
@@ -478,6 +518,12 @@ void UserView::ToggleAddUserMenuOption() {
         views::BubbleBorder::TOP_LEFT,
         gfx::Size(parent()->bounds().width() - kPopupMessageOffset, 0),
         2 * kPopupMessageOffset));
+  } else {
+    // We activate the entry automatically if invoked with focus.
+    if (user_card_view_->HasFocus()) {
+      button->GetFocusManager()->SetFocusedView(button);
+      user_card_view_->GetFocusManager()->SetFocusedView(button);
+    }
   }
   // Find the screen area which encloses both elements and sets then a mouse
   // watcher which will close the "menu".
@@ -486,6 +532,19 @@ void UserView::ToggleAddUserMenuOption() {
   mouse_watcher_.reset(
       new views::MouseWatcher(new UserViewMouseWatcherHost(area), this));
   mouse_watcher_->Start();
+  // Install a listener to focus changes so that we can remove the card when
+  // the focus gets changed.
+  user_card_view_->GetFocusManager()->AddFocusChangeListener(this);
+}
+
+void UserView::RemoveAddUserMenuOption() {
+  if (!add_menu_option_.get())
+    return;
+  user_card_view_->GetFocusManager()->RemoveFocusChangeListener(this);
+  user_card_view_->GetFocusManager()->ClearFocus();
+  popup_message_.reset();
+  mouse_watcher_.reset();
+  add_menu_option_.reset();
 }
 
 }  // namespace tray
diff --git a/ash/system/user/user_view.h b/ash/system/user/user_view.h
index e1dc65a..303d8b1 100644
--- a/ash/system/user/user_view.h
+++ b/ash/system/user/user_view.h
@@ -11,6 +11,7 @@
 #include "ash/system/user/tray_user.h"
 #include "base/macros.h"
 #include "ui/views/controls/button/button.h"
+#include "ui/views/focus/focus_manager.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/mouse_watcher.h"
 #include "ui/views/view.h"
@@ -30,7 +31,8 @@ namespace tray {
 // The view of a user item in system tray bubble.
 class UserView : public views::View,
                  public views::ButtonListener,
-                 public views::MouseWatcherListener {
+                 public views::MouseWatcherListener,
+                 public views::FocusChangeListener {
  public:
   UserView(SystemTrayItem* owner,
            ash::user::LoginStatus login,
@@ -54,6 +56,12 @@ class UserView : public views::View,
   virtual void ButtonPressed(views::Button* sender,
                              const ui::Event& event) OVERRIDE;
 
+  // Overridden from views::FocusChangeListener:
+  virtual void OnWillChangeFocus(View* focused_before,
+                                 View* focused_now) OVERRIDE;
+  virtual void OnDidChangeFocus(View* focused_before,
+                                View* focused_now) OVERRIDE;
+
   void AddLogoutButton(user::LoginStatus login);
   void AddUserCard(user::LoginStatus login);
 
@@ -61,6 +69,9 @@ class UserView : public views::View,
   // cannot actively click on the item.
   void ToggleAddUserMenuOption();
 
+  // Removes the add user menu option.
+  void RemoveAddUserMenuOption();
+
   MultiProfileIndex multiprofile_index_;
   // The view of the user card.
   views::View* user_card_view_;