Tablet WM : Swiping on system tray bubble.

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 32 matching lines @@
 #include "ash/system/tray/tray_bubble_wrapper.h"
 #include "ash/system/tray/tray_constants.h"
 #include "ash/system/tray/tray_container.h"
 #include "ash/system/tray_accessibility.h"
 #include "ash/system/tray_caps_lock.h"
 #include "ash/system/tray_tracing.h"
 #include "ash/system/update/tray_update.h"
 #include "ash/system/user/tray_user.h"
 #include "ash/system/web_notification/web_notification_tray.h"
 #include "ash/wm/container_finder.h"
+#include "ash/wm/maximize_mode/maximize_mode_controller.h"
 #include "ash/wm/widget_finder.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/timer/timer.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"
@@ skipping 102 matching lines @@
   // WmActivationObserver:
   void OnWindowActivated(ActivationReason reason,
                          aura::Window* gained_active,
                          aura::Window* lost_active) override {
     if (!tray_->HasSystemBubble() || !gained_active)
       return;
 
     int container_id = wm::GetContainerForWindow(gained_active)->id();
 
     // Don't close the bubble if a popup notification is activated.
-    if (container_id == kShellWindowId_StatusContainer)
+    if (container_id == kShellWindowId_StatusContainer ||
+        container_id == kShellWindowId_SettingBubbleContainer) {
       return;
+    }
 
     views::Widget* bubble_widget =
         tray_->GetSystemBubble()->bubble_view()->GetWidget();
     // Don't close the bubble if a transient child is gaining or losing
     // activation.
     if (bubble_widget == GetInternalWidgetForWindow(gained_active) ||
         ::wm::HasTransientAncestor(gained_active,
                                    bubble_widget->GetNativeWindow()) ||
         (lost_active && ::wm::HasTransientAncestor(
                             lost_active, bubble_widget->GetNativeWindow()))) {
       return;
     }
 
     tray_->CloseSystemBubble();
   }
 
  private:
   SystemTray* tray_;
 
   DISALLOW_COPY_AND_ASSIGN(ActivationObserver);
 };
 
 // SystemTray
 
-SystemTray::SystemTray(Shelf* shelf) : TrayBackgroundView(shelf) {
+SystemTray::SystemTray(Shelf* shelf)
+    : TrayBackgroundView(shelf), shelf_(shelf) {
   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);
+
+  Shell::Get()->AddShellObserver(this);
+  in_maximize_mode_ = Shell::Get()
+                          ->maximize_mode_controller()
+                          ->IsMaximizeModeWindowManagerEnabled();
 }
 
 SystemTray::~SystemTray() {
   // Destroy any child views that might have back pointers before ~View().
   activation_observer_.reset();
   key_event_watcher_.reset();
   system_bubble_.reset();
   for (const auto& item : items_)
     item->OnTrayViewDestroyed();
+  Shell::Get()->RemoveShellObserver(this);
 }
 
 void SystemTray::InitializeTrayItems(
     SystemTrayDelegate* delegate,
     WebNotificationTray* web_notification_tray) {
   DCHECK(web_notification_tray);
   web_notification_tray_ = web_notification_tray;
   TrayBackgroundView::Initialize();
   CreateItems(delegate);
 }
@@ skipping 355 matching lines @@
 }
 
 base::string16 SystemTray::GetAccessibleNameForBubble() {
   return GetAccessibleNameForTray();
 }
 
 void SystemTray::HideBubble(const TrayBubbleView* bubble_view) {
   HideBubbleWithView(bubble_view);
 }
 
+void SystemTray::OnMaximizeModeStarted() {
+  in_maximize_mode_ = true;
+}
+
+void SystemTray::OnMaximizeModeEnded() {
+  in_maximize_mode_ = false;
+}
+
 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();
@@ skipping 21 matching lines @@
 void SystemTray::ActivateBubble() {
   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();
 }
 
+void SystemTray::OnGestureEvent(ui::GestureEvent* event) {

[tdanderson, 2017/06/19 16:19:38]

Why are you restricting this to just a horizontally-aligned shelf?

[minch1, 2017/06/20 16:45:34]

Since tablet will has horizontal shelf only in the future.

[tdanderson, 2017/06/20 22:42:19]

Acknowledged.

+  if (in_maximize_mode_ && shelf_->IsHorizontalAlignment() &&

[tdanderson, 2017/06/19 16:19:38]

Side question to ask UX (OK to handle in a follow-on CL): since we're letting
the user open the system menu with a swipe-up, should we also be letting them
close the system menu with a swipe-down on a menu that is already open?

[minch1, 2017/06/20 16:45:34]

Yes, that's right. We definitely need that, I will do that in next CL.

[tdanderson, 2017/06/20 22:42:20]

Acknowledged.

+      ProcessGestureEvent(*event)) {
+    event->StopPropagation();
+  } else {
+    CustomButton::OnGestureEvent(event);

[tdanderson, 2017/06/19 16:19:38]

I think you'd instead want to call up to TrayBackrgroundView::OnGestureEvent()
since SystemTray inherits directly from TrayBackgroundView. 

If you were to call up to CustomButton::OnGestureEvent() and at some point in
the future someone added a TrayBackgroundView::OnGestureEvent() or
ActionableView::OnGestureEvent() override, these overrides would be skipped when
a gesture targeted SystemTray.

[minch1, 2017/06/20 16:45:34]

Done.

+  }
+}
+
+gfx::Rect SystemTray::GetWorkAreaBoundsInScreen() const {
+  return shelf_->GetUserWorkAreaBounds();
+}
+
 bool SystemTray::PerformAction(const ui::Event& event) {
   // If we're already showing a full system tray menu, either default or
   // detailed menu, hide it; otherwise, show it (and hide any popup that's
   // currently shown).
   if (HasSystemBubble() && full_system_tray_menu_) {
     system_bubble_->bubble()->Close();
   } else {
     ShowDefaultView(BUBBLE_CREATE_NEW);
     if (event.IsKeyEvent() || (event.flags() & ui::EF_TOUCH_ACCESSIBILITY))
       ActivateBubble();
   }
   return true;
 }
 
+bool SystemTray::ProcessGestureEvent(const ui::GestureEvent& event) {

[tdanderson, 2017/06/19 16:19:38]

When building and trying out this CL locally, I can get a crash when I do the
following when the system menu is closed: without your finger leaving the
screen, very quickly swipe up on the system menu, then down, then up, then down
again.

One possible cause for this crash could be that you are not handling all
relevant possibilities for gesture events. For instance you may be receiving
ET_SCROLL_FLING_CANCEL, ET_GESTURE_SWIPE, ET_GESTURE_END, etc. I suggest adding
logging to this function to see which event types and sequences can be seen.

[minch1, 2017/06/20 16:45:34]

I am sorry I cannot repro this locally. But I guess this maybe the same reason
as another crash following (one finger tap the bubble then another finger tap
elsewhere). Quickly swiping maybe touch the desktop by accident. All the other
gesture events will return false here directly.
Let me know if you can still repro the crash after my change.

[tdanderson, 2017/06/20 22:42:20]

I can still repro locally with Patch Set 8. I will share/send you a video of
what I'm seeing.

+  if (event.type() == ui::ET_GESTURE_SCROLL_BEGIN)

[xdai1, 2017/06/19 19:01:30]

I think "switch case" might make the code cleaner and easier to follow.

[minch1, 2017/06/20 16:45:34]

I think "switch case" is not so good after update. Please check.

+    return StartGestureDrag(event);
+
+  if (event.type() == ui::ET_GESTURE_SCROLL_UPDATE) {
+    UpdateGestureDrag(event);
+    return true;
+  }
+
+  if (event.type() == ui::ET_GESTURE_SCROLL_END ||
+      event.type() == ui::ET_SCROLL_FLING_START) {
+    CompleteGestureDrag(event);
+    return true;
+  }
+  return false;
+}
+
+bool SystemTray::StartGestureDrag(const ui::GestureEvent& gesture) {
+  if ((HasSystemBubble() && full_system_tray_menu_) ||
+      IsDraggingDownOnShelf(gesture)) {

[tdanderson, 2017/06/19 16:19:38]

I don't understand what the check IsDraggingDownOnShelf() does and why you need
it. Can you please explain?

[minch1, 2017/06/20 16:45:34]

When there is no bubble. Swiping down on the system tray should actor the same
as if swiping down on the other positions on the shelf to hide the shelf.
If I didn't check this, swiping down on the system tray at the beginning will
not hide the shelf.

+    return false;
+  }
+
+  gesture_drag_amount_ = 0.f;
+  ShowDefaultView(BUBBLE_CREATE_NEW);
+  system_tray_bubble_bounds_ =
+      system_bubble_->bubble_view()->GetWidget()->GetWindowBoundsInScreen();
+  UpdateBoundsOnDragging(gesture);
+  return true;
+}
+
+void SystemTray::UpdateGestureDrag(const ui::GestureEvent& gesture) {
+  UpdateBoundsOnDragging(gesture);
+  gesture_drag_amount_ += gesture.details().scroll_y();
+}
+
+void SystemTray::CompleteGestureDrag(const ui::GestureEvent& gesture) {
+  gfx::Rect target_bounds = system_tray_bubble_bounds_;
+  if (ShouldShowSystemBubbleForDragging(gesture)) {
+    system_bubble_->bubble()->UpdateBounds(target_bounds, false);
+  } else {
+    target_bounds.set_y(shelf_->GetIdealBounds().y());
+    system_bubble_->bubble()->UpdateBounds(target_bounds, true);
+  }
+}
+
+bool SystemTray::IsDraggingDownOnShelf(const ui::GestureEvent& gesture) {
+  gfx::Point gesture_location = gesture.location();
+  View::ConvertPointToScreen(this, &gesture_location);
+
+  if (gesture_location.y() >= shelf_->GetIdealBounds().y() &&
+      gesture.details().scroll_y_hint() > 0) {
+    return true;
+  }
+  return false;
+}
+
+void SystemTray::UpdateBoundsOnDragging(const ui::GestureEvent& gesture) {
+  gfx::Point gesture_location = gesture.location();
+  View::ConvertPointToScreen(this, &gesture_location);
+
+  // System tray bubble should not be dragged higher than its original height.
+  if (gesture_location.y() < system_tray_bubble_bounds_.y())
+    gesture_location.set_y(system_tray_bubble_bounds_.y());
+
+  gfx::Rect bounds_on_gesture(
+      system_tray_bubble_bounds_.x(), gesture_location.y(),
+      system_tray_bubble_bounds_.width(), system_tray_bubble_bounds_.height());
+  system_bubble_->bubble_view()->GetWidget()->SetBounds(bounds_on_gesture);
+}
+
+bool SystemTray::ShouldShowSystemBubbleBasedOnDragAmount() {
+  if (gesture_drag_amount_ > 0)
+    return false;
+
+  int system_bubble_height = system_tray_bubble_bounds_.height();
+  float one_third = system_bubble_height / 3.0;
+
+  return fabs(gesture_drag_amount_) >= one_third;
+}
+
+bool SystemTray::ShouldShowSystemBubbleForDragging(
+    const ui::GestureEvent& gesture) {
+  if (gesture.type() == ui::ET_GESTURE_SCROLL_END)
+    return ShouldShowSystemBubbleBasedOnDragAmount();
+
+  const float kSwipingVelocity = 100.0f;
+  if (fabs(gesture.details().velocity_y()) > kSwipingVelocity)

[tdanderson, 2017/06/19 16:19:38]

Consider setting kSwipingVelocity to -100 and inlining
ShouldShowSystemBubbleBasedOnDragAmount(). Then you could simplify this function
to just:

const float kSwipingVelocity = -100.0f;
if (gesture.type() == FLING_START &&
    gesture.details.velocity_y() < kSwipingVelocity()) {
  return true;
}

return -gesture_drag_amount_ >= system_tray_bubble_bounds_.height() / 3.0;

[minch1, 2017/06/20 16:45:34]

Thanks. Done the change for ShouldShowSystemBubbleBasedOnDragAmount. But keep
line 764-766. Since should return false (close the bubble) if velocity > 100.0f.

[tdanderson, 2017/06/20 22:42:20]

Acknowledged.

+    return gesture.details().velocity_y() < 0;
+  return ShouldShowSystemBubbleBasedOnDragAmount();
+}
+
 void SystemTray::CloseSystemBubbleAndDeactivateSystemTray() {
   activation_observer_.reset();
   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_) {
     SetIsActive(false);
     full_system_tray_menu_ = false;
   }
@@ skipping 20 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