More accelerator code leanup

* Remove null check
* Simplify NestedAcceleratorDelegate interface.

BUG=None
R=sky@chromium.org

Review URL: https://codereview.chromium.org/312483002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@274560 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 60 insertions, 38 deletions

diff --git a/ui/wm/core/accelerator_filter.cc b/ui/wm/core/accelerator_filter.cc
index 5010262..706cb2e 100644
--- a/ui/wm/core/accelerator_filter.cc
+++ b/ui/wm/core/accelerator_filter.cc
@@ -11,9 +11,6 @@
 namespace wm {
 namespace {
 
-const int kModifierFlagMask =
-    (ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN);
-
 // Returns true if |key_code| is a key usually handled directly by the shell.
 bool IsSystemKey(ui::KeyboardCode key_code) {
 #if defined(OS_CHROMEOS)
@@ -52,14 +49,13 @@ AcceleratorFilter::~AcceleratorFilter() {
 
 void AcceleratorFilter::OnKeyEvent(ui::KeyEvent* event) {
   const ui::EventType type = event->type();
+  DCHECK(event->target());
   if ((type != ui::ET_KEY_PRESSED && type != ui::ET_KEY_RELEASED) ||
-      event->is_char()) {
+      event->is_char() || !event->target()) {
     return;
   }
 
-  ui::Accelerator accelerator(event->key_code(),
-                              event->flags() & kModifierFlagMask);
-  accelerator.set_type(event->type());
+  ui::Accelerator accelerator = CreateAcceleratorFromKeyEvent(*event);
 
   delegate_->PreProcessAccelerator(accelerator);
 
@@ -71,4 +67,15 @@ void AcceleratorFilter::OnKeyEvent(ui::KeyEvent* event) {
     event->StopPropagation();
 }
 
+ui::Accelerator CreateAcceleratorFromKeyEvent(const ui::KeyEvent& key_event) {
+  const int kModifierFlagMask =
+      (ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN);
+
+  ui::Accelerator accelerator(key_event.key_code(),
+                              key_event.flags() & kModifierFlagMask);
+  if (key_event.type() == ui::ET_KEY_RELEASED)
+    accelerator.set_type(ui::ET_KEY_RELEASED);
+  return accelerator;
+}
+
 }  // namespace wm
diff --git a/ui/wm/core/accelerator_filter.h b/ui/wm/core/accelerator_filter.h
index d42a6e2..4e5295e 100644
--- a/ui/wm/core/accelerator_filter.h
+++ b/ui/wm/core/accelerator_filter.h
@@ -10,6 +10,10 @@
 #include "ui/events/event_handler.h"
 #include "ui/wm/wm_export.h"
 
+namespace ui {
+class Accelerator;
+}
+
 namespace wm {
 class AcceleratorDelegate;
 
@@ -29,6 +33,8 @@ class WM_EXPORT AcceleratorFilter : public ui::EventHandler {
   DISALLOW_COPY_AND_ASSIGN(AcceleratorFilter);
 };
 
+ui::Accelerator CreateAcceleratorFromKeyEvent(const ui::KeyEvent& key_event);
+
 }  // namespace wm
 
 #endif  // UI_WM_CORE_ACCELERATOR_FILTER_H_
diff --git a/ui/wm/core/nested_accelerator_controller_unittest.cc b/ui/wm/core/nested_accelerator_controller_unittest.cc
index fae9f8e..9fa394e 100644
--- a/ui/wm/core/nested_accelerator_controller_unittest.cc
+++ b/ui/wm/core/nested_accelerator_controller_unittest.cc
@@ -104,17 +104,10 @@ class MockNestedAcceleratorDelegate : public NestedAcceleratorDelegate {
   virtual ~MockNestedAcceleratorDelegate() {}
 
   // NestedAcceleratorDelegate:
-  virtual bool ShouldProcessEventNow(const ui::KeyEvent& key_event) OVERRIDE {
-    return true;
-  }
-  virtual bool ProcessEvent(const ui::KeyEvent& key_event) OVERRIDE {
-    const int kModifierMask =
-        (ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN);
-    ui::Accelerator accelerator(key_event.key_code(),
-                                key_event.flags() & kModifierMask);
-    if (key_event.type() == ui::ET_KEY_RELEASED)
-      accelerator.set_type(ui::ET_KEY_RELEASED);
-    return accelerator_manager_->Process(accelerator);
+  virtual Result ProcessAccelerator(
+      const ui::Accelerator& accelerator) OVERRIDE {
+    return accelerator_manager_->Process(accelerator) ?
+        RESULT_PROCESSED : RESULT_NOT_PROCESSED;
   }
 
   void Register(const ui::Accelerator& accelerator,
diff --git a/ui/wm/core/nested_accelerator_delegate.h b/ui/wm/core/nested_accelerator_delegate.h
index 45b9e2a..4b13c0d 100644
--- a/ui/wm/core/nested_accelerator_delegate.h
+++ b/ui/wm/core/nested_accelerator_delegate.h
@@ -6,7 +6,7 @@
 #define UI_WM_CORE_NESTED_ACCELERATOR_DELEGATE_H_
 
 namespace ui {
-class KeyEvent;
+class Accelerator;
 }
 
 namespace wm {
@@ -15,15 +15,18 @@ namespace wm {
 // handling.
 class NestedAcceleratorDelegate {
  public:
-  virtual ~NestedAcceleratorDelegate() {}
+  enum Result {
+    RESULT_PROCESSED,
+    RESULT_NOT_PROCESSED,
+    // The key event should be ignored now and instead be reposted so that
+    // next event loop.
+    RESULT_PROCESS_LATER,
+  };
 
-  // If the key event should be ignored now and instead be reposted so that next
-  // event loop.
-  virtual bool ShouldProcessEventNow(const ui::KeyEvent& key_event) = 0;
+  virtual ~NestedAcceleratorDelegate() {}
 
-  // Attempts to process an accelerator for the key-event.
-  // Returns whether an accelerator was triggered and processed.
-  virtual bool ProcessEvent(const ui::KeyEvent& key_event) = 0;
+  // Attempts to process the |accelerator|.
+  virtual Result ProcessAccelerator(const ui::Accelerator& accelerator) = 0;
 };
 
 }  // namespace wm
diff --git a/ui/wm/core/nested_accelerator_dispatcher_linux.cc b/ui/wm/core/nested_accelerator_dispatcher_linux.cc
index bf21d3f..12340b6 100644
--- a/ui/wm/core/nested_accelerator_dispatcher_linux.cc
+++ b/ui/wm/core/nested_accelerator_dispatcher_linux.cc
@@ -6,10 +6,12 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
+#include "ui/base/accelerators/accelerator.h"
 #include "ui/events/event.h"
 #include "ui/events/platform/platform_event_dispatcher.h"
 #include "ui/events/platform/platform_event_source.h"
 #include "ui/events/platform/scoped_event_dispatcher.h"
+#include "ui/wm/core/accelerator_filter.h"
 #include "ui/wm/core/nested_accelerator_delegate.h"
 
 #if defined(USE_X11)
@@ -65,17 +67,21 @@ class NestedAcceleratorDispatcherLinux : public NestedAcceleratorDispatcher,
   virtual uint32_t DispatchEvent(const ui::PlatformEvent& event) OVERRIDE {
     if (IsKeyEvent(event)) {
       ui::KeyEvent key_event(event, false);
-      if (!delegate_->ShouldProcessEventNow(key_event)) {
+      ui::Accelerator accelerator = CreateAcceleratorFromKeyEvent(key_event);
+
+      switch (delegate_->ProcessAccelerator(accelerator)) {
+        case NestedAcceleratorDelegate::RESULT_PROCESS_LATER:
 #if defined(USE_X11)
-        XPutBackEvent(event->xany.display, event);
+          XPutBackEvent(event->xany.display, event);
 #else
-        NOTIMPLEMENTED();
+          NOTIMPLEMENTED();
 #endif
-        return ui::POST_DISPATCH_NONE;
+          return ui::POST_DISPATCH_NONE;
+        case NestedAcceleratorDelegate::RESULT_PROCESSED:
+          return ui::POST_DISPATCH_NONE;
+        case NestedAcceleratorDelegate::RESULT_NOT_PROCESSED:
+          break;
       }
-
-      if (delegate_->ProcessEvent(key_event))
-        return ui::POST_DISPATCH_NONE;
     }
     ui::PlatformEventDispatcher* prev = *restore_dispatcher_;
 
diff --git a/ui/wm/core/nested_accelerator_dispatcher_win.cc b/ui/wm/core/nested_accelerator_dispatcher_win.cc
index a6a5bd9..a810bb9 100644
--- a/ui/wm/core/nested_accelerator_dispatcher_win.cc
+++ b/ui/wm/core/nested_accelerator_dispatcher_win.cc
@@ -7,7 +7,9 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_pump_dispatcher.h"
 #include "base/run_loop.h"
+#include "ui/base/accelerators/accelerator.h"
 #include "ui/events/event.h"
+#include "ui/wm/core/accelerator_filter.h"
 #include "ui/wm/core/nested_accelerator_delegate.h"
 
 using base::MessagePumpDispatcher;
@@ -41,11 +43,16 @@ class NestedAcceleratorDispatcherWin : public NestedAcceleratorDispatcher,
   virtual uint32_t Dispatch(const MSG& event) OVERRIDE {
     if (IsKeyEvent(event)) {
       ui::KeyEvent key_event(event, false);
-      if (!delegate_->ShouldProcessEventNow(key_event))
-        return POST_DISPATCH_QUIT_LOOP;
-
-      if (delegate_->ProcessEvent(key_event))
-        return POST_DISPATCH_NONE;
+      ui::Accelerator accelerator = CreateAcceleratorFromKeyEvent(key_event);
+
+      switch (delegate_->ProcessAccelerator(accelerator)) {
+        case NestedAcceleratorDelegate::RESULT_PROCESS_LATER:
+          return POST_DISPATCH_QUIT_LOOP;
+        case NestedAcceleratorDelegate::RESULT_PROCESSED:
+          return POST_DISPATCH_NONE;
+        case NestedAcceleratorDelegate::RESULT_NOT_PROCESSED:
+          break;
+      }
     }
 
     return nested_dispatcher_ ? nested_dispatcher_->Dispatch(event)