Open system menu when click the status tray with volume popup opend

ash/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/system/tray/system_tray.h"
 
 #include <algorithm>
 #include <map>
 #include <vector>
 
@@ skipping 191 matching lines @@
   }
 
  private:
   SystemTray* tray_;
 
   DISALLOW_COPY_AND_ASSIGN(ActivationObserver);
 };
 
 // SystemTray
 
-SystemTray::SystemTray(WmShelf* wm_shelf)
-    : TrayBackgroundView(wm_shelf, true),
-      web_notification_tray_(nullptr),
-      detailed_item_(nullptr),
-      default_bubble_height_(0),
-      full_system_tray_menu_(false),
-      tray_accessibility_(nullptr),
-      tray_audio_(nullptr),
-      tray_cast_(nullptr),
-      tray_network_(nullptr),
-      tray_tiles_(nullptr),
-      tray_system_info_(nullptr),
-      tray_update_(nullptr),
-      screen_capture_tray_item_(nullptr),
-      screen_share_tray_item_(nullptr) {
+SystemTray::SystemTray(WmShelf* wm_shelf) : TrayBackgroundView(wm_shelf, true) {
   SetInkDropMode(InkDropMode::ON);
 
   // Since user avatar is on the right hand side of System tray of a
   // horizontal shelf and that is sufficient to indicate separation, no
   // separator is required.
   set_separator_visibility(false);
 }
 
 SystemTray::~SystemTray() {
   // Destroy any child views that might have back pointers before ~View().
@@ skipping 100 matching lines @@
 }
 
 void SystemTray::ShowPersistentDefaultView() {
   ShowItems(GetTrayItems(), false, false, BUBBLE_CREATE_NEW, true);
 }
 
 void SystemTray::ShowDetailedView(SystemTrayItem* item,
                                   int close_delay,
                                   bool activate,
                                   BubbleCreationType creation_type) {
+  detailed_view_with_close_delay_ = close_delay > 0 ? true : false;

[tdanderson, 2017/04/18 17:59:14]

I think you can just say = close_delay > 0 here instead of using a ternary.

Also, I am concerned about this member staying up-to-date. Would you also want
to modify this member in cases such as:

* If SystemTray::SetDetailedViewCloseDelay() is called
* (maybe) when the detailed view closes?

And/or you should also update the member documentation to clarify that this
member only has meaning IF a detailed view is currently open.

[minch1, 2017/04/18 18:29:34]

I think the ternary here is necessary. Since every time 'ShowDetailedView' is
called, the variable detailed_view_with_close_delay will be updated according to
the type of the detailed view (popup with close_delay or detailed view in system
menu without close_delay)

[tdanderson, 2017/04/19 16:10:40]

I agree that you should be updating |detailed_view_with_close_delay_| here, but
my comment is that I think you can just write this:

detailed_view_with_close_delay_ = close_delay > 0;

instead of this:

detailed_view_with_close_delay_ = close_delay > 0 ? true : false;

   std::vector<SystemTrayItem*> items;
   // The detailed view with timeout means a UI to show the current system state,
   // like the audio level or brightness. Such UI should behave as persistent and
   // keep its own logic for the appearance.
   bool persistent =
       (!activate && close_delay > 0 && creation_type == BUBBLE_CREATE_NEW);
   items.push_back(item);
   ShowItems(items, true, activate, creation_type, persistent);
   if (system_bubble_)
     system_bubble_->bubble()->StartAutoCloseTimer(close_delay);
@@ skipping 334 matching lines @@
   TrayBubbleView* bubble_view = GetSystemBubble()->bubble_view();
   // If system tray bubble is in the process of closing, do not try to activate
   // bubble.
   if (bubble_view->GetWidget()->IsClosed())
     return;
   bubble_view->set_can_activate(true);
   bubble_view->GetWidget()->Activate();
 }
 
 bool SystemTray::PerformAction(const ui::Event& event) {
-  // If we're already showing the menu, hide it; otherwise, show it (and hide
-  // any popup that's currently shown).
-  if (HasSystemBubble()) {
+  // If we're already showing the default view or detailed view in system menu,
+  // hide it; otherwise, show it (and hide any popup that's currently shown).
+  if (HasSystemBubbleType(SystemTrayBubble::BUBBLE_TYPE_DEFAULT) ||
+      (HasSystemBubbleType(SystemTrayBubble::BUBBLE_TYPE_DETAILED) &&
+       !detailed_view_with_close_delay_)) {

[tdanderson, 2017/04/18 17:59:14]

With this change, if you:

* Open the system menu
* Open one of the detailed views (a11y for instance)
* Click the tray button

What happens? From reading this it seems like the menu won't close, and this
would be regressing the change you made in
https://codereview.chromium.org/2784163003

[minch1, 2017/04/18 18:29:34]

I tried this in my device. The system menu will be closed correctly in this
scenario. Since detailed_view_with_close_delay_ is updated in ShowDetailedView. 

[tdanderson, 2017/04/19 16:10:40]

You're right, I was reading this incorrectly. What you have here will work.

     system_bubble_->bubble()->Close();
   } else {
     ShowDefaultView(BUBBLE_CREATE_NEW);
     if (event.IsKeyEvent() || (event.flags() & ui::EF_TOUCH_ACCESSIBILITY))
       ActivateBubble();
   }
   return true;
 }
 
 void SystemTray::CloseSystemBubbleAndDeactivateSystemTray() {
@@ skipping 29 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