Make mash register initial batch of accelerators in single shot.

ash/mus/accelerators/accelerator_controller_registrar.cc

 // Copyright 2016 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 "ash/mus/accelerators/accelerator_controller_registrar.h"
 
 #include <limits>
 
 #include "ash/common/accelerators/accelerator_controller.h"
 #include "ash/common/accelerators/accelerator_router.h"
@@ skipping 10 matching lines @@
 namespace {
 
 // Callback from registering the accelerator.
 void OnAcceleratorAdded(const ui::Accelerator& accelerator, bool added) {
   // All our accelerators should be registered, so we expect |added| to be true.
   DCHECK(added) << "duplicate accelerator key_code=" << accelerator.key_code()
                 << " type=" << accelerator.type()
                 << " modifiers=" << accelerator.modifiers();
 }
 
+// Callback from registering the accelerators.
+void OnAcceleratorsAdded(const std::vector<ui::Accelerator>& ui_accelerators,
+                         bool added) {
+  for (auto iter = ui_accelerators.begin(); iter != ui_accelerators.end();
+       ++iter)
+    OnAcceleratorAdded(*iter, added);
 }  // namespace
+}
 
 AcceleratorControllerRegistrar::AcceleratorControllerRegistrar(
     WindowManager* window_manager,
     uint16_t id_namespace)
     : window_manager_(window_manager),
       id_namespace_(id_namespace),
       next_id_(0),
       router_(new AcceleratorRouter) {
   window_manager_->AddAcceleratorHandler(id_namespace, this);
 }
@@ skipping 50 matching lines @@
                ? ui::mojom::EventResult::HANDLED
                : ui::mojom::EventResult::UNHANDLED;
   }
   DCHECK_EQ(GetAcceleratorLocalId(id), ids.post_id);
   // NOTE: for post return value doesn't really matter.
   return WmShell::Get()->accelerator_controller()->Process(accelerator)
              ? ui::mojom::EventResult::HANDLED
              : ui::mojom::EventResult::UNHANDLED;
 }
 
+// add_accelerator_immediately default set to true.
 void AcceleratorControllerRegistrar::OnAcceleratorRegistered(
     const ui::Accelerator& accelerator) {
-  Ids ids;
-  if (!GenerateIds(&ids)) {
-    DVLOG(1) << "max number of accelerators registered, dropping request";
-    return;
-  }
-  DCHECK_EQ(0u, accelerator_to_ids_.count(accelerator));
-  accelerator_to_ids_[accelerator] = ids;
-  DCHECK_EQ(accelerator_to_ids_.size() * 2, ids_.size());
+  std::vector<ui::mojom::AcceleratorPtr> accelerator_vector;
 
-  ui::mojom::EventMatcherPtr event_matcher = ui::CreateKeyMatcher(
-      static_cast<ui::mojom::KeyboardCode>(accelerator.key_code()),
-      accelerator.modifiers());
-  event_matcher->accelerator_phase = ui::mojom::AcceleratorPhase::PRE_TARGET;
-  DCHECK(accelerator.type() == ui::ET_KEY_PRESSED ||
-         accelerator.type() == ui::ET_KEY_RELEASED);
-  event_matcher->type_matcher->type = accelerator.type() == ui::ET_KEY_PRESSED
-                                          ? ui::mojom::EventType::KEY_PRESSED
-                                          : ui::mojom::EventType::KEY_RELEASED;
-
-  ui::mojom::EventMatcherPtr post_event_matcher = event_matcher.Clone();
-  post_event_matcher->accelerator_phase =
-      ui::mojom::AcceleratorPhase::POST_TARGET;
+  AddAcceleratorToVector(accelerator, accelerator_vector);
 
   window_manager_->window_manager_client()->AddAccelerators(
-      ui::CreateAcceleratorVector(
-          ComputeAcceleratorId(id_namespace_, ids.pre_id),
-          std::move(event_matcher)),
-      base::Bind(OnAcceleratorAdded, accelerator));
-
-  window_manager_->window_manager_client()->AddAccelerators(
-      ui::CreateAcceleratorVector(
-          ComputeAcceleratorId(id_namespace_, ids.post_id),
-          std::move(post_event_matcher)),
+      std::move(accelerator_vector),

[sadrul, 2017/01/11 17:55:13]

It looks like you send at most two accelerators at a time here, right? So we are
not really using a single IPC to register all accelerators. For X accelerators
registered in ash, we are still sending X IPC (each IPC carrying two
accelerators). That is my understanding of this code. Is this correct? Because I
don't think this is what we want.

[thanhph1, 2017/01/11 18:32:22]

This current cl (hopefully) does what you mention here.

This function OnAcceleratorRegistered is used for registering single accelerator
only. For multiple accelerator registration, OnAcceleratorsRegistered will be
used.

This function call Register
(https://cs.chromium.org/chromium/src/ash/common/accelerators/accelerator_cont...
) has been moved outside of the loop to register a whole vector instead of
single accelerator in every loop.

       base::Bind(OnAcceleratorAdded, accelerator));
 }
 
 void AcceleratorControllerRegistrar::OnAcceleratorUnregistered(
     const ui::Accelerator& accelerator) {
   auto iter = accelerator_to_ids_.find(accelerator);
   DCHECK(iter != accelerator_to_ids_.end());
   Ids ids = iter->second;
   accelerator_to_ids_.erase(iter);
   ids_.erase(ids.pre_id);
   ids_.erase(ids.post_id);
   DCHECK_EQ(accelerator_to_ids_.size() * 2, ids_.size());
   window_manager_->window_manager_client()->RemoveAccelerator(
       ComputeAcceleratorId(id_namespace_, ids.pre_id));
   window_manager_->window_manager_client()->RemoveAccelerator(
       ComputeAcceleratorId(id_namespace_, ids.post_id));
 }
 
+void AcceleratorControllerRegistrar::OnAcceleratorsRegistered(
+    const std::vector<ui::Accelerator>& accelerators) {
+  std::vector<ui::mojom::AcceleratorPtr> accelerator_vector;
+
+  for (auto iter = accelerators.begin(); iter != accelerators.end(); ++iter)
+    AddAcceleratorToVector(*iter, accelerator_vector);
+
+  window_manager_->window_manager_client()->AddAccelerators(
+      std::move(accelerator_vector),
+      base::Bind(OnAcceleratorsAdded, accelerators));
+}
+
+void AcceleratorControllerRegistrar::AddAcceleratorToVector(
+    const ui::Accelerator& accelerator,
+    std::vector<ui::mojom::AcceleratorPtr>& accelerator_vector) {
+  Ids ids;
+  if (!GenerateIds(&ids)) {
+    DVLOG(1) << "max number of accelerators registered, dropping request";
+    return;
+  }
+  DCHECK_EQ(0u, accelerator_to_ids_.count(accelerator));
+  accelerator_to_ids_[accelerator] = ids;
+  DCHECK_EQ(accelerator_to_ids_.size() * 2, ids_.size());
+
+  ui::mojom::EventMatcherPtr pre_event_matcher = ui::CreateKeyMatcher(
+      static_cast<ui::mojom::KeyboardCode>(accelerator.key_code()),
+      accelerator.modifiers());
+  pre_event_matcher->accelerator_phase =
+      ui::mojom::AcceleratorPhase::PRE_TARGET;
+  DCHECK(accelerator.type() == ui::ET_KEY_PRESSED ||
+         accelerator.type() == ui::ET_KEY_RELEASED);
+  pre_event_matcher->type_matcher->type =
+      accelerator.type() == ui::ET_KEY_PRESSED
+          ? ui::mojom::EventType::KEY_PRESSED
+          : ui::mojom::EventType::KEY_RELEASED;
+
+  ui::mojom::EventMatcherPtr post_event_matcher = pre_event_matcher.Clone();
+  post_event_matcher->accelerator_phase =
+      ui::mojom::AcceleratorPhase::POST_TARGET;
+
+  accelerator_vector.push_back(
+      ui::CreateAccelerator(ComputeAcceleratorId(id_namespace_, ids.pre_id),
+                            std::move(pre_event_matcher)));
+
+  accelerator_vector.push_back(
+      ui::CreateAccelerator(ComputeAcceleratorId(id_namespace_, ids.post_id),
+                            std::move(post_event_matcher)));
+}
+
 bool AcceleratorControllerRegistrar::GenerateIds(Ids* ids) {
   if (ids_.size() + 2 >= std::numeric_limits<uint16_t>::max())
     return false;
   ids->pre_id = GetNextLocalAcceleratorId();
   ids->post_id = GetNextLocalAcceleratorId();
   return true;
 }
 
 uint16_t AcceleratorControllerRegistrar::GetNextLocalAcceleratorId() {
   DCHECK_LT(ids_.size(), std::numeric_limits<uint16_t>::max());
   // Common case is we never wrap once, so this is typically cheap. Additionally
   // we expect there not to be too many accelerators.
   while (ids_.count(next_id_) > 0)
     ++next_id_;
   ids_.insert(next_id_);
   return next_id_++;
 }
 
 }  // namespace mus
 }  // namespace ash