[MacViews] Show combobox menu popup at mouse press

ui/views/controls/combobox/combobox_unittest.cc

 // Copyright 2013 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 "ui/views/controls/combobox/combobox.h"
 
 #include <set>
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 215 matching lines @@
     FocusManager* focus_manager = widget_->GetFocusManager();
     widget_->OnKeyEvent(&event);
     if (!event.handled() && focus_manager)
       focus_manager->OnKeyEvent(event);
   }
 
   View* GetFocusedView() {
     return widget_->GetFocusManager()->GetFocusedView();
   }
 
-  void PerformClick(const gfx::Point& point) {
+  void PerformMousePress(const gfx::Point& point) {
     ui::MouseEvent pressed_event = ui::MouseEvent(
         ui::ET_MOUSE_PRESSED, point, point, ui::EventTimeForNow(),
         ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
     widget_->OnMouseEvent(&pressed_event);
+  }
+
+  void PerformMouseRelease(const gfx::Point& point) {
     ui::MouseEvent released_event = ui::MouseEvent(
         ui::ET_MOUSE_RELEASED, point, point, ui::EventTimeForNow(),
         ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
     widget_->OnMouseEvent(&released_event);
   }
 
+  void PerformClick(const gfx::Point& point) {
+    PerformMousePress(point);
+    PerformMouseRelease(point);
+  }
+
   // We need widget to populate wrapper class.
   Widget* widget_;
 
   // |combobox_| will be allocated InitCombobox() and then owned by |widget_|.
   TestCombobox* combobox_;
   std::unique_ptr<ComboboxTestApi> test_api_;
 
   // Combobox does not take ownership of the model, hence it needs to be scoped.
   std::unique_ptr<TestComboboxModel> model_;
 
@@ skipping 322 matching lines @@
   InitCombobox(nullptr, Combobox::STYLE_ACTION);
 
   TestComboboxListener listener;
   combobox_->set_listener(&listener);
 
   combobox_->Layout();
 
   // Click the right side (arrow button). The menu is shown.
   int menu_show_count = 0;
   test_api_->InstallTestMenuRunner(&menu_show_count);
+  gfx::Point right_point(combobox_->x() + combobox_->width() - 1,

[tapted, 2016/06/29 12:02:55]

nit: const gfx::Point right_point

[spqchan, 2016/06/29 18:29:14]

Done.

+                         combobox_->y() + combobox_->height() / 2);
+
   EXPECT_EQ(0, menu_show_count);
-  PerformClick(gfx::Point(combobox_->x() + combobox_->width() - 1,
-                          combobox_->y() + combobox_->height() / 2));
+
+// Since the menu appears on a mouse press on Mac, we need to check between
+// the mouse press and release on that platform.
+#if defined(OS_MACOSX)

[tapted, 2016/06/29 12:02:55]

I'd structure this something like the following. This way we do the same thing
on all platforms, just have different expectations.

// On Mac, actions occur on mouse down. Otherwise mouse up.
#if defined(OS_MACOSX)
const int kActOnMouseDown = 1;
#else
const int kActOnMouseDown = 0;
#endif

PerformMousePress(right_point);
EXPECT_EQ(kActOnMouseDown, menu_show_count);
PerformMouseRelease(right_point);
EXPECT_EQ(1, menu_show_count);

...

PerformMousePress(left_point);
EXPECT_EQ(kActOnMouseDown, listener.actions_performed());
PerformMouseRelease(left_point);
EXPECT_EQ(1, listener.actions_performed());

[spqchan, 2016/06/29 18:29:14]

Done.

+  PerformMousePress(right_point);
   EXPECT_FALSE(listener.on_perform_action_called());
+  PerformMouseRelease(right_point);
+#else
+  PerformClick(right_point);
+  EXPECT_FALSE(listener.on_perform_action_called());
+#endif
+
   EXPECT_EQ(1, menu_show_count);
 
   // Click the left side (text button). The click event is notified.
+  gfx::Point left_point(

[tapted, 2016/06/29 12:02:55]

nit: const

[spqchan, 2016/06/29 18:29:14]

Done.

+      gfx::Point(combobox_->x() + 1, combobox_->y() + combobox_->height() / 2));
   test_api_->InstallTestMenuRunner(&menu_show_count);
-  PerformClick(gfx::Point(combobox_->x() + 1,
-                          combobox_->y() + combobox_->height() / 2));
+
+#if defined(OS_MACOSX)
+  PerformMousePress(left_point);
   EXPECT_TRUE(listener.on_perform_action_called());
+  PerformMouseRelease(left_point);
+#else
+  PerformClick(left_point);
+  EXPECT_TRUE(listener.on_perform_action_called());
+#endif
+
   EXPECT_EQ(1, menu_show_count);  // Unchanged.
   EXPECT_EQ(0, listener.perform_action_index());
 }
 
 TEST_F(ComboboxTest, ConsumingPressKeyEvents) {
   InitCombobox(nullptr, Combobox::STYLE_NORMAL);
 
   EXPECT_FALSE(combobox_->OnKeyPressed(
       ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_RETURN, ui::EF_NONE)));
   EXPECT_FALSE(combobox_->OnKeyPressed(
@@ skipping 181 matching lines @@
   EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu_model->GetTypeAt(0));
   EXPECT_EQ(ui::MenuModel::TYPE_COMMAND, menu_model->GetTypeAt(1));
 
   EXPECT_EQ(ASCIIToUTF16("PEANUT BUTTER"), menu_model->GetLabelAt(0));
   EXPECT_EQ(ASCIIToUTF16("JELLY"), menu_model->GetLabelAt(1));
   EXPECT_FALSE(menu_model->IsVisibleAt(0));
   EXPECT_TRUE(menu_model->IsVisibleAt(1));
 }
 
 }  // namespace views