Make mash register initial batch of accelerators in single shot.

ui/base/accelerators/accelerator_manager_unittest.cc

 // Copyright (c) 2012 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/base/accelerators/accelerator_manager.h"
 
 #include <map>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ skipping 53 matching lines @@
   int result = 0;
   for (size_t i = 0; i < arraysize(kAcceleratorModifiers); ++i) {
     if (((1 << i) & id) != 0)
       result |= kAcceleratorModifiers[i];
   }
   return result;
 }
 
 // AcceleratorManagerDelegate implementation that records calls to interface
 // using the following format.
-// . OnAcceleratorRegistered() -> 'Register ' + id
-// . OnAcceleratorRegistered() -> 'Unregister' + id
+// . OnAcceleratorUnregistered() -> 'Unregister' + id
+// . OnAcceleratorVectorRegistered() -> 'Register a list of ' + ids

[mfomitchev, 2017/01/26 21:39:49]

This doesn't seem to match what's implemented in OnAcceleratorVectorRegistered.

We should also either change the existing "Register" test to register multiple
accelerators at once, or add a new test for that. Currently only "Unregister"
tests adding multiple accelerators at once, which doesn't make a lot of sense.

[thanhph1, 2017/01/26 22:24:05]

Done. I modified "Register" test to add a vector of 4 accelerators. The
unregister accelerators is already done in "Unregister" test so I don't
duplicate here.

 // where the id is specified using SetIdForAccelerator().
 class TestAcceleratorManagerDelegate : public AcceleratorManagerDelegate {
  public:
   TestAcceleratorManagerDelegate() {}
   ~TestAcceleratorManagerDelegate() override {}
 
   void SetIdForAccelerator(const Accelerator& accelerator,
                            const std::string& id) {
     accelerator_to_id_[accelerator] = id;
   }
 
   std::string GetAndClearCommands() {
     std::string commands;
     std::swap(commands, commands_);
     return commands;
   }
 
   // AcceleratorManagerDelegate:
-  void OnAcceleratorRegistered(const Accelerator& accelerator) override {
-    if (!commands_.empty())
-      commands_ += " ";
-    commands_ += "Register " + accelerator_to_id_[accelerator];
-  }
   void OnAcceleratorUnregistered(const Accelerator& accelerator) override {
     if (!commands_.empty())
       commands_ += " ";
     commands_ += "Unregister " + accelerator_to_id_[accelerator];
   }
 
+  void OnAcceleratorVectorRegistered(
+      const std::vector<ui::Accelerator>& accelerators) override {
+    for (auto iter = accelerators.begin(); iter != accelerators.end(); ++iter) {
+      if (!commands_.empty())
+        commands_ += " ";
+      commands_ += "Register " + accelerator_to_id_[*iter];
+    }
+  }
+
  private:
   std::map<Accelerator, std::string> accelerator_to_id_;
   std::string commands_;
 
   DISALLOW_COPY_AND_ASSIGN(TestAcceleratorManagerDelegate);
 };
 
 }  // namespace
 
 class AcceleratorManagerTest : public testing::Test {
  public:
   AcceleratorManagerTest() : manager_(&delegate_) {}
   ~AcceleratorManagerTest() override {}
 
  protected:
   TestAcceleratorManagerDelegate delegate_;
   AcceleratorManager manager_;
 };
 
 TEST_F(AcceleratorManagerTest, Register) {
   const Accelerator accelerator_a(VKEY_A, EF_NONE);
   TestTarget target;
   delegate_.SetIdForAccelerator(accelerator_a, "a");
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
+  manager_.Register({accelerator_a}, AcceleratorManager::kNormalPriority,
                     &target);
   EXPECT_EQ("Register a", delegate_.GetAndClearCommands());
 
   // The registered accelerator is processed.
   EXPECT_TRUE(manager_.Process(accelerator_a));
   EXPECT_EQ(1, target.accelerator_pressed_count());
 }
 
 TEST_F(AcceleratorManagerTest, RegisterMultipleTarget) {
   const Accelerator accelerator_a(VKEY_A, EF_NONE);
   delegate_.SetIdForAccelerator(accelerator_a, "a");
   TestTarget target1;
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
+  manager_.Register({accelerator_a}, AcceleratorManager::kNormalPriority,
                     &target1);
   EXPECT_EQ("Register a", delegate_.GetAndClearCommands());
   TestTarget target2;
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
+  manager_.Register({accelerator_a}, AcceleratorManager::kNormalPriority,
                     &target2);
   // Registering the same command shouldn't notify the delegate.
   EXPECT_TRUE(delegate_.GetAndClearCommands().empty());
 
   // If multiple targets are registered with the same accelerator, the target
   // registered later processes the accelerator.
   EXPECT_TRUE(manager_.Process(accelerator_a));
   EXPECT_EQ(0, target1.accelerator_pressed_count());
   EXPECT_EQ(1, target2.accelerator_pressed_count());
 }
 
 TEST_F(AcceleratorManagerTest, Unregister) {
   const Accelerator accelerator_a(VKEY_A, EF_NONE);
   delegate_.SetIdForAccelerator(accelerator_a, "a");
   TestTarget target;
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
-                    &target);
-  EXPECT_EQ("Register a", delegate_.GetAndClearCommands());
+
   const Accelerator accelerator_b(VKEY_B, EF_NONE);
   delegate_.SetIdForAccelerator(accelerator_b, "b");
-  manager_.Register(accelerator_b, AcceleratorManager::kNormalPriority,
-                    &target);
-  EXPECT_EQ("Register b", delegate_.GetAndClearCommands());
+
+  manager_.Register({accelerator_a, accelerator_b},
+                    AcceleratorManager::kNormalPriority, &target);
+
+  EXPECT_EQ("Register a Register b", delegate_.GetAndClearCommands());
 
   // Unregistering a different accelerator does not affect the other
   // accelerator.
   manager_.Unregister(accelerator_b, &target);
   EXPECT_EQ("Unregister b", delegate_.GetAndClearCommands());
   EXPECT_TRUE(manager_.Process(accelerator_a));
   EXPECT_EQ(1, target.accelerator_pressed_count());
 
   // The unregistered accelerator is no longer processed.
   target.set_accelerator_pressed_count(0);
   manager_.Unregister(accelerator_a, &target);
   EXPECT_EQ("Unregister a", delegate_.GetAndClearCommands());
   EXPECT_FALSE(manager_.Process(accelerator_a));
   EXPECT_EQ(0, target.accelerator_pressed_count());
 }
 
 TEST_F(AcceleratorManagerTest, UnregisterAll) {
   const Accelerator accelerator_a(VKEY_A, EF_NONE);
   delegate_.SetIdForAccelerator(accelerator_a, "a");
   TestTarget target1;
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
-                    &target1);
   const Accelerator accelerator_b(VKEY_B, EF_NONE);
   delegate_.SetIdForAccelerator(accelerator_b, "b");
-  manager_.Register(accelerator_b, AcceleratorManager::kNormalPriority,
-                    &target1);
+  manager_.Register({accelerator_a, accelerator_b},
+                    AcceleratorManager::kNormalPriority, &target1);
+
   const Accelerator accelerator_c(VKEY_C, EF_NONE);
   delegate_.SetIdForAccelerator(accelerator_c, "c");
   TestTarget target2;
-  manager_.Register(accelerator_c, AcceleratorManager::kNormalPriority,
+  manager_.Register({accelerator_c}, AcceleratorManager::kNormalPriority,
                     &target2);
   EXPECT_EQ("Register a Register b Register c",
             delegate_.GetAndClearCommands());
   manager_.UnregisterAll(&target1);
   {
     const std::string commands = delegate_.GetAndClearCommands();
     // Ordering is not guaranteed.
     EXPECT_TRUE(commands == "Unregister a Unregister b" ||
                 commands == "Unregister b Unregister a");
   }
 
   // All the accelerators registered for |target1| are no longer processed.
   EXPECT_FALSE(manager_.Process(accelerator_a));
   EXPECT_FALSE(manager_.Process(accelerator_b));
   EXPECT_EQ(0, target1.accelerator_pressed_count());
 
   // UnregisterAll with a different target does not affect the other target.
   EXPECT_TRUE(manager_.Process(accelerator_c));
   EXPECT_EQ(1, target2.accelerator_pressed_count());
 }
 
 TEST_F(AcceleratorManagerTest, Process) {
   TestTarget target;
 
   // Test all cases of possible modifiers.
   for (size_t i = 0; i < (1 << arraysize(kAcceleratorModifiers)); ++i) {
     const int modifiers = BuildAcceleratorModifier(i);
     Accelerator accelerator(GetAccelerator(VKEY_A, modifiers));
-    manager_.Register(accelerator, AcceleratorManager::kNormalPriority,
+    manager_.Register({accelerator}, AcceleratorManager::kNormalPriority,
                       &target);
 
     // The registered accelerator is processed.
     const int last_count = target.accelerator_pressed_count();
     EXPECT_TRUE(manager_.Process(accelerator)) << i;
     EXPECT_EQ(last_count + 1, target.accelerator_pressed_count()) << i;
 
     // The non-registered accelerators are not processed.
     accelerator.set_type(ET_UNKNOWN);
     EXPECT_FALSE(manager_.Process(accelerator)) << i;  // different type
@@ skipping 22 matching lines @@
     manager_.UnregisterAll(&target);
   }
 }
 
 // Verifies delegate is notifed correctly when unregistering and registering
 // with the same accelerator.
 TEST_F(AcceleratorManagerTest, Reregister) {
   const Accelerator accelerator_a(VKEY_A, EF_NONE);
   TestTarget target;
   delegate_.SetIdForAccelerator(accelerator_a, "a");
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
+  manager_.Register({accelerator_a}, AcceleratorManager::kNormalPriority,
                     &target);
   EXPECT_EQ("Register a", delegate_.GetAndClearCommands());
   manager_.UnregisterAll(&target);
   EXPECT_EQ("Unregister a", delegate_.GetAndClearCommands());
-  manager_.Register(accelerator_a, AcceleratorManager::kNormalPriority,
+  manager_.Register({accelerator_a}, AcceleratorManager::kNormalPriority,
                     &target);
   EXPECT_EQ("Register a", delegate_.GetAndClearCommands());
 }
 
 }  // namespace test
 }  // namespace ui