WindowManagerClient::AddAccelerator() should take an array

services/ui/public/interfaces/window_manager.mojom

 // 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.
 
 module ui.mojom;
 
 import "services/ui/public/interfaces/cursor.mojom";
 import "services/ui/public/interfaces/display/display.mojom";
 import "services/ui/public/interfaces/event_matcher.mojom";
 import "services/ui/public/interfaces/window_manager_constants.mojom";
@@ skipping 120 matching lines @@
 
   // Asks the WindowManager to cancel any outstanding move loop on |change_id|.
   WmCancelMoveLoop(uint32 change_id);
 
   // An accelerator registered via AddAccelerator() has been triggered. If
   // |ack_id| is non-zero the accelerator matches a PRE_TARGET and must be
   // acknowledged by WindowManagerClient::OnAcceleratorAck().
   OnAccelerator(uint32 ack_id, uint32 accelerator_id, ui.mojom.Event event);
 };
 
+struct Accelerator{

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

nit: space after Accelerator

[thanhph, 2016/12/01 19:03:19]

Done.

[mfomitchev, 2016/12/01 20:51:51]

No, not a blank line. Space. I.e. "Accelerator {"

+   uint32 id;
+   EventMatcher event_matcher;
+};
+
 // This interface is only used as an associated interface and is associated
 // with WindowTree.
 interface WindowManagerClient {
   // Enables (or disables) child windows of |window_id| to be activated.
   AddActivationParent(uint32 window_id);
   RemoveActivationParent(uint32 window_id);
 
   ActivateNextWindow();
 
   // Sets the underlay surface offset for the specified window and additional
   // hit area. The underlay surface is drawn at the bounds of the window minus
   // the offset. The hit area is extended from the bounds of the window by
   // |hit_area|.
   SetUnderlaySurfaceOffsetAndExtendedHitArea(uint32 window_id,
                                              int32 x_offset,
                                              int32 y_offset,
                                              gfx.mojom.Insets hit_area);
 
   // Add and remove accelerators. When accelerators are registered the
   // WindowManager receives the event via OnAccelerator() rather than the
   // target window. The id is defined by the client and can be used to more
   // easily identify the accelerator's action. If an accelerator with the same
-  // id or the same matcher already exists, then the accelerator is not added.
+  // id or matcher already exists, skip it.Returns true if all accelerators were

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

Nit: There's some stuff in this comment specific to Add, while most of it
applies to both Add and Remove, which makes the comment a little confusionl. Can
you please clarify which part applies to what. E.g. you can separate the
comments that are specific to AddAccelerators from the
comments that apply to both Add and Remove and put them right before
AddAccelerators method below.

[thanhph, 2016/12/01 19:03:19]

Done.

+  // added successfully.
   // Accelerator ids 1 << 31 and above are reserved for internal use.
   //
   // See WindowTree for details on event dispatch.
-  AddAccelerator(uint32 id, EventMatcher matcher) => (bool success);
+  AddAccelerators(array<Accelerator> accelerators) => (bool success);
   RemoveAccelerator(uint32 id);
 
   // The window manager has completed a request with the specific change id.
   WmResponse(uint32 change_id, bool response);
 
   // Calls WindowTreeClient::RequestClose() on the embedded app at the
   // specified window.
   WmRequestClose(uint32 window_id);
 
   // Sets the frame decoration constants of the display the window manager is
   // associated with.
   WmSetFrameDecorationValues(FrameDecorationValues values);
 
   // Sets the cursor that the non-client areas of the window should use.
   WmSetNonClientCursor(uint32 window_id, Cursor cursor_id);
 
   // Response from WmCreateTopLevelWindow() informing the client of the id for
   // the new window.
   OnWmCreatedTopLevelWindow(uint32 change_id, uint32 window_id);
 
   // See description in WindowManager::OnAccelerator(). |ack_id| is the value
   // that was passed to OnAccelerator().
   OnAcceleratorAck(uint32 ack_id, EventResult event_result);
 };