Do not activate system tray bubble by default

ash/common/system/tray/system_tray.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/tray/system_tray.h"
 
+#include "ash/common/key_event_watcher.h"
 #include "ash/common/login_status.h"
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/shelf/wm_shelf_util.h"
 #include "ash/common/shell_window_ids.h"
 #include "ash/common/system/cast/tray_cast.h"
 #include "ash/common/system/date/tray_date.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/system/tray/system_tray_item.h"
 #include "ash/common/system/tray/tray_bubble_wrapper.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray_accessibility.h"
 #include "ash/common/system/update/tray_update.h"
 #include "ash/common/system/user/tray_user.h"
 #include "ash/common/system/user/tray_user_separator.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/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/timer/timer.h"
 #include "grit/ash_strings.h"
+#include "ui/base/accelerators/accelerator.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/compositor/layer.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/events/event_constants.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/view.h"
@@ skipping 14 matching lines @@
 #include "ash/common/system/chromeos/screen_security/screen_share_tray_item.h"
 #include "ash/common/system/chromeos/session/tray_session_length_limit.h"
 #include "ash/common/system/chromeos/settings/tray_settings.h"
 #include "ash/common/system/chromeos/supervised/tray_supervised_user.h"
 #include "ash/common/system/chromeos/tray_caps_lock.h"
 #include "ash/common/system/chromeos/tray_tracing.h"
 #include "ash/common/system/ime/tray_ime_chromeos.h"
 #include "ui/message_center/message_center.h"
 #endif
 
+#include "base/debug/stack_trace.h"

[James Cook, 2016/09/22 21:17:16]

remove

[oshima, 2016/09/23 09:37:20]

Oops, done.

+
 using views::TrayBubbleView;
 
 namespace ash {
 
 // The minimum width of the system tray menu.
 const int kMinimumSystemTrayMenuWidth = 300;
 const int kMinimumSystemTrayMenuWidthMd = 332;
 
 // Class to initialize and manage the SystemTrayBubble and TrayBubbleWrapper
 // instances for a bubble.
@@ skipping 56 matching lines @@
       tray_cast_(nullptr),
       tray_date_(nullptr),
       tray_update_(nullptr),
       screen_capture_tray_item_(nullptr),
       screen_share_tray_item_(nullptr) {
   SetContentsBackground();
 }
 
 SystemTray::~SystemTray() {
   // Destroy any child views that might have back pointers before ~View().
+  key_event_watcher_.reset();
   system_bubble_.reset();
   notification_bubble_.reset();
   for (std::vector<SystemTrayItem*>::iterator it = items_.begin();
        it != items_.end(); ++it) {
     (*it)->DestroyTrayView();
   }
 }
 
 void SystemTray::InitializeTrayItems(
     SystemTrayDelegate* delegate,
     WebNotificationTray* web_notification_tray) {
   DCHECK(web_notification_tray);
   web_notification_tray_ = web_notification_tray;
   TrayBackgroundView::Initialize();
   CreateItems(delegate);
 }
 
 void SystemTray::Shutdown() {
   DCHECK(web_notification_tray_);
+  key_event_watcher_.reset();

[James Cook, 2016/09/22 21:17:16]

Does this need to be both here and in destructor?

[oshima, 2016/09/23 09:37:19]

Yes, it is in dtor too.

[James Cook, 2016/09/23 16:32:28]

I meant, does it have to be in the destructor? In theory all users of SystemTray
should be calling Shutdown() before they delete the object. If someone isn't
calling Shutdown() that sounds like a bug.

[oshima, 2016/09/23 19:19:53]

It should actually be dtor, where the bubble is reset. This Shutdown is only to
reset the web_notification_tray_, so i removed them.

   web_notification_tray_ = nullptr;
 }
 
 void SystemTray::CreateItems(SystemTrayDelegate* delegate) {
   WmShell* wm_shell = WmShell::Get();
 #if !defined(OS_WIN)
   // Create user items for each possible user.
   int maximum_user_profiles =
       wm_shell->GetSessionStateDelegate()->GetMaximumNumberOfLoggedInUsers();
   for (int i = 0; i < maximum_user_profiles; i++)
@@ skipping 274 matching lines @@
 void SystemTray::ShowDefaultViewWithOffset(BubbleCreationType creation_type,
                                            int arrow_offset,
                                            bool persistent) {
   if (creation_type != BUBBLE_USE_EXISTING)
     WmShell::Get()->RecordUserMetricsAction(UMA_STATUS_AREA_MENU_OPENED);
   ShowItems(items_.get(), false, true, creation_type, arrow_offset, persistent);
 }
 
 void SystemTray::ShowItems(const std::vector<SystemTrayItem*>& items,
                            bool detailed,
                            bool can_activate,

[James Cook, 2016/09/22 21:17:16]

This needs to be renamed. It no longer means "can activate".

[oshima, 2016/09/23 09:37:20]

The name should have been "activate", and that's what is used in the header. I
think can_activate is actually better now because it can be activated by several
means. I updated the header instead.

                            BubbleCreationType creation_type,
                            int arrow_offset,
                            bool persistent) {
   // No system tray bubbles in kiosk mode.
   if (WmShell::Get()->system_tray_delegate()->GetUserLoginStatus() ==
       LoginStatus::KIOSK_APP) {
     return;
   }
 
   // Destroy any existing bubble and create a new one.
@@ skipping 24 matching lines @@
     // The menu width is fixed, and it is a per language setting.
     int menu_width = std::max(
         MaterialDesignController::IsSystemTrayMenuMaterial()
             ? kMinimumSystemTrayMenuWidthMd
             : kMinimumSystemTrayMenuWidth,
         WmShell::Get()->system_tray_delegate()->GetSystemTrayMenuWidth());
 
     TrayBubbleView::InitParams init_params(TrayBubbleView::ANCHOR_TYPE_TRAY,
                                            GetAnchorAlignment(), menu_width,
                                            kTrayPopupMaxWidth);
-    init_params.can_activate = can_activate;
+    // TODO(oshima): Change TrayBubbleView itself.
+    init_params.can_activate = false;
     init_params.first_item_has_no_margin = true;
     if (detailed) {
       // This is the case where a volume control or brightness control bubble
       // is created.
       init_params.max_height = default_bubble_height_;
       init_params.arrow_color = kBackgroundColor;
     } else {
       init_params.arrow_color = kHeaderBackgroundColor;
     }
     init_params.arrow_offset = arrow_offset;
@@ skipping 10 matching lines @@
     system_bubble_->InitView(this, tray_container(), &init_params, persistent);
 
     // Record metrics for the system menu when the default view is invoked.
     if (!detailed)
       RecordSystemMenuMetrics();
   }
   // Save height of default view for creating detailed views directly.
   if (!detailed)
     default_bubble_height_ = system_bubble_->bubble_view()->height();
 
+  key_event_watcher_.reset();

[James Cook, 2016/09/22 21:17:16]

optional: This might be clearer in an else{} clause below.

[oshima, 2016/09/23 09:37:19]

I prefer this way because this can avoid potential issue in having two
KeyEventWatcher instances at the same time. (New one is created before
resetting)

+  if (can_activate) {

[James Cook, 2016/09/22 21:17:16]

Document what you are doing in this block of code and/or split into a separate
function, since ShowItems() is > 100 lines long now.

[oshima, 2016/09/23 09:37:20]

Done.

+    key_event_watcher_ = WmShell::Get()->CreateKeyEventWatcher();
+    key_event_watcher_->AddKeyEventCallback(
+        ui::Accelerator(ui::VKEY_ESCAPE, 0),

[James Cook, 2016/09/22 21:17:16]

0 -> EF_NONE

[oshima, 2016/09/23 09:37:19]

Done.

+        base::Bind(&SystemTray::CloseBubble, base::Unretained(this)));
+    key_event_watcher_->AddKeyEventCallback(
+        ui::Accelerator(ui::VKEY_TAB, 0),
+        base::Bind(&SystemTray::ActivateAndStartNavigation,
+                   base::Unretained(this)));
+    key_event_watcher_->AddKeyEventCallback(
+        ui::Accelerator(ui::VKEY_TAB, ui::EF_SHIFT_DOWN),
+        base::Bind(&SystemTray::ActivateAndStartNavigation,
+                   base::Unretained(this)));
+  }
+
   if (detailed && items.size() > 0)
     detailed_item_ = items[0];
   else
     detailed_item_ = NULL;
 
   UpdateNotificationBubble();  // State changed, re-create notifications.
   if (!notification_bubble_)
     UpdateWebNotifications();
   shelf()->UpdateAutoHideState();
 
@@ skipping 182 matching lines @@
 }
 
 TrayDate* SystemTray::GetTrayDateForTesting() const {
   return tray_date_;
 }
 
 TrayUpdate* SystemTray::GetTrayUpdateForTesting() const {
   return tray_update_;
 }
 
+void SystemTray::CloseBubble(const ui::KeyEvent& key_event) {
+  CloseSystemBubble();
+}
+
+void SystemTray::ActivateAndStartNavigation(const ui::KeyEvent& key_event) {
+  if (!system_bubble_)
+    return;
+  ActivateBubble();
+  views::Widget* widget = GetSystemBubble()->bubble_view()->GetWidget();
+  widget->GetFocusManager()->OnKeyEvent(key_event);
+}
+
+void SystemTray::ActivateBubble() {
+  TrayBubbleView* bubble_view = GetSystemBubble()->bubble_view();
+  bubble_view->set_can_activate(true);
+  bubble_view->GetWidget()->Activate();
+}
+
 bool SystemTray::PerformAction(const ui::Event& event) {
   // If we're already showing the default view, hide it; otherwise, show it
   // (and hide any popup that's currently shown).
   if (HasSystemBubbleType(SystemTrayBubble::BUBBLE_TYPE_DEFAULT)) {
     system_bubble_->bubble()->Close();
   } else {
     int arrow_offset = TrayBubbleView::InitParams::kArrowDefaultOffset;
     if (event.IsMouseEvent() || event.type() == ui::ET_GESTURE_TAP) {
       const ui::LocatedEvent& located_event =
           static_cast<const ui::LocatedEvent&>(event);
       if (IsHorizontalAlignment(shelf_alignment())) {
         gfx::Point point(located_event.x(), 0);
         ConvertPointToWidget(this, &point);
         arrow_offset = point.x();
       }
     }
     ShowDefaultViewWithOffset(BUBBLE_CREATE_NEW, arrow_offset, false);
   }
   return true;
 }
 
 void SystemTray::CloseSystemBubbleAndDeactivateSystemTray() {
+  key_event_watcher_.reset();
   system_bubble_.reset();
   // When closing a system bubble with the alternate shelf layout, we need to
   // turn off the active tinting of the shelf.
   if (full_system_tray_menu_) {
     SetDrawBackgroundAsActive(false);
     full_system_tray_menu_ = false;
   }
 }
 
 void SystemTray::RecordSystemMenuMetrics() {
@@ skipping 17 matching lines @@
           .work_area()
           .height();
   if (work_area_height > 0) {
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "Ash.SystemMenu.PercentageOfWorkAreaHeightCoveredByMenu",
         100 * bubble_view->height() / work_area_height, 1, 300, 100);
   }
 }
 
 }  // namespace ash