mash: Have chrome set itself as a controller interface for changing volume

ash/common/accelerators/accelerator_controller.h

 // 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.
 
 #ifndef ASH_COMMON_ACCELERATORS_ACCELERATOR_CONTROLLER_H_
 #define ASH_COMMON_ACCELERATORS_ACCELERATOR_CONTROLLER_H_
 
 #include <stddef.h>
 
 #include <map>
 #include <memory>
 #include <set>
 
 #include "ash/ash_export.h"
 #include "ash/common/accelerators/accelerator_table.h"
 #include "ash/common/accelerators/exit_warning_handler.h"
 #include "ash/public/interfaces/volume.mojom.h"
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/accelerator_history.h"
 
 namespace ui {
 class AcceleratorManager;
 class AcceleratorManagerDelegate;
 }
 
 namespace ash {
 
 struct AcceleratorData;
 class AcceleratorControllerDelegate;
 class ExitWarningHandler;
 class ImeControlDelegate;
 
 // AcceleratorController provides functions for registering or unregistering
 // global keyboard accelerators, which are handled earlier than any windows. It
-// also implements several handlers as an accelerator target.
-class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget {
+// also implements several handlers as an accelerator target. Implements
+// mojom::Volume so that Chrome can set itself as the mojom::VolumeClient,
+// which is only used by accelerator keys.
+class ASH_EXPORT AcceleratorController
+    : public ui::AcceleratorTarget,
+      NON_EXPORTED_BASE(public mojom::Volume) {

[msw, 2016/12/03 00:26:43]

Inheriting a Volume-specific interface here is somewhat odd. Consider instead
adding mojom:Accelerator[Controller] with SetVolumeClient(mojom::VolumeClientPtr
client). Renaming the interface doesn't commit us to anything more, it just
reflects the implementor. It'll be a little odd for Chrome to tell ash's
accelerator controller "here's your volume client", but it can all be scrapped
when we deprecate chrome's CrasAudioHandler/dbus use. WDYT?

[James Cook, 2016/12/06 19:23:57]

Done.

  public:
   AcceleratorController(AcceleratorControllerDelegate* delegate,
                         ui::AcceleratorManagerDelegate* manager_delegate);
   ~AcceleratorController() override;
 
   // A list of possible ways in which an accelerator should be restricted before
   // processing. Any target registered with this controller should respect
   // restrictions by calling |GetCurrentAcceleratorRestriction| during
   // processing.
   enum AcceleratorProcessingRestriction {
@@ skipping 64 matching lines @@
       const ui::Accelerator& accelerator) const;
 
   ui::AcceleratorHistory* accelerator_history() {
     return accelerator_history_.get();
   }
 
   // Overridden from ui::AcceleratorTarget:
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
   bool CanHandleAccelerators() const override;
 
+  // Binds the mojom::Volume interface to this object.
+  void BindRequest(mojom::VolumeRequest request);
+
+  // mojom::Volume:
+  void SetClient(mojom::VolumeClientPtr client) override;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(AcceleratorControllerTest, GlobalAccelerators);
   FRIEND_TEST_ALL_PREFIXES(AcceleratorControllerTest,
                            DontRepeatToggleFullscreen);
   FRIEND_TEST_ALL_PREFIXES(DeprecatedAcceleratorTester,
                            TestDeprecatedAcceleratorsBehavior);
 
   // Initializes the accelerators this class handles as a target.
   void Init();
 
@@ skipping 15 matching lines @@
                      const ui::Accelerator& accelerator);
 
   // Returns whether performing |action| should consume the key event.
   bool ShouldActionConsumeKeyEvent(AcceleratorAction action);
 
   // Get the accelerator restriction for the given action. Supply an |action|
   // of -1 to get restrictions that apply for the current context.
   AcceleratorProcessingRestriction GetAcceleratorProcessingRestriction(
       int action);
 
-  // Returns the volume controller interface raw pointer, may be null in tests.
-  mojom::VolumeController* GetVolumeController();
-  void OnVolumeControllerConnectionError();
-
   AcceleratorControllerDelegate* delegate_;
 
   std::unique_ptr<ui::AcceleratorManager> accelerator_manager_;
 
   // A tracker for the current and previous accelerators.
   std::unique_ptr<ui::AcceleratorHistory> accelerator_history_;
 
   std::unique_ptr<ImeControlDelegate> ime_control_delegate_;
 
   // Handles the exit accelerator which requires a double press to exit and
   // shows a popup with an explanation.
   ExitWarningHandler exit_warning_handler_;
 
   // A map from accelerators to the AcceleratorAction values, which are used in
   // the implementation.
   std::map<ui::Accelerator, AcceleratorAction> accelerators_;
 
   std::map<AcceleratorAction, const DeprecatedAcceleratorData*>
       actions_with_deprecations_;
   std::set<ui::Accelerator> deprecated_accelerators_;
 
-  // The cached volume controller interface pointer.
-  mojom::VolumeControllerPtr volume_controller_;
+  // Bindings for the mojom::Volume interface.
+  mojo::BindingSet<mojom::Volume> bindings_;
+
+  // Client interface in chrome browser. May be null in tests.

[msw, 2016/12/05 20:15:09]

optional nit: Mention here that it's used for [CrasAudioHandler] volume control
functions? Changing 'controller' -> 'client' makes the purpose of this member
less clear to me (I think "Why does the accelerator controller need to access
some random client of the volume interface?").

[James Cook, 2016/12/06 19:23:57]

Done.

+  mojom::VolumeClientPtr volume_client_;
 
   // Actions allowed when the user is not signed in.
   std::set<int> actions_allowed_at_login_screen_;
   // Actions allowed when the screen is locked.
   std::set<int> actions_allowed_at_lock_screen_;
   // Actions allowed when a modal window is up.
   std::set<int> actions_allowed_at_modal_window_;
   // Preferred actions. See accelerator_table.h for details.
   std::set<int> preferred_actions_;
   // Reserved actions. See accelerator_table.h for details.
   std::set<int> reserved_actions_;
   // Actions which will be repeated while holding the accelerator key.
   std::set<int> repeatable_actions_;
   // Actions allowed in app mode.
   std::set<int> actions_allowed_in_app_mode_;
   // Actions allowed in pinned mode.
   std::set<int> actions_allowed_in_pinned_mode_;
   // Actions disallowed if there are no windows.
   std::set<int> actions_needing_window_;
   // Actions that can be performed without closing the menu (if one is present).
   std::set<int> actions_keeping_menu_open_;
 
   DISALLOW_COPY_AND_ASSIGN(AcceleratorController);
 };
 
 }  // namespace ash
 
 #endif  // ASH_COMMON_ACCELERATORS_ACCELERATOR_CONTROLLER_H_