Enable accelerators on web activity window

athena/input/accelerator_manager_impl.cc

 // Copyright 2014 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 "athena/input/accelerator_manager_impl.h"
 
 #include "athena/common/switches.h"
 #include "athena/input/public/input_manager.h"
 #include "base/logging.h"
 #include "ui/aura/window.h"
 #include "ui/base/accelerators/accelerator_manager.h"
 #include "ui/events/event.h"
 #include "ui/events/event_target.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/focus/focus_manager_delegate.h"
 #include "ui/views/focus/focus_manager_factory.h"
 #include "ui/wm/core/accelerator_delegate.h"
 #include "ui/wm/core/accelerator_filter.h"
 #include "ui/wm/core/nested_accelerator_controller.h"
 #include "ui/wm/core/nested_accelerator_delegate.h"
 #include "ui/wm/public/dispatcher_client.h"
 
 namespace athena {
 
+// This wrapper interface provides a common interface that handles global
+// accelerators as well as local accelerators.
+class AcceleratorManagerImpl::AcceleratorWrapper {
+ public:
+  virtual ~AcceleratorWrapper() {}
+  virtual void Register(const ui::Accelerator& accelerator,
+                        ui::AcceleratorTarget* target) = 0;
+  virtual bool Process(const ui::Accelerator& accelerator) = 0;
+  virtual ui::AcceleratorTarget* GetCurrentTarget(
+      const ui::Accelerator& accelertor) const = 0;
+};
+
 namespace {
 
 // Accelerators inside nested message loop are handled by
 // wm::NestedAcceleratorController while accelerators in normal case are
 // handled by wm::AcceleratorFilter. These delegates act bridges in these
 // two different environment so that AcceleratorManagerImpl can handle
 // accelerators in an uniform way.
 
 class NestedAcceleratorDelegate : public wm::NestedAcceleratorDelegate {
  public:
@@ skipping 21 matching lines @@
       : accelerator_manager_(accelerator_manager) {}
   virtual ~AcceleratorDelegate() {}
 
  private:
   // wm::AcceleratorDelegate:
   virtual bool ProcessAccelerator(const ui::KeyEvent& event,
                                   const ui::Accelerator& accelerator,
                                   KeyType key_type) OVERRIDE {
     aura::Window* target = static_cast<aura::Window*>(event.target());
     if (!target->IsRootWindow() &&
-        !accelerator_manager_->IsReserved(accelerator)) {
+        !accelerator_manager_->IsRegistered(accelerator, AF_RESERVED)) {
       // TODO(oshima): do the same when the active window is in fullscreen.
       return false;
     }
     return accelerator_manager_->Process(accelerator);
   }
 
   AcceleratorManagerImpl* accelerator_manager_;
   DISALLOW_COPY_AND_ASSIGN(AcceleratorDelegate);
 };
 
 class FocusManagerDelegate : public views::FocusManagerDelegate {
  public:
   explicit FocusManagerDelegate(AcceleratorManagerImpl* accelerator_manager)
       : accelerator_manager_(accelerator_manager) {}
   virtual ~FocusManagerDelegate() {}
 
   virtual bool ProcessAccelerator(const ui::Accelerator& accelerator) OVERRIDE {
     return accelerator_manager_->Process(accelerator);
   }
 
   virtual ui::AcceleratorTarget* GetCurrentTargetForAccelerator(
       const ui::Accelerator& accelerator) const OVERRIDE {
-    return accelerator_manager_->IsRegistered(accelerator)
+    return accelerator_manager_->IsRegistered(accelerator, AF_NONE)
                ? accelerator_manager_
                : NULL;
   }
 
  private:
   AcceleratorManagerImpl* accelerator_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusManagerDelegate);
 };
 
@@ skipping 14 matching lines @@
         widget,
         desktop_widget ? NULL : new FocusManagerDelegate(accelerator_manager_));
   }
 
  private:
   AcceleratorManagerImpl* accelerator_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(FocusManagerFactory);
 };
 
+class UIAcceleratorManagerWrapper
+    : public AcceleratorManagerImpl::AcceleratorWrapper {
+ public:
+  UIAcceleratorManagerWrapper()
+      : ui_accelerator_manager_(new ui::AcceleratorManager) {}
+  virtual ~UIAcceleratorManagerWrapper() {}
+
+  virtual void Register(const ui::Accelerator& accelerator,
+                        ui::AcceleratorTarget* target) OVERRIDE {
+    return ui_accelerator_manager_->Register(
+        accelerator, ui::AcceleratorManager::kNormalPriority, target);
+  }
+
+  virtual bool Process(const ui::Accelerator& accelerator) OVERRIDE {
+    return ui_accelerator_manager_->Process(accelerator);
+  }
+
+  virtual ui::AcceleratorTarget* GetCurrentTarget(
+      const ui::Accelerator& accelerator) const OVERRIDE {
+    return ui_accelerator_manager_->GetCurrentTarget(accelerator);
+  }
+
+ private:
+  scoped_ptr<ui::AcceleratorManager> ui_accelerator_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(UIAcceleratorManagerWrapper);
+};
+
+class FocusManagerWrapper : public AcceleratorManagerImpl::AcceleratorWrapper {
+ public:
+  explicit FocusManagerWrapper(views::FocusManager* focus_manager)

[Jun Mukai, 2014/06/11 22:07:51]

Can we make FocusManager being a subclass of ui::AcceleratorManager instead?

[oshima, 2014/06/11 22:46:43]

what do you mean? Focus manager is defined in views (views/focus/focus_manager),
and a focus manager doesn't have is-a relation to AcceleratorManager.

[Jun Mukai, 2014/06/11 23:13:22]

Why not?  views::FocusManager creates its own AcceleratorManager in ctor and
provides really similar APIs of accelerator_manager. What is the reason
views::FocusManager must not inherit AcceleratorManager?  It surely manages
accelerators, right?


You created the wrapper class just because FocusManager mostly behaves like
ui::AcceleratorManager but you cannot use the same interface.  My suggestion is
to make the interface same. So you don't have to create the wrapper anymore,
AcceleratorManagerImpl just receives ui::AcceleratorManager* for ctor parameter.

[Jun Mukai, 2014/06/11 23:16:10]

Or -- stop the AcceleratorWrapper class, but creates a FocusManagerWrapper class
which is like:
class FocusManagerWrapper : public ui::AcceleratorManager {
 public:
   ...
};

Or, add AcceleratorManager* accelerator_manager() { return
accelerator_manager_.get(); } to views::FocusManager and use it.

[oshima, 2014/06/12 17:06:38]

AcceleratorManager is internals state because FocusManager::ProcessAccelerator
does different thing than ui::AcceleratorManager::Process. Your design exposes
both which is error prone.

[oshima, 2014/06/12 17:06:38]

Using inheritance for has-a relation is considered bad design. I can discuss f2f
but you can also find a lot of article if you search "inheritance composition".

Also AcceleratorManager is not designed to be extended (No virtual dtor). 
It is good reason, at least better than alternating hierarchy just for this.
One possible approach is to separate ui::AcceleratorManager to be an interface,
and provide a separate implementation class, and use that interface in
FocusManager, and then delegate the call to AcceleratorManager impl. But I think
it's overkill.

Yet aonther issue is the object ownership. ui::AcceleratorManager for global
accelerator is owned here, while the focus manager's accelerator manager is
owned by the focus manager. You can have a flag to tell the ownership, but I
believe the current design is better than other options.

+      : focus_manager_(focus_manager) {}
+  virtual ~FocusManagerWrapper() {}
+
+  virtual void Register(const ui::Accelerator& accelerator,
+                        ui::AcceleratorTarget* target) OVERRIDE {
+    return focus_manager_->RegisterAccelerator(
+        accelerator, ui::AcceleratorManager::kNormalPriority, target);
+  }
+
+  virtual bool Process(const ui::Accelerator& accelerator) OVERRIDE {
+    NOTREACHED();
+    return true;
+  }
+
+  virtual ui::AcceleratorTarget* GetCurrentTarget(
+      const ui::Accelerator& accelerator) const OVERRIDE {
+    return focus_manager_->GetCurrentTargetForAccelerator(accelerator);
+  }
+
+ private:
+  views::FocusManager* focus_manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(FocusManagerWrapper);
+};
+
 }  // namespace
 
 class AcceleratorManagerImpl::InternalData {
  public:
   InternalData(int command_id, AcceleratorHandler* handler, int flags)
       : command_id_(command_id), handler_(handler), flags_(flags) {}
 
   bool IsNonAutoRepeatable() const { return flags_ & AF_NON_AUTO_REPEATABLE; }
   bool IsDebug() const { return flags_ & AF_DEBUG; }
-  bool IsReserved() const { return flags_ & AF_RESERVED; }
+  int flags() const { return flags_; }
 
   bool IsCommandEnabled() const {
     return handler_->IsCommandEnabled(command_id_);
   }
 
   bool OnAcceleratorFired(const ui::Accelerator& accelerator) {
     return handler_->OnAcceleratorFired(command_id_, accelerator);
   }
 
  private:
   int command_id_;
   AcceleratorHandler* handler_;
   int flags_;
 
   // This class is copyable by design.
 };
 
-AcceleratorManagerImpl::AcceleratorManagerImpl()
-    : accelerator_manager_(new ui::AcceleratorManager),
+// static
+AcceleratorManagerImpl*
+AcceleratorManagerImpl::CreateGlobalAcceleratorManager() {
+  return new AcceleratorManagerImpl(new UIAcceleratorManagerWrapper());
+}
+
+AcceleratorManagerImpl::AcceleratorManagerImpl(
+    AcceleratorWrapper* accelerator_wrapper)
+    : accelerator_wrapper_(accelerator_wrapper),
       debug_accelerators_enabled_(switches::IsDebugAcceleratorsEnabled()) {
 }
 
 AcceleratorManagerImpl::~AcceleratorManagerImpl() {
   nested_accelerator_controller_.reset();
   accelerator_filter_.reset();
   // Reset to use the default focus manager because the athena's
   // FocusManager has the reference to this object.
   views::FocusManagerFactory::Install(NULL);
 }
@@ skipping 11 matching lines @@
   accelerator_filter_.reset(
       new wm::AcceleratorFilter(accelerator_delegate.Pass()));
   toplevel->AddPreTargetHandler(accelerator_filter_.get());
 }
 
 void AcceleratorManagerImpl::OnRootWindowCreated(aura::Window* root_window) {
   aura::client::SetDispatcherClient(root_window,
                                     nested_accelerator_controller_.get());
 }
 
-bool AcceleratorManagerImpl::IsRegistered(
-    const ui::Accelerator& accelerator) const {
-  return accelerator_manager_->GetCurrentTarget(accelerator) != NULL;
+bool AcceleratorManagerImpl::Process(const ui::Accelerator& accelerator) {
+  return accelerator_wrapper_->Process(accelerator);
 }
 
-bool AcceleratorManagerImpl::IsReserved(
-    const ui::Accelerator& accelerator) const {
+bool AcceleratorManagerImpl::IsRegistered(const ui::Accelerator& accelerator,
+                                          int flags) const {
   std::map<ui::Accelerator, InternalData>::const_iterator iter =
       accelerators_.find(accelerator);
   if (iter == accelerators_.end())
     return false;
-  return iter->second.IsReserved();
-}
-
-bool AcceleratorManagerImpl::Process(const ui::Accelerator& accelerator) {
-  return accelerator_manager_->Process(accelerator);
+  DCHECK(accelerator_wrapper_->GetCurrentTarget(accelerator));
+  return flags == AF_NONE || iter->second.flags() & flags;
 }
 
 void AcceleratorManagerImpl::RegisterAccelerators(
     const AcceleratorData accelerators[],
     size_t num_accelerators,
     AcceleratorHandler* handler) {
   for (size_t i = 0; i < num_accelerators; ++i)
     RegisterAccelerator(accelerators[i], handler);
 }
 
@@ skipping 21 matching lines @@
 }
 
 void AcceleratorManagerImpl::RegisterAccelerator(
     const AcceleratorData& accelerator_data,
     AcceleratorHandler* handler) {
   ui::Accelerator accelerator(accelerator_data.keycode,
                               accelerator_data.keyevent_flags);
   accelerator.set_type(accelerator_data.trigger_event == TRIGGER_ON_PRESS
                            ? ui::ET_KEY_PRESSED
                            : ui::ET_KEY_RELEASED);
-  accelerator_manager_->Register(
-      accelerator, ui::AcceleratorManager::kNormalPriority, this);
+  accelerator_wrapper_->Register(accelerator, this);
   accelerators_.insert(
       std::make_pair(accelerator,
                      InternalData(accelerator_data.command_id,
                                   handler,
                                   accelerator_data.accelerator_flags)));
 }
 
 // static
 AcceleratorManager* AcceleratorManager::Get() {
   return InputManager::Get()->GetAcceleratorManager();
 }
 
+// static
+scoped_ptr<AcceleratorManager> AcceleratorManager::CreateForFocusManager(
+    views::FocusManager* focus_manager) {
+  return scoped_ptr<AcceleratorManager>(
+      new AcceleratorManagerImpl(new FocusManagerWrapper(focus_manager)));
+}
+
 }  // namespace athena