Signin screen and locked screen status area focus advancing

ash/common/system/status_area_widget.cc

 // Copyright (c) 2012 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 "ash/common/system/status_area_widget.h"
 
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/system/overview/overview_button_tray.h"
 #include "ash/common/system/status_area_widget_delegate.h"
 #include "ash/common/system/tray/system_tray.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
+#include "ash/common/system/tray/system_tray_notifier.h"
 #include "ash/common/system/web_notification/web_notification_tray.h"
 #include "ash/common/wm_lookup.h"
 #include "ash/common/wm_root_window_controller.h"
 #include "ash/common/wm_shell.h"
 #include "ash/common/wm_window.h"
 #include "base/i18n/time_formatting.h"
 #include "ui/native_theme/native_theme_dark_aura.h"
+#include "ui/views/focus/focus_manager.h"
 
 #if defined(OS_CHROMEOS)
 #include "ash/common/system/chromeos/ime_menu/ime_menu_tray.h"
 #include "ash/common/system/chromeos/palette/palette_tray.h"
 #include "ash/common/system/chromeos/palette/palette_utils.h"
 #include "ash/common/system/chromeos/session/logout_button_tray.h"
 #include "ash/common/system/chromeos/virtual_keyboard/virtual_keyboard_tray.h"
 #endif
 
 namespace ash {
@@ skipping 181 matching lines @@
              ? ui::NativeThemeDarkAura::instance()
              : Widget::GetNativeTheme();
 }
 
 void StatusAreaWidget::OnNativeWidgetActivationChanged(bool active) {
   Widget::OnNativeWidgetActivationChanged(active);
   if (active)
     status_area_widget_delegate_->SetPaneFocusAndFocusDefault();
 }
 
+void StatusAreaWidget::OnKeyEvent(ui::KeyEvent* event) {
+  WmShell* wm_shell = WmShell::Get();
+  SystemTrayDelegate* delegate = wm_shell->system_tray_delegate();
+  LoginStatus status = delegate->GetUserLoginStatus();
+  if (status == LoginStatus::NOT_LOGGED_IN || status == LoginStatus::LOCKED) {
+    if (event->type() == ui::ET_KEY_PRESSED &&
+        views::FocusManager::IsTabTraversalKeyEvent(*event)) {

[stevenjb, 2016/09/02 16:29:53]

Combine these ifs, invert the logic, call Widget::OnKeyEvent(event), and early
exit.

[Qiang(Joe) Xu, 2016/09/02 19:32:51]

Done.

+      bool reverse = event->IsShiftDown();
+      views::FocusManager* focus_manager = GetFocusManager();
+      if (!focus_manager) {
+        Widget::OnKeyEvent(event);
+        return;
+      }

[stevenjb, 2016/09/02 16:29:53]

Like this :)

[Qiang(Joe) Xu, 2016/09/02 19:32:51]

Done.

+      views::View* next_focusable_view = focus_manager->GetNextFocusableView(
+          focus_manager->GetFocusedView(), nullptr, false, false);
+#if defined(OS_CHROMEOS)
+      // When |system_tray_|, |web_notification_tray_|, |virtual_keyboard_tray_|
+      // are all visible, the reverse sequence needs special processing as
+      // |virtual_keyboard_tray_|'s next focusable view is |system_tray_|,
+      // however, |virtual_keyboard_tray_|'s reverse next view should be
+      // |web_notification_tray_|.
+      if (reverse && web_notification_tray_ &&
+          web_notification_tray_->visible() && virtual_keyboard_tray_ &&
+          virtual_keyboard_tray_->visible() && system_tray_ &&
+          system_tray_->visible()) {
+        if (virtual_keyboard_tray_->HasFocus()) {
+          Widget::OnKeyEvent(event);
+          return;

[stevenjb, 2016/09/02 16:29:53]

And this. Also no else after a return.

Also, you may want to put all of this in a helper function, e.g.:

void StatusAreaWidget::OnKeyEvent(ui::KeyEvent* event) {
  HandleFocusKeyEvent(event);
  Widget::OnKeyEvent(event);
}

Then HandleFocusKeyEvent() can just return without having to call
Widget::OnKeyEvent().

[Qiang(Joe) Xu, 2016/09/02 19:32:51]

Combine this return seems to add code lines for OS_CHROMEOS macros and if
condition. I leave it here.

Done for HandleFocusKeyEvent().

+        } else if (web_notification_tray_->HasFocus()) {
+          wm_shell->system_tray_notifier()->NotifyWillReturnFocusToWebContents(
+              reverse);
+          event->SetHandled();
+        }
+      }
+#endif
+      // At most two visible trays that need focus on status area:
+      if (system_tray_ && system_tray_->visible() &&
+          system_tray_->Contains(next_focusable_view)) {
+        wm_shell->system_tray_notifier()->NotifyWillReturnFocusToWebContents(
+            reverse);
+        event->SetHandled();
+      }
+    }
+  }
+  Widget::OnKeyEvent(event);
+}
+
 void StatusAreaWidget::UpdateShelfItemBackground(int alpha) {
   web_notification_tray_->UpdateShelfItemBackground(alpha);
   system_tray_->UpdateShelfItemBackground(alpha);
 #if defined(OS_CHROMEOS)
   virtual_keyboard_tray_->UpdateShelfItemBackground(alpha);
   logout_button_tray_->UpdateShelfItemBackground(alpha);
   ime_menu_tray_->UpdateShelfItemBackground(alpha);
   if (palette_tray_)
     palette_tray_->UpdateShelfItemBackground(alpha);
 #endif
@@ skipping 52 matching lines @@
   return false;
 }
 #endif
 
 void StatusAreaWidget::AddOverviewButtonTray() {
   overview_button_tray_ = new OverviewButtonTray(wm_shelf_);
   status_area_widget_delegate_->AddTray(overview_button_tray_);
 }
 
 }  // namespace ash