WindowManagerClient::AddAccelerator() should take an array

services/ui/ws/event_dispatcher.h

 // 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.
 
 #ifndef SERVICES_UI_WS_EVENT_DISPATCHER_H_
 #define SERVICES_UI_WS_EVENT_DISPATCHER_H_
 
 #include <stdint.h>
 
 #include <map>
 #include <memory>
 #include <utility>
 
 #include "base/macros.h"
 #include "services/ui/common/types.h"
 #include "services/ui/public/interfaces/cursor.mojom.h"
-#include "services/ui/public/interfaces/event_matcher.mojom.h"
+#include "services/ui/public/interfaces/window_manager.mojom.h"
 #include "services/ui/ws/drag_cursor_updater.h"
 #include "services/ui/ws/modal_window_controller.h"
 #include "services/ui/ws/server_window_observer.h"
 #include "ui/gfx/geometry/rect_f.h"
 
 namespace ui {
 class Event;
 class KeyEvent;
 class LocatedEvent;
 
@@ skipping 87 matching lines @@
   // If the mouse cursor is still over |mouse_cursor_source_window_|, updates
   // whether we are in the non-client area. Used when
   // |mouse_cursor_source_window_| has changed its properties.
   void UpdateNonClientAreaForCurrentWindow();
 
   // Possibly updates the cursor. If we aren't in an implicit capture, we take
   // the last known location of the mouse pointer, and look for the
   // ServerWindow* under it.
   void UpdateCursorProviderByLastKnownLocation();
 
-  // Adds an accelerator with the given id and event-matcher. If an accelerator
-  // already exists with the same id or the same matcher, then the accelerator
-  // is not added. Returns whether adding the accelerator was successful or not.
-  bool AddAccelerator(uint32_t id, mojom::EventMatcherPtr event_matcher);
+  // Adds the supplied accelerators. If an accelerator with the same id or
+  // matcher already exists, skip it. Returns true if all accelerators were
+  // added successfully.
+  bool AddAccelerators(std::vector<ui::mojom::AcceleratorPtr> accelerators);

[sky, 2016/12/01 22:17:04]

I think this code would be easier if you lave this function as it was and have
the caller be responsible for calling multiple times. That way the caller can
decide how to handle failures.

[thanhph, 2016/12/01 23:32:42]

Nice, I put this AddAccelerator back. Thanks!

+
   void RemoveAccelerator(uint32_t id);
 
   // Processes the supplied event, informing the delegate as approriate. This
   // may result in generating any number of events. If |match_phase| is
   // ANY and there is a matching accelerator with PRE_TARGET found, than only
   // OnAccelerator() is called. The expectation is after the PRE_TARGET has been
   // handled this is again called with an AcceleratorMatchPhase of POST_ONLY.
   void ProcessEvent(const ui::Event& event, AcceleratorMatchPhase match_phase);
 
  private:
@@ skipping 132 matching lines @@
       AcceleratorMatchPhase::ANY;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(EventDispatcher);
 };
 
 }  // namespace ws
 }  // namespace ui
 
 #endif  // SERVICES_UI_WS_EVENT_DISPATCHER_H_