Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(503)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: services/ui/ws/event_dispatcher_unittest.cc

Issue 2520093003: WindowManagerClient::AddAccelerator() should take an array (Closed)
Patch Set: Adding 1 test that has two unique and one duped accelerators. Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« services/ui/public/interfaces/window_manager.mojom ('K') | « services/ui/ws/event_dispatcher.cc ('k') | services/ui/ws/window_manager_state.cc » ('j') | services/ui/ws/window_manager_state_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: services/ui/ws/event_dispatcher_unittest.cc
diff --git a/services/ui/ws/event_dispatcher_unittest.cc b/services/ui/ws/event_dispatcher_unittest.cc
index e06b70ef6fc6d71a862e065b0c894fc5eeec9635..70208d46e7474a5ee835dc59197569bead6fbb20 100644
--- a/services/ui/ws/event_dispatcher_unittest.cc
+++ b/services/ui/ws/event_dispatcher_unittest.cc
@@ -11,7 +11,7 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
-#include "services/ui/common/event_matcher_util.h"
+#include "services/ui/common/accelerator_util.h"
#include "services/ui/ws/accelerator.h"
#include "services/ui/ws/event_dispatcher_delegate.h"
#include "services/ui/ws/server_window.h"
@@ -385,34 +385,113 @@ TEST_F(EventDispatcherTest, AcceleratorBasic) {
uint32_t accelerator_1 = 1;
mojom::EventMatcherPtr matcher = ui::CreateKeyMatcher(
ui::mojom::KeyboardCode::W, ui::mojom::kEventFlagControlDown);
- EXPECT_TRUE(dispatcher.AddAccelerator(accelerator_1, std::move(matcher)));
+ EXPECT_TRUE(dispatcher.AddAccelerators(
+ CreateAcceleratorVector(accelerator_1, std::move(matcher))));
uint32_t accelerator_2 = 2;
matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::N,
ui::mojom::kEventFlagNone);
- EXPECT_TRUE(dispatcher.AddAccelerator(accelerator_2, std::move(matcher)));
+ EXPECT_TRUE(dispatcher.AddAccelerators(
+ CreateAcceleratorVector(accelerator_2, std::move(matcher))));
// Attempting to add a new accelerator with the same id should fail.
matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::T,
ui::mojom::kEventFlagNone);
- EXPECT_FALSE(dispatcher.AddAccelerator(accelerator_2, std::move(matcher)));
+ EXPECT_FALSE(dispatcher.AddAccelerators(
+ CreateAcceleratorVector(accelerator_2, std::move(matcher))));
// Adding the accelerator with the same id should succeed once the existing
// accelerator is removed.
dispatcher.RemoveAccelerator(accelerator_2);
matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::T,
ui::mojom::kEventFlagNone);
- EXPECT_TRUE(dispatcher.AddAccelerator(accelerator_2, std::move(matcher)));
+ EXPECT_TRUE(dispatcher.AddAccelerators(
+ CreateAcceleratorVector(accelerator_2, std::move(matcher))));
// Attempting to add an accelerator with the same matcher should fail.
uint32_t accelerator_3 = 3;
matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::T,
ui::mojom::kEventFlagNone);
- EXPECT_FALSE(dispatcher.AddAccelerator(accelerator_3, std::move(matcher)));
+ EXPECT_FALSE(dispatcher.AddAccelerators(
+ CreateAcceleratorVector(accelerator_3, std::move(matcher))));
matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::T,
ui::mojom::kEventFlagControlDown);
- EXPECT_TRUE(dispatcher.AddAccelerator(accelerator_3, std::move(matcher)));
+ EXPECT_TRUE(dispatcher.AddAccelerators(
+ CreateAcceleratorVector(accelerator_3, std::move(matcher))));
+}
+
+TEST_F(EventDispatcherTest, AddAccelerators) {
mfomitchev 2016/12/01 15:32:36 nit: consider putting this after EventMatching, si
mfomitchev 2016/12/01 15:32:36 Can we call this test AddMultipleAccelerators or s
thanhph 2016/12/01 19:03:19 Done, thanks!
+ ClearSetup();
+ TestEventDispatcherDelegate event_dispatcher_delegate(nullptr);
+ EventDispatcher dispatcher(&event_dispatcher_delegate);
+
+ std::vector<ui::mojom::AcceleratorPtr> accelerators;
+
+ uint32_t accelerator_1 = 1;
+ mojom::EventMatcherPtr matcher = ui::CreateKeyMatcher(
+ ui::mojom::KeyboardCode::W, ui::mojom::kEventFlagControlDown);
+ accelerators.push_back(CreateAccelerator(accelerator_1, std::move(matcher)));
+
+ uint32_t accelerator_2 = 2;
+ matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::N,
+ ui::mojom::kEventFlagNone);
+ accelerators.push_back(CreateAccelerator(accelerator_2, std::move(matcher)));
+
+ uint32_t accelerator_3 = 3;
+ matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::T,
+ ui::mojom::kEventFlagNone);
+ accelerators.push_back(CreateAccelerator(accelerator_3, std::move(matcher)));
+
+ // Adding unique accelerators should pass.
+ EXPECT_TRUE(dispatcher.AddAccelerators(std::move(accelerators)));
mfomitchev 2016/12/01 15:32:36 Add a test for accelerator_3 below confirming that
thanhph 2016/12/01 19:03:19 Done.
+
+ accelerators.clear();
+
+ uint32_t accelerator_4 = 4;
+ matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::W,
+ ui::mojom::kEventFlagControlDown);
+ accelerators.push_back(CreateAccelerator(accelerator_4, std::move(matcher)));
+
+ uint32_t accelerator_6 = 6;
+ matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::Q,
+ ui::mojom::kEventFlagControlDown);
+ accelerators.push_back(CreateAccelerator(accelerator_6, std::move(matcher)));
+
+ uint32_t accelerator_7 = 7;
+ matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::R,
+ ui::mojom::kEventFlagControlDown);
+ accelerators.push_back(CreateAccelerator(accelerator_7, std::move(matcher)));
+
+ // Adding accelerator_4 with the same matcher as accelerator_1 should fail
+ // while accelerator_6 and accelerator_7 should be added.
+ EXPECT_FALSE(dispatcher.AddAccelerators(std::move(accelerators)));
+
+ ui::KeyEvent key_1(ui::ET_KEY_PRESSED, ui::VKEY_R, ui::EF_CONTROL_DOWN);
mfomitchev 2016/12/01 15:32:37 nit: Just call this key, otherwise it makes one th
thanhph 2016/12/01 19:03:19 Done.
+ dispatcher.ProcessEvent(key_1, EventDispatcher::AcceleratorMatchPhase::ANY);
+
+ // Check if the accelerator_7 was still added regardless of failling
mfomitchev 2016/12/01 15:32:37 nit: move this comment about key_1 declaration, si
thanhph 2016/12/01 19:03:19 Done.
+ // adding attempt of accelerator_4 in the accelerators vector.
+ EXPECT_EQ(accelerator_7,
+ event_dispatcher_delegate.GetAndClearLastAccelerator());
+
+ ui::KeyEvent key_2(ui::ET_KEY_PRESSED, ui::VKEY_W, ui::EF_CONTROL_DOWN);
mfomitchev 2016/12/01 15:32:36 ditto: key
thanhph 2016/12/01 19:03:19 Done.
+ dispatcher.ProcessEvent(key_2, EventDispatcher::AcceleratorMatchPhase::ANY);
+
+ // Check if the accelerator_1 id hasn't changed because of accelerator_4
mfomitchev 2016/12/01 15:32:37 ditto: move
thanhph 2016/12/01 19:03:19 Done.
+ // adding attempt above.
+ EXPECT_EQ(accelerator_1,
+ event_dispatcher_delegate.GetAndClearLastAccelerator());
+
+ accelerators.clear();
mfomitchev 2016/12/01 15:32:36 I don't think we need the test below - you've alre
+
+ uint32_t accelerator_5 = accelerator_1;
+ matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::M,
+ ui::mojom::kEventFlagNone);
+ accelerators.push_back(CreateAccelerator(accelerator_5, std::move(matcher)));
+
+ // Adding accelerator_5 with the same id as accelerator_1 should fail.
+ EXPECT_FALSE(dispatcher.AddAccelerators(std::move(accelerators)));
}
TEST_F(EventDispatcherTest, EventMatching) {
@@ -423,7 +502,8 @@ TEST_F(EventDispatcherTest, EventMatching) {
mojom::EventMatcherPtr matcher = ui::CreateKeyMatcher(
ui::mojom::KeyboardCode::W, ui::mojom::kEventFlagControlDown);
uint32_t accelerator_1 = 1;
- dispatcher->AddAccelerator(accelerator_1, std::move(matcher));
+ dispatcher->AddAccelerators(
+ CreateAcceleratorVector(accelerator_1, std::move(matcher)));
ui::KeyEvent key(ui::ET_KEY_PRESSED, ui::VKEY_W, ui::EF_CONTROL_DOWN);
dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
@@ -445,7 +525,8 @@ TEST_F(EventDispatcherTest, EventMatching) {
uint32_t accelerator_2 = 2;
matcher = ui::CreateKeyMatcher(ui::mojom::KeyboardCode::W,
ui::mojom::kEventFlagNone);
- dispatcher->AddAccelerator(accelerator_2, std::move(matcher));
+ dispatcher->AddAccelerators(
+ CreateAcceleratorVector(accelerator_2, std::move(matcher)));
dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
EXPECT_EQ(accelerator_2,
event_dispatcher_delegate->GetAndClearLastAccelerator());
@@ -465,7 +546,8 @@ TEST_F(EventDispatcherTest, PostTargetAccelerator) {
ui::mojom::KeyboardCode::W, ui::mojom::kEventFlagControlDown);
matcher->accelerator_phase = ui::mojom::AcceleratorPhase::POST_TARGET;
uint32_t accelerator_1 = 1;
- dispatcher->AddAccelerator(accelerator_1, std::move(matcher));
+ dispatcher->AddAccelerators(
+ CreateAcceleratorVector(accelerator_1, std::move(matcher)));
ui::KeyEvent key(ui::ET_KEY_PRESSED, ui::VKEY_W, ui::EF_CONTROL_DOWN);
// The post-target accelerator should be fired if there is no focused window.
@@ -510,7 +592,8 @@ TEST_F(EventDispatcherTest, ProcessPost) {
mojom::EventMatcherPtr matcher = ui::CreateKeyMatcher(
ui::mojom::KeyboardCode::W, ui::mojom::kEventFlagControlDown);
matcher->accelerator_phase = ui::mojom::AcceleratorPhase::PRE_TARGET;
- dispatcher->AddAccelerator(pre_id, std::move(matcher));
+ dispatcher->AddAccelerators(
+ CreateAcceleratorVector(pre_id, std::move(matcher)));
}
uint32_t post_id = 2;
@@ -518,7 +601,8 @@ TEST_F(EventDispatcherTest, ProcessPost) {
mojom::EventMatcherPtr matcher = ui::CreateKeyMatcher(
ui::mojom::KeyboardCode::W, ui::mojom::kEventFlagControlDown);
matcher->accelerator_phase = ui::mojom::AcceleratorPhase::POST_TARGET;
- dispatcher->AddAccelerator(post_id, std::move(matcher));
+ dispatcher->AddAccelerators(
+ CreateAcceleratorVector(post_id, std::move(matcher)));
}
// Set focused window for EventDispatcher dispatches key events.
« services/ui/public/interfaces/window_manager.mojom ('K') | « services/ui/ws/event_dispatcher.cc ('k') | services/ui/ws/window_manager_state.cc » ('j') | services/ui/ws/window_manager_state_unittest.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698