Tablet WM : Swiping on system tray bubble.

ash/system/tray/system_tray.cc

Index: ash/system/tray/system_tray.cc
diff --git a/ash/system/tray/system_tray.cc b/ash/system/tray/system_tray.cc
index 66ca2527aa0a3fcbdf222f0f1912912471c14af7..8ff827ebfbdba51e51a171ce28d7ab1892be839f 100644
--- a/ash/system/tray/system_tray.cc
+++ b/ash/system/tray/system_tray.cc
@@ -78,6 +78,9 @@ using views::TrayBubbleView;
 
 namespace ash {
 
+// The velocity of the swiping on the system bubble.
+constexpr float kSwipingVelocity = 100.0;

[xiyuan, 2017/06/12 18:00:51]

nit: move this to where it is used since it is referenced only in one place.

[minch1, 2017/06/16 03:30:34]

Done.

+
 namespace {
 
 // A tray item that just reserves space in the tray.
@@ -171,8 +174,11 @@ class SystemTray::ActivationObserver : public ::wm::ActivationChangeObserver {
 
     int container_id = wm::GetContainerForWindow(gained_active)->id();
 
-    // Don't close the bubble if a popup notification is activated.
-    if (container_id == kShellWindowId_StatusContainer)
+    // Don't close the bubble if a popup notification is activated and don't
+    // close the the bubble if click the items in the default system tray bubble
+    // on maximize mode.

[xiyuan, 2017/06/12 18:00:50]

Where is the relevant code for "on maximize mode."? If maximize mode does not
matter, removing this part in the comment.

[minch1, 2017/06/16 03:30:34]

Since we added a clipping window in maximize mode. When we click the items in
the default menu it will trigger OnWindowActivated here. We should make sure the
menu will not be closed in this situation though the comment here maybe not so
good.

+    if (container_id == kShellWindowId_StatusContainer ||
+        container_id == kShellWindowId_SettingBubbleContainer)
       return;
 
     views::Widget* bubble_widget =
@@ -198,13 +204,16 @@ class SystemTray::ActivationObserver : public ::wm::ActivationChangeObserver {
 
 // 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);

[xiyuan, 2017/06/12 18:00:51]

What if Shell starts in maximize mode? |on_maximize_mode_| is initialized to
false and would be wrong in this case.

[minch1, 2017/06/16 03:30:34]

Done.

 }
 
 SystemTray::~SystemTray() {
@@ -214,6 +223,7 @@ SystemTray::~SystemTray() {
   system_bubble_.reset();
   for (const auto& item : items_)
     item->OnTrayViewDestroyed();
+  Shell::Get()->RemoveShellObserver(this);
 }
 
 void SystemTray::InitializeTrayItems(
@@ -589,6 +599,14 @@ void SystemTray::HideBubble(const TrayBubbleView* bubble_view) {
   HideBubbleWithView(bubble_view);
 }
 
+void SystemTray::OnMaximizeModeStarted() {
+  on_maximize_mode_ = true;

[xiyuan, 2017/06/12 18:00:51]

on_maximize_mode_ -> in_maximize_mode_ (or is_maximize_mode_)

[minch1, 2017/06/16 03:30:34]

Done.

+}
+
+void SystemTray::OnMaximizeModeEnded() {
+  on_maximize_mode_ = false;
+}
+
 void SystemTray::CloseBubble(const ui::KeyEvent& key_event) {
   CloseSystemBubble();
 }
@@ -630,6 +648,10 @@ void SystemTray::ActivateBubble() {
   bubble_view->GetWidget()->Activate();
 }
 
+gfx::Rect SystemTray::GetWorkAreaBoundsInScreen() const {
+  return shelf_->GetUserWorkAreaBounds();
+}
+
 bool SystemTray::PerformAction(const ui::Event& event) {
   // 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).
@@ -645,6 +667,108 @@ bool SystemTray::PerformAction(const ui::Event& event) {
   return true;
 }
 
+void SystemTray::OnGestureEvent(ui::GestureEvent* event) {
+  if (on_maximize_mode_ && shelf_->IsHorizontalAlignment() &&
+      ProcessGestureEvent(*event))
+    event->StopPropagation();
+  else
+    CustomButton::OnGestureEvent(event);
+}
+
+bool SystemTray::ProcessGestureEvent(const ui::GestureEvent& event) {
+  if (event.type() == ui::ET_GESTURE_SCROLL_BEGIN) {
+    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 (full_system_tray_menu_ || IsSwipingDownOnShelf(gesture))
+    return false;
+
+  gesture_drag_amount_ = 0.f;
+  ShowDefaultView(BUBBLE_CREATE_NEW);
+  system_tray_bubble_bounds_ =
+      system_bubble_->bubble_view()->GetWidget()->GetWindowBoundsInScreen();
+  system_bubble_->bubble_view()->GetWidget()->Hide();
+  return true;
+}
+
+void SystemTray::UpdateGestureDrag(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);
+  system_bubble_->bubble_view()->GetWidget()->Show();

[xiyuan, 2017/06/12 18:00:50]

Can we restrict to call Show() only when necessary? I would recommend to
calculate initial bounds in StartGestureDrag() and call Show() there.

[minch1, 2017/06/16 03:30:34]

Done.

+
+  gesture_drag_amount_ += gesture.details().scroll_y();
+  final_scroll_y_ = gesture.details().scroll_y();
+}
+
+void SystemTray::CompleteGestureDrag(const ui::GestureEvent& gesture) {
+  if (gesture.details().scroll_y())
+    final_scroll_y_ = gesture.details().scroll_y();
+  if (ShouldShowSystemBubbleForSwiping(gesture))
+    system_bubble_->bubble_view()->GetWidget()->SetBounds(
+        system_tray_bubble_bounds_);

[xiyuan, 2017/06/12 18:00:51]

nit: wrap with {} since it takes more than one line. And wrap "else" branch too.

[minch1, 2017/06/16 03:30:34]

Acknowledged.

+  else
+    system_bubble_->bubble()->Close();
+}
+
+bool SystemTray::IsSwipingDownOnShelf(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() > 0) {
+    return true;
+  }
+  return false;
+}
+
+bool SystemTray::ShouldShowSystemBubbleBasedOnPosition() {
+  int system_bubble_height = system_tray_bubble_bounds_.height();
+  float one_third = system_bubble_height / 3.0;

[xiyuan, 2017/06/12 18:00:51]

move this two lines down to where they are used.

[minch1, 2017/06/16 03:30:34]

Done.

+
+  if (gesture_drag_amount_ > 0)
+    return false;
+
+  if (fabs(gesture_drag_amount_) < one_third)
+    return false;
+  else if (fabs(gesture_drag_amount_) > 2 * one_third)

[xiyuan, 2017/06/12 18:00:51]

no "else" after "return".

See
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

[minch1, 2017/06/16 03:30:34]

Done.

+    return true;
+  else
+    return final_scroll_y_ < 0;
+}
+
+bool SystemTray::ShouldShowSystemBubbleForSwiping(
+    const ui::GestureEvent& gesture) {
+  if (gesture.type() == ui::ET_GESTURE_SCROLL_END) {
+    return ShouldShowSystemBubbleBasedOnPosition();
+  } else {

[xiyuan, 2017/06/12 18:00:50]

no "else" after "return".

[minch1, 2017/06/16 03:30:34]

Done.

+    if (fabs(gesture.details().velocity_y()) > kSwipingVelocity)
+      return gesture.details().velocity_y() < 0;
+    return ShouldShowSystemBubbleBasedOnPosition();
+  }
+}
+
 void SystemTray::CloseSystemBubbleAndDeactivateSystemTray() {
   activation_observer_.reset();
   key_event_watcher_.reset();