Signin screen and locked screen status area focus advancing

This CL is aimed at solving the bug described in comment 7 in crbug.com/468969. The original method only considered the system tray alone.

After the fix:

On signin screen, tabing focus order is:
three dots -> system tray -> (virtual keyboard) -> user login
shift-tabing focus order is:
user login -> (virtual keyboard) -> system tray -> three dots

On locked screen, tabing focus order is:
signout -> system tray -> notification tray -> (virtual keyboard) -> user login
reverse tabing focus order is:
user login -> (virtual keyboard) -> notification tray -> system tray -> signout

BUG=468969
TEST=device test, works good on the above description.

Review-Url: https://codereview.chromium.org/2295843006
Cr-Commit-Position: refs/heads/master@{#419234}

ash/common/system/status_area_widget_delegate.cc

@@ -69,7 +69,25 @@ void StatusAreaWidgetDelegate::SetFocusCyclerForTesting(
 }
 
 views::View* StatusAreaWidgetDelegate::GetDefaultFocusableChild() {
-  return child_at(0);
+  return default_last_focusable_child_ ? GetLastFocusableChild()
+                                       : GetFirstFocusableChild();
+}
+
+views::FocusSearch* StatusAreaWidgetDelegate::GetFocusSearch() {
+  return custom_focus_traversable_ ? custom_focus_traversable_->GetFocusSearch()
+                                   : AccessiblePaneView::GetFocusSearch();
+}
+
+views::FocusTraversable* StatusAreaWidgetDelegate::GetFocusTraversableParent() {
+  return custom_focus_traversable_
+             ? custom_focus_traversable_->GetFocusTraversableParent()
+             : AccessiblePaneView::GetFocusTraversableParent();
+}
+
+views::View* StatusAreaWidgetDelegate::GetFocusTraversableParentView() {
+  return custom_focus_traversable_
+             ? custom_focus_traversable_->GetFocusTraversableParentView()
+             : AccessiblePaneView::GetFocusTraversableParentView();
 }
 
 views::Widget* StatusAreaWidgetDelegate::GetWidget() {

ash/common/system/status_area_widget_delegate.h

@@ -33,8 +33,20 @@ class ASH_EXPORT StatusAreaWidgetDelegate : public views::AccessiblePaneView,
 
   void set_alignment(ShelfAlignment alignment) { alignment_ = alignment; }
 
+  void set_custom_focus_traversable(
+      views::FocusTraversable* custom_focus_traversable) {
+    custom_focus_traversable_ = custom_focus_traversable;
+  }
+
+  void set_default_last_focusable_child(bool default_last_focusable_child) {
+    default_last_focusable_child_ = default_last_focusable_child;
+  }
+
   // Overridden from views::AccessiblePaneView.
   View* GetDefaultFocusableChild() override;
+  views::FocusSearch* GetFocusSearch() override;
+  views::FocusTraversable* GetFocusTraversableParent() override;
+  views::View* GetFocusTraversableParentView() override;
 
   // Overridden from views::View:
   views::Widget* GetWidget() override;
@@ -60,6 +72,12 @@ class ASH_EXPORT StatusAreaWidgetDelegate : public views::AccessiblePaneView,
   // screen.
   void SetBorderOnChild(views::View* child, bool extend_border_to_edge);
 
+  views::FocusTraversable* custom_focus_traversable_ = nullptr;
+
+  // When true, the default focus of the status area widget is the last
+  // focusable child.
+  bool default_last_focusable_child_ = false;
+
   const FocusCycler* focus_cycler_for_testing_;
 
   // TODO(jamescook): Get this from WmShelf.

chrome/browser/chromeos/login/ui/webui_login_view.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/login/ui/webui_login_view.h"
 
 #include "ash/common/focus_cycler.h"
+#include "ash/common/system/status_area_widget_delegate.h"
 #include "ash/common/system/tray/system_tray.h"
 #include "ash/common/wm_shell.h"
 #include "ash/shell.h"
@@ -53,6 +54,8 @@
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/views/controls/webview/webview.h"
+#include "ui/views/focus/focus_manager.h"
+#include "ui/views/focus/focus_search.h"
 #include "ui/views/widget/widget.h"
 
 using content::NativeWebKeyboardEvent;
@@ -96,6 +99,15 @@ class ScopedArrowKeyTraversal {
   DISALLOW_COPY_AND_ASSIGN(ScopedArrowKeyTraversal);
 };
 
+// A helper method returns status area widget delegate if exists,
+// otherwise nullptr.
+ash::StatusAreaWidgetDelegate* GetStatusAreaWidgetDelegate() {
+  ash::SystemTray* tray = ash::Shell::GetInstance()->GetPrimarySystemTray();
+  return tray ? static_cast<ash::StatusAreaWidgetDelegate*>(
+                    tray->GetWidget()->GetContentsView())
+              : nullptr;
+}
+
 }  // namespace
 
 namespace chromeos {
@@ -104,6 +116,59 @@ namespace chromeos {
 const char WebUILoginView::kViewClassName[] =
     "browser/chromeos/login/WebUILoginView";
 
+// WebUILoginView::CycleFocusTraversable ---------------------------------------
+class WebUILoginView::CycleFocusTraversable : public views::FocusTraversable {
+ public:
+  explicit CycleFocusTraversable(WebUILoginView* webui_login_view)
+      : cycle_focus_search_(webui_login_view, true, false) {}
+  ~CycleFocusTraversable() override {}
+
+  // views::FocusTraversable
+  views::FocusSearch* GetFocusSearch() override { return &cycle_focus_search_; }
+
+  views::FocusTraversable* GetFocusTraversableParent() override {
+    return nullptr;
+  }
+
+  views::View* GetFocusTraversableParentView() override { return nullptr; }
+
+ private:
+  views::FocusSearch cycle_focus_search_;
+
+  DISALLOW_COPY_AND_ASSIGN(CycleFocusTraversable);
+};
+
+// WebUILoginView::StatusAreaFocusTraversable ----------------------------------
+class WebUILoginView::StatusAreaFocusTraversable
+    : public views::FocusTraversable {
+ public:
+  StatusAreaFocusTraversable(
+      ash::StatusAreaWidgetDelegate* status_area_widget_delegate,
+      WebUILoginView* webui_login_view)
+      : webui_login_view_(webui_login_view),
+        status_area_focus_search_(status_area_widget_delegate, false, false) {}
+  ~StatusAreaFocusTraversable() override {}
+
+  // views::FocusTraversable
+  views::FocusSearch* GetFocusSearch() override {
+    return &status_area_focus_search_;
+  }
+
+  views::FocusTraversable* GetFocusTraversableParent() override {
+    return webui_login_view_->cycle_focus_traversable_.get();
+  }
+
+  views::View* GetFocusTraversableParentView() override {
+    return webui_login_view_->status_area_widget_host_;
+  }
+
+ private:
+  WebUILoginView* const webui_login_view_;
+  views::FocusSearch status_area_focus_search_;
+
+  DISALLOW_COPY_AND_ASSIGN(StatusAreaFocusTraversable);
+};
+
 // WebUILoginView public: ------------------------------------------------------
 
 WebUILoginView::WebUILoginView()
@@ -169,8 +234,13 @@ WebUILoginView::~WebUILoginView() {
 
   if (!chrome::IsRunningInMash() &&
       ash::Shell::GetInstance()->HasPrimaryStatusArea()) {
-    ash::Shell::GetInstance()->GetPrimarySystemTray()->
-        SetNextFocusableView(NULL);
+    views::Widget* tray_widget =
+        ash::Shell::GetInstance()->GetPrimarySystemTray()->GetWidget();
+    ash::StatusAreaWidgetDelegate* status_area_widget_delegate =
+        static_cast<ash::StatusAreaWidgetDelegate*>(
+            tray_widget->GetContentsView());
+    status_area_widget_delegate->set_custom_focus_traversable(nullptr);
+    status_area_widget_delegate->set_default_last_focusable_child(false);
   } else {
     NOTIMPLEMENTED();
   }
@@ -206,6 +276,9 @@ void WebUILoginView::Init() {
   content::RendererPreferences* prefs = web_contents->GetMutableRendererPrefs();
   renderer_preferences_util::UpdateFromSystemSettings(
       prefs, signin_profile, web_contents);
+
+  status_area_widget_host_ = new views::View;
+  AddChildView(status_area_widget_host_);
 }
 
 const char* WebUILoginView::GetClassName() const {
@@ -281,6 +354,15 @@ void WebUILoginView::LoadURL(const GURL & url) {
       ->GetWidget()
       ->GetView()
       ->SetBackgroundColor(SK_ColorTRANSPARENT);
+
+  ash::StatusAreaWidgetDelegate* status_area_widget_delegate =
+      GetStatusAreaWidgetDelegate();
+  DCHECK(status_area_widget_delegate);
+  cycle_focus_traversable_.reset(new CycleFocusTraversable(this));
+  status_area_focus_traversable_.reset(
+      new StatusAreaFocusTraversable(status_area_widget_delegate, this));
+  status_area_widget_delegate->set_custom_focus_traversable(
+      status_area_focus_traversable_.get());
 }
 
 content::WebUI* WebUILoginView::GetWebUI() {
@@ -435,9 +517,11 @@ bool WebUILoginView::TakeFocus(content::WebContents* source, bool reverse) {
   if (chrome::IsRunningInMash())
     return true;
 
-  ash::SystemTray* tray = ash::Shell::GetInstance()->GetPrimarySystemTray();
-  if (tray && tray->GetWidget()->IsVisible()) {
-    tray->SetNextFocusableView(this);
+  ash::StatusAreaWidgetDelegate* status_area_widget_delegate =
+      GetStatusAreaWidgetDelegate();
+  if (status_area_widget_delegate &&
+      status_area_widget_delegate->GetWidget()->IsVisible()) {
+    status_area_widget_delegate->set_default_last_focusable_child(reverse);
     ash::WmShell::Get()->focus_cycler()->RotateFocus(
         reverse ? ash::FocusCycler::BACKWARD : ash::FocusCycler::FORWARD);
   }

chrome/browser/chromeos/login/ui/webui_login_view.h

@@ -162,6 +162,17 @@ class WebUILoginView : public views::View,
   // True to forward keyboard event.
   bool forward_keyboard_event_;
 
+  // A FocusTraversable for StatusAreaWidget that uses
+  // |status_area_widget_host_| as placeholder in WebUiLoginView's focus chain.
+  class StatusAreaFocusTraversable;
+  std::unique_ptr<StatusAreaFocusTraversable> status_area_focus_traversable_;
+  views::View* status_area_widget_host_ = nullptr;
+
+  // A FocusTraversable for WebUILoginView that loops back at the end of its
+  // focus chain.
+  class CycleFocusTraversable;
+  std::unique_ptr<CycleFocusTraversable> cycle_focus_traversable_;
+
   base::ObserverList<web_modal::ModalDialogHostObserver> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(WebUILoginView);