Revert of Refactor and move ash independent nested accelerator code t…

…o ui/wm/core (https://codereview.chromium.org/298703007/) Reason for revert: caused test failures: http://build.chromium.org/p/chromium.win/builders/Win8%20Aura/builds/19236 NestedAcceleratorTest.AcceleratorsHandled (run #1): [ RUN ] NestedAcceleratorTest.AcceleratorsHandled Backtrace: ui::PlatformEventSource::OverrideDispatcher [0x0028B66D+253] wm::test::NestedAcceleratorTest_AcceleratorsHandled_Test::TestBody [0x00455D37+215] testing::internal::HandleExceptionsInMethodIfSupported\u003Ctesting::Test,void> [0x004D5829+329] testing::Test::Run [0x004EFF9E+174] testing::TestInfo::Run [0x004F023D+221] testing::TestCase::Run [0x004F00DF+239] testing::internal::UnitTestImpl::RunAllTests [0x004F0696+726] testing::internal::HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl,bool> [0x004D5D21+337] testing::UnitTest::Run [0x004F0393+211] RUN_ALL_TESTS [0x004A0EDF+15] base::TestSuite::Run [0x004A1129+233] base::internal::RunnableAdapter\u003Cint (__thiscall base::TestSuite::*)(void)>::Run [0x004150EB+27] base::internal::InvokeHelper\u003C0,int,base::internal::RunnableAdapter\u003Cint (__thiscall base::TestSuite::*)(void)>,void __cdecl(WMTestSuite *)>::MakeItSo [0x00414FFA+26] base::internal::Invoker\u003C1,base::internal::BindState\u003Cbase::internal::RunnableAdapter\u003Cint (__thiscall base::TestSuite::*)(void)>,int __cdecl(base::TestSuite *),void __cdecl(base::internal::UnretainedWrapper\u003CWMTestSuite>)>,int __cdecl(base::TestSuite *)>::Ru [0x0041506A+74] base::Callback\u003Cint __cdecl(void)>::Run [0x0049A03F+47] base::`anonymous namespace'::LaunchUnitTestsInternal [0x004990C9+921] base::LaunchUnitTests [0x00498D15+37] main [0x0041681C+140] __tmainCRTStartup [0x00647D39+409] (f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c:626) mainCRTStartup [0x00647E7D+13] (f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c:466) BaseThreadInitThunk [0x76568624+14] RtlInitializeExceptionChain [0x773CAC69+133] RtlInitializeExceptionChain [0x773CAC3C+88] NestedAcceleratorTest.AssociatedWindowAboveLockScreen (run #1): [ RUN ] NestedAcceleratorTest.AssociatedWindowAboveLockScreen Backtrace: ui::PlatformEventSource::OverrideDispatcher [0x0286B66D+253] wm::test::NestedAcceleratorTest_AssociatedWindowAboveLockScreen_Test::TestBody [0x004563B5+517] testing::internal::HandleExceptionsInMethodIfSupported\u003Ctesting::Test,void> [0x004D5829+329] testing::Test::Run [0x004EFF9E+174] testing::TestInfo::Run [0x004F023D+221] testing::TestCase::Run [0x004F00DF+239] testing::internal::UnitTestImpl::RunAllTests [0x004F0696+726] testing::internal::HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl,bool> [0x004D5D21+337] testing::UnitTest::Run [0x004F0393+211] RUN_ALL_TESTS [0x004A0EDF+15] base::TestSuite::Run [0x004A1129+233] base::internal::RunnableAdapter\u003Cint (__thiscall base::TestSuite::*)(void)>::Run [0x004150EB+27] base::internal::InvokeHelper\u003C0,int,base::internal::RunnableAdapter\u003Cint (__thiscall base::TestSuite::*)(void)>,void __cdecl(WMTestSuite *)>::MakeItSo [0x00414FFA+26] base::internal::Invoker\u003C1,base::internal::BindState\u003Cbase::internal::RunnableAdapter\u003Cint (__thiscall base::TestSuite::*)(void)>,int __cdecl(base::TestSuite *),void __cdecl(base::internal::UnretainedWrapper\u003CWMTestSuite>)>,int __cdecl(base::TestSuite *)>::Ru [0x0041506A+74] base::Callback\u003Cint __cdecl(void)>::Run [0x0049A03F+47] base::`anonymous namespace'::LaunchUnitTestsInternal [0x004990C9+921] base::LaunchUnitTests [0x00498D15+37] main [0x0041681C+140] __tmainCRTStartup [0x00647D39+409] (f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c:626) mainCRTStartup [0x00647E7D+13] (f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c:466) BaseThreadInitThunk [0x76568624+14] RtlInitializeExceptionChain [0x773CAC69+133] RtlInitializeExceptionChain [0x773CAC3C+88] Original issue's description: > Refactor and move ash independent accelerator handling code in nested loop to ui/wm/core > > I also renamed classes to NestedAcceleratorXxx. I felt this is a bit more clearer than NestedDispatcher, especially in ui/wm/core. Please let me know if you disagree or have better suggestion. I'm happy to rename them. > > BUG=None > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272740 > > R=ben@chromium.org > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272995 TBR=ben@chromium.org,oshima@chromium.org NOTREECHECKS=true NOTRY=true BUG=None Review URL: https://codereview.chromium.org/301743002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273003 0039d316-1c4b-4281-b951-d872f2087c98
280723148 · May 27, 2014 · 284145e · 284145e
1 parent b90ab63
commit 284145e
Show file tree

Hide file tree

Showing 17 changed files with 286 additions and 439 deletions.
diff --git a/...celerators/nested_accelerator_delegate.cc → ash/accelerators/accelerator_dispatcher.cc b/...celerators/nested_accelerator_delegate.cc → ash/accelerators/accelerator_dispatcher.cc
@@ -1,8 +1,8 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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 "ash/accelerators/nested_accelerator_delegate.h"
+#include "ash/accelerators/accelerator_dispatcher.h"
 
 #include "ash/accelerators/accelerator_controller.h"
 #include "ash/shell.h"
@@ -35,26 +35,21 @@ bool IsPossibleAcceleratorNotForMenu(const ui::KeyEvent& key_event) {
 
 }  // namespace
 
-NestedAcceleratorDelegate::NestedAcceleratorDelegate() {
-}
-
-NestedAcceleratorDelegate::~NestedAcceleratorDelegate() {
-}
-
-bool NestedAcceleratorDelegate::ShouldProcessEventNow(
+bool AcceleratorDispatcher::MenuClosedForPossibleAccelerator(
     const ui::KeyEvent& key_event) {
   if (!IsPossibleAcceleratorNotForMenu(key_event))
-    return true;
+    return false;
 
   if (views::MenuController* menu_controller =
           views::MenuController::GetActiveInstance()) {
     menu_controller->CancelAll();
-    return false;
+    return true;
   }
-  return true;
+  return false;
 }
 
-bool NestedAcceleratorDelegate::ProcessEvent(const ui::KeyEvent& key_event) {
+bool AcceleratorDispatcher::AcceleratorProcessedForKeyEvent(
+    const ui::KeyEvent& key_event) {
   ash::AcceleratorController* accelerator_controller =
       ash::Shell::GetInstance()->accelerator_controller();
   if (!accelerator_controller)

diff --git a/ash/accelerators/accelerator_dispatcher.h b/ash/accelerators/accelerator_dispatcher.h
@@ -0,0 +1,58 @@
+// 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.
+
+#ifndef ASH_ACCELERATORS_ACCELERATOR_DISPATCHER_H_
+#define ASH_ACCELERATORS_ACCELERATOR_DISPATCHER_H_
+
+#include "ash/ash_export.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace base {
+class MessagePumpDispatcher;
+class RunLoop;
+}
+
+namespace ui {
+class KeyEvent;
+}
+
+namespace ash {
+
+// Dispatcher for handling accelerators from menu.
+//
+// Wraps a nested dispatcher to which control is passed if no accelerator key
+// has been pressed. If the nested dispatcher is NULL, then the control is
+// passed back to the default dispatcher.
+// TODO(pkotwicz): Add support for a |nested_dispatcher| which sends
+//  events to a system IME.
+class ASH_EXPORT AcceleratorDispatcher {
+ public:
+  virtual ~AcceleratorDispatcher() {}
+
+  static scoped_ptr<AcceleratorDispatcher> Create(
+      base::MessagePumpDispatcher* nested_dispatcher);
+
+  // Creates a base::RunLoop object to run a nested message loop.
+  virtual scoped_ptr<base::RunLoop> CreateRunLoop() = 0;
+
+ protected:
+  AcceleratorDispatcher() {}
+
+  // Closes any open menu if the key-event could potentially be a system
+  // accelerator.
+  // Returns whether a menu was closed.
+  bool MenuClosedForPossibleAccelerator(const ui::KeyEvent& key_event);
+
+  // Attempts to trigger an accelerator for the key-event.
+  // Returns whether an accelerator was triggered.
+  bool AcceleratorProcessedForKeyEvent(const ui::KeyEvent& key_event);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AcceleratorDispatcher);
+};
+
+}  // namespace ash
+
+#endif  // ASH_ACCELERATORS_ACCELERATOR_DISPATCHER_H_
diff --git a/...re/nested_accelerator_dispatcher_linux.cc → ...elerators/accelerator_dispatcher_linux.cc b/...re/nested_accelerator_dispatcher_linux.cc → ...elerators/accelerator_dispatcher_linux.cc
@@ -2,21 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/wm/core/nested_accelerator_dispatcher.h"
+#include "ash/accelerators/accelerator_dispatcher.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.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/nested_accelerator_delegate.h"
 
 #if defined(USE_X11)
 #include <X11/Xlib.h>
 #endif
 
-namespace wm {
+namespace ash {
 
 namespace {
 
@@ -42,14 +41,13 @@ scoped_ptr<ui::ScopedEventDispatcher> OverrideDispatcher(
 
 }  // namespace
 
-class NestedAcceleratorDispatcherLinux : public NestedAcceleratorDispatcher,
-                                         public ui::PlatformEventDispatcher {
+class AcceleratorDispatcherLinux : public AcceleratorDispatcher,
+                                   public ui::PlatformEventDispatcher {
  public:
-  explicit NestedAcceleratorDispatcherLinux(NestedAcceleratorDelegate* delegate)
-      : NestedAcceleratorDispatcher(delegate),
-        restore_dispatcher_(OverrideDispatcher(this)) {}
+  AcceleratorDispatcherLinux()
+      : restore_dispatcher_(OverrideDispatcher(this)) {}
 
-  virtual ~NestedAcceleratorDispatcherLinux() {}
+  virtual ~AcceleratorDispatcherLinux() {}
 
  private:
   // AcceleratorDispatcher:
@@ -65,7 +63,7 @@ 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)) {
+      if (MenuClosedForPossibleAccelerator(key_event)) {
 #if defined(USE_X11)
         XPutBackEvent(event->xany.display, event);
 #else
@@ -74,25 +72,23 @@ class NestedAcceleratorDispatcherLinux : public NestedAcceleratorDispatcher,
         return ui::POST_DISPATCH_NONE;
       }
 
-      if (delegate_->ProcessEvent(key_event))
+      if (AcceleratorProcessedForKeyEvent(key_event))
         return ui::POST_DISPATCH_NONE;
     }
-    ui::PlatformEventDispatcher* prev = *restore_dispatcher_;
 
+    ui::PlatformEventDispatcher* prev = *restore_dispatcher_;
     return prev ? prev->DispatchEvent(event)
                 : ui::POST_DISPATCH_PERFORM_DEFAULT;
   }
 
   scoped_ptr<ui::ScopedEventDispatcher> restore_dispatcher_;
 
-  DISALLOW_COPY_AND_ASSIGN(NestedAcceleratorDispatcherLinux);
+  DISALLOW_COPY_AND_ASSIGN(AcceleratorDispatcherLinux);
 };
 
-scoped_ptr<NestedAcceleratorDispatcher> NestedAcceleratorDispatcher::Create(
-    NestedAcceleratorDelegate* delegate,
+scoped_ptr<AcceleratorDispatcher> AcceleratorDispatcher::Create(
     base::MessagePumpDispatcher* nested_dispatcher) {
-  return scoped_ptr<NestedAcceleratorDispatcher>(
-      new NestedAcceleratorDispatcherLinux(delegate));
+  return scoped_ptr<AcceleratorDispatcher>(new AcceleratorDispatcherLinux());
 }
 
-}  // namespace wm
+}  // namespace ash
diff --git a/...core/nested_accelerator_dispatcher_win.cc → ...ccelerators/accelerator_dispatcher_win.cc b/...core/nested_accelerator_dispatcher_win.cc → ...ccelerators/accelerator_dispatcher_win.cc
@@ -2,17 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ui/wm/core/nested_accelerator_dispatcher.h"
+#include "ash/accelerators/accelerator_dispatcher.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_pump_dispatcher.h"
 #include "base/run_loop.h"
 #include "ui/events/event.h"
-#include "ui/wm/core/nested_accelerator_delegate.h"
 
 using base::MessagePumpDispatcher;
 
-namespace wm {
+namespace ash {
 
 namespace {
 
@@ -23,16 +22,15 @@ bool IsKeyEvent(const MSG& msg) {
 
 }  // namespace
 
-class NestedAcceleratorDispatcherWin : public NestedAcceleratorDispatcher,
-                                       public MessagePumpDispatcher {
+class AcceleratorDispatcherWin : public AcceleratorDispatcher,
+                                 public MessagePumpDispatcher {
  public:
-  NestedAcceleratorDispatcherWin(NestedAcceleratorDelegate* delegate,
-                                 MessagePumpDispatcher* nested)
-      : NestedAcceleratorDispatcher(delegate), nested_dispatcher_(nested) {}
-  virtual ~NestedAcceleratorDispatcherWin() {}
+  explicit AcceleratorDispatcherWin(MessagePumpDispatcher* nested)
+      : nested_dispatcher_(nested) {}
+  virtual ~AcceleratorDispatcherWin() {}
 
  private:
-  // NestedAcceleratorDispatcher:
+  // AcceleratorDispatcher:
   virtual scoped_ptr<base::RunLoop> CreateRunLoop() OVERRIDE {
     return scoped_ptr<base::RunLoop>(new base::RunLoop(this));
   }
@@ -41,10 +39,10 @@ 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))
+      if (MenuClosedForPossibleAccelerator(key_event))
         return POST_DISPATCH_QUIT_LOOP;
 
-      if (delegate_->ProcessEvent(key_event))
+      if (AcceleratorProcessedForKeyEvent(key_event))
         return POST_DISPATCH_NONE;
     }
 
@@ -54,14 +52,13 @@ class NestedAcceleratorDispatcherWin : public NestedAcceleratorDispatcher,
 
   MessagePumpDispatcher* nested_dispatcher_;
 
-  DISALLOW_COPY_AND_ASSIGN(NestedAcceleratorDispatcherWin);
+  DISALLOW_COPY_AND_ASSIGN(AcceleratorDispatcherWin);
 };
 
-scoped_ptr<NestedAcceleratorDispatcher> NestedAcceleratorDispatcher::Create(
-    NestedAcceleratorDelegate* delegate,
+scoped_ptr<AcceleratorDispatcher> AcceleratorDispatcher::Create(
     MessagePumpDispatcher* nested_dispatcher) {
-  return scoped_ptr<NestedAcceleratorDispatcher>(
-      new NestedAcceleratorDispatcherWin(delegate, nested_dispatcher));
+  return scoped_ptr<AcceleratorDispatcher>(
+      new AcceleratorDispatcherWin(nested_dispatcher));
 }
 
-}  // namespace wm
+}  // namespace ash
diff --git a/ash/accelerators/nested_accelerator_delegate.h b/ash/accelerators/nested_accelerator_delegate.h
diff --git a/ui/wm/core/nested_accelerator_controller.cc → ...elerators/nested_dispatcher_controller.cc b/ui/wm/core/nested_accelerator_controller.cc → ...elerators/nested_dispatcher_controller.cc
@@ -1,34 +1,30 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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 "ui/wm/core/nested_accelerator_controller.h"
+#include "ash/accelerators/nested_dispatcher_controller.h"
 
+#include "ash/accelerators/accelerator_dispatcher.h"
+#include "ash/shell.h"
 #include "base/auto_reset.h"
 #include "base/run_loop.h"
-#include "ui/wm/core/nested_accelerator_delegate.h"
-#include "ui/wm/core/nested_accelerator_dispatcher.h"
 
-namespace wm {
+namespace ash {
 
-NestedAcceleratorController::NestedAcceleratorController(
-    NestedAcceleratorDelegate* delegate)
-    : dispatcher_delegate_(delegate) {
-  DCHECK(delegate);
+NestedDispatcherController::NestedDispatcherController() {
 }
 
-NestedAcceleratorController::~NestedAcceleratorController() {
+NestedDispatcherController::~NestedDispatcherController() {
 }
 
-void NestedAcceleratorController::RunWithDispatcher(
+void NestedDispatcherController::RunWithDispatcher(
     base::MessagePumpDispatcher* nested_dispatcher) {
   base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
   base::MessageLoopForUI::ScopedNestableTaskAllower allow_nested(loop);
 
-  scoped_ptr<NestedAcceleratorDispatcher> old_accelerator_dispatcher =
+  scoped_ptr<AcceleratorDispatcher> old_accelerator_dispatcher =
       accelerator_dispatcher_.Pass();
-  accelerator_dispatcher_ = NestedAcceleratorDispatcher::Create(
-      dispatcher_delegate_.get(), nested_dispatcher);
+  accelerator_dispatcher_ = AcceleratorDispatcher::Create(nested_dispatcher);
 
   // TODO(jbates) crbug.com/134753 Find quitters of this RunLoop and have them
   //              use run_loop.QuitClosure().
@@ -39,10 +35,10 @@ void NestedAcceleratorController::RunWithDispatcher(
   accelerator_dispatcher_ = old_accelerator_dispatcher.Pass();
 }
 
-void NestedAcceleratorController::QuitNestedMessageLoop() {
+void NestedDispatcherController::QuitNestedMessageLoop() {
   CHECK(!quit_closure_.is_null());
   quit_closure_.Run();
   accelerator_dispatcher_.reset();
 }
 
-}  // namespace wm
+}  // namespace ash