MenuController ignore touch if owner wants gesture

Gesture recognizer handles touches in two phases: event pre-dispatch
and post-dispatch. Closing MenuHost during dispatching skips the
post-dispatch phase because target (MenuHost) is destroyed. As
a result, the touch is never acked and gesture recognizer stops
generating any further gestures.

The above problem could be resolved either by asynchronously
closing the menu, or manually ack the touch event. The next problem
is that MenuController could not mark the touch event as handled.
Otherwise, gesture recognizer cancels the current sequence without
GESTURE_END event and breaks the owner's expectation.

If MenuController does not mark the touch as handled, gesture
recognizer would then translate the 2nd touch pressed into a
two-finger tap and causing the menu to flash because it is shown
again.

Given all that, a reasonable fix would be make MenuController to
skip touch events handling when the owner wants the current gesture
sequence. The problems would not happen if MenuHost is not closed
in middle of touch events dispatching. Side effect is that menu is
no longer closed on the 2nd figure down and could only be closed
after all figures are lifted, which is probably okay.

Bug: 874215
Test: MenuControllerTest.NoTouchCloseWhenSendingGesturesToOwner
Change-Id: I1818b6c35c80006c999f96ddbea38fd1f79948c3
Reviewed-on: https://chromium-review.googlesource.com/1179250Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584519}

ui/views/controls/menu/menu_controller.cc

@@ -866,6 +866,10 @@ void MenuController::OnGestureEvent(SubmenuView* source,
 }
 
 void MenuController::OnTouchEvent(SubmenuView* source, ui::TouchEvent* event) {
+  // Bail if owner wants the current active gesture sequence.
+  if (owner_ && send_gesture_events_to_owner())
+    return;
+
   if (event->type() == ui::ET_TOUCH_PRESSED) {
     MenuPart part = GetMenuPart(source, event->location());
     if (part.type == MenuPart::NONE) {

ui/views/controls/menu/menu_controller_unittest.cc

@@ -1167,6 +1167,8 @@ TEST_F(MenuControllerTest, DoubleAsynchronousNested) {
   EXPECT_EQ(1, nested_delegate->on_menu_closed_called());
 }
 
+// Tests that setting send_gesture_events_to_owner flag forwards gesture events
+// to owner and the forwarding stops when the current gesture sequence ends.
 TEST_F(MenuControllerTest, PreserveGestureForOwner) {
   MenuController* controller = menu_controller();
   MenuItemView* item = menu_item();
@@ -1203,6 +1205,40 @@ TEST_F(MenuControllerTest, PreserveGestureForOwner) {
   EXPECT_EQ(CountOwnerOnGestureEvent(), 2);
 }
 
+// Tests that touch outside menu does not closes the menu when forwarding
+// gesture events to owner.
+TEST_F(MenuControllerTest, NoTouchCloseWhenSendingGesturesToOwner) {
+  MenuController* controller = menu_controller();
+
+  // Owner wants the gesture events.
+  controller->set_send_gesture_events_to_owner(true);
+
+  // Show a sub menu and touch outside of it.
+  MenuItemView* item = menu_item();
+  SubmenuView* sub_menu = item->GetSubmenu();
+  sub_menu->ShowAt(owner(), item->bounds(), false);
+  gfx::Point location(sub_menu->bounds().bottom_right());
+  location.Offset(1, 1);
+  ui::TouchEvent touch_event(
+      ui::ET_TOUCH_PRESSED, location, ui::EventTimeForNow(),
+      ui::PointerDetails(ui::EventPointerType::POINTER_TYPE_TOUCH, 0));
+  controller->OnTouchEvent(sub_menu, &touch_event);
+
+  // Menu should still be visible.
+  EXPECT_TRUE(IsShowing());
+
+  // The current gesture sequence ends.
+  ui::GestureEvent gesture_end_event(
+      location.x(), location.y(), 0, ui::EventTimeForNow(),
+      ui::GestureEventDetails(ui::ET_GESTURE_END));
+  controller->OnGestureEvent(sub_menu, &gesture_end_event);
+
+  // Touch outside again and menu should be closed.
+  controller->OnTouchEvent(sub_menu, &touch_event);
+  EXPECT_FALSE(IsShowing());
+  EXPECT_EQ(MenuController::EXIT_ALL, controller->exit_type());
+}
+
 // Tests that a nested menu does not crash when trying to repost events that
 // occur outside of the bounds of the menu. Instead a proper shutdown should
 // occur.