WindowManagerClient::AddAccelerator() should take an array

services/ui/ws/event_dispatcher_unittest.cc

 // Copyright 2015 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 "services/ui/ws/event_dispatcher.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <queue>
 
 #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"
 #include "services/ui/ws/server_window_compositor_frame_sink_manager_test_api.h"
 #include "services/ui/ws/test_server_window_delegate.h"
 #include "services/ui/ws/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/event.h"
 
 namespace ui {
@@ skipping 353 matching lines @@
 }
 
 TEST_F(EventDispatcherTest, AcceleratorBasic) {
   ClearSetup();
   TestEventDispatcherDelegate event_dispatcher_delegate(nullptr);
   EventDispatcher dispatcher(&event_dispatcher_delegate);
 
   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, since you are relying on the
functionality tested in that test to work here.

[mfomitchev, 2016/12/01 15:32:36]

Can we call this test AddMultipleAccelerators or something similar to highlight
the fact we are testing adding a bunch at once?

[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 it works.

[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 think there's some connection
with accelerator_1, which is not the case.

[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, since these three lines are all
testing what's described in this comment.

[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 already tested this just above
with accelerator_4.

+
+  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) {
   TestEventDispatcherDelegate* event_dispatcher_delegate =
       test_event_dispatcher_delegate();
   EventDispatcher* dispatcher = event_dispatcher();
 
   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);
   EXPECT_EQ(accelerator_1,
             event_dispatcher_delegate->GetAndClearLastAccelerator());
 
   // EF_NUM_LOCK_ON should be ignored since CreateKeyMatcher defaults to
   // ignoring.
   key = ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_W,
                      ui::EF_CONTROL_DOWN | ui::EF_NUM_LOCK_ON);
   dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
   EXPECT_EQ(accelerator_1,
             event_dispatcher_delegate->GetAndClearLastAccelerator());
 
   key = ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_W, ui::EF_NONE);
   dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
   EXPECT_EQ(0u, event_dispatcher_delegate->GetAndClearLastAccelerator());
 
   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());
 
   dispatcher->RemoveAccelerator(accelerator_2);
   dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
   EXPECT_EQ(0u, event_dispatcher_delegate->GetAndClearLastAccelerator());
 }
 
 // Tests that a post-target accelerator is not triggered by ProcessEvent.
 TEST_F(EventDispatcherTest, PostTargetAccelerator) {
   TestEventDispatcherDelegate* event_dispatcher_delegate =
       test_event_dispatcher_delegate();
   EventDispatcher* dispatcher = event_dispatcher();
 
   mojom::EventMatcherPtr matcher = ui::CreateKeyMatcher(
       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.
   dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
   EXPECT_EQ(accelerator_1,
             event_dispatcher_delegate->GetAndClearLastAccelerator());
   std::unique_ptr<DispatchedEventDetails> details =
       event_dispatcher_delegate->GetAndAdvanceDispatchedEventDetails();
   EXPECT_FALSE(details);
 
@@ skipping 24 matching lines @@
 TEST_F(EventDispatcherTest, ProcessPost) {
   TestEventDispatcherDelegate* event_dispatcher_delegate =
       test_event_dispatcher_delegate();
   EventDispatcher* dispatcher = event_dispatcher();
 
   uint32_t pre_id = 1;
   {
     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;
   {
     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.
   std::unique_ptr<ServerWindow> child = CreateChildWindow(WindowId(1, 3));
   event_dispatcher_delegate->SetFocusedWindowFromEventDispatcher(child.get());
 
   // Dispatch for ANY, which should trigger PRE and not call
   // DispatchInputEventToWindow().
   ui::KeyEvent key(ui::ET_KEY_PRESSED, ui::VKEY_W, ui::EF_CONTROL_DOWN);
   dispatcher->ProcessEvent(key, EventDispatcher::AcceleratorMatchPhase::ANY);
@@ skipping 1173 matching lines @@
   // The delegate can decide if it really wants to forward the event or not.
   EXPECT_EQ(child.get(),
             test_event_dispatcher_delegate()->lost_capture_window());
   EXPECT_EQ(child.get(), event_dispatcher()->capture_window());
   EXPECT_EQ(kClientAreaId, event_dispatcher()->capture_window_client_id());
 }
 
 }  // namespace test
 }  // namespace ws
 }  // namespace ui