Skip to content

Commit

Permalink
ash: Remove the associated-window from AcceleratorDispatcher.
Browse files Browse the repository at this point in the history
The associated-window parameter was added to fix event-dispatch if the
lock screen came up from a nested message-loop created by the menu
(crbug.com/113247). This is not possible anymore, and so remove this code.

BUG=none
R=oshima@chromium.org, sky@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257299 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
sadrul@chromium.org committed Mar 15, 2014
1 parent 9fc4b43 commit dfd3484
Show file tree
Hide file tree
Showing 15 changed files with 25 additions and 73 deletions.
25 changes: 2 additions & 23 deletions ash/accelerators/accelerator_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@

#if defined(USE_X11)
#include <X11/Xlib.h>

// Xlib defines RootWindow
#ifdef RootWindow
#undef RootWindow
#endif
#endif // defined(USE_X11)

#include "ash/accelerators/accelerator_controller.h"
Expand Down Expand Up @@ -66,30 +61,14 @@ bool IsPossibleAcceleratorNotForMenu(const ui::KeyEvent& key_event) {
} // namespace

AcceleratorDispatcher::AcceleratorDispatcher(
base::MessagePumpDispatcher* nested_dispatcher,
aura::Window* associated_window)
: nested_dispatcher_(nested_dispatcher),
associated_window_(associated_window) {
associated_window_->AddObserver(this);
base::MessagePumpDispatcher* nested_dispatcher)
: nested_dispatcher_(nested_dispatcher) {
}

AcceleratorDispatcher::~AcceleratorDispatcher() {
if (associated_window_)
associated_window_->RemoveObserver(this);
}

void AcceleratorDispatcher::OnWindowDestroying(aura::Window* window) {
if (associated_window_ == window)
associated_window_ = NULL;
}

uint32_t AcceleratorDispatcher::Dispatch(const base::NativeEvent& event) {
if (!associated_window_)
return POST_DISPATCH_QUIT_LOOP;

if (!associated_window_->CanReceiveEvents())
return POST_DISPATCH_PERFORM_DEFAULT;

if (IsKeyEvent(event)) {
ui::KeyEvent key_event(event, false);
if (IsPossibleAcceleratorNotForMenu(key_event)) {
Expand Down
16 changes: 3 additions & 13 deletions ash/accelerators/accelerator_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
#define ASH_ACCELERATORS_ACCELERATOR_DISPATCHER_H_

#include "ash/ash_export.h"
#include "base/macros.h"
#include "base/message_loop/message_pump_dispatcher.h"
#include "ui/aura/window.h"
#include "ui/aura/window_observer.h"

namespace ash {

Expand All @@ -19,26 +18,17 @@ namespace ash {
// 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 base::MessagePumpDispatcher,
public aura::WindowObserver {
class ASH_EXPORT AcceleratorDispatcher : public base::MessagePumpDispatcher {
public:
AcceleratorDispatcher(base::MessagePumpDispatcher* nested_dispatcher,
aura::Window* associated_window);
explicit AcceleratorDispatcher(base::MessagePumpDispatcher* dispatcher);
virtual ~AcceleratorDispatcher();

// MessagePumpDispatcher overrides:
virtual uint32_t Dispatch(const base::NativeEvent& event) OVERRIDE;

// aura::WindowObserver overrides:
virtual void OnWindowDestroying(aura::Window* window) OVERRIDE;

private:
base::MessagePumpDispatcher* nested_dispatcher_;

// Window associated with |nested_dispatcher_| which is used to determine
// whether the |nested_dispatcher_| is allowed to receive events.
aura::Window* associated_window_;

DISALLOW_COPY_AND_ASSIGN(AcceleratorDispatcher);
};

Expand Down
5 changes: 2 additions & 3 deletions ash/accelerators/nested_dispatcher_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ NestedDispatcherController::~NestedDispatcherController() {
}

void NestedDispatcherController::RunWithDispatcher(
base::MessagePumpDispatcher* nested_dispatcher,
aura::Window* associated_window) {
base::MessagePumpDispatcher* nested_dispatcher) {
base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
base::MessageLoopForUI::ScopedNestableTaskAllower allow_nested(loop);

AcceleratorDispatcher dispatcher(nested_dispatcher, associated_window);
AcceleratorDispatcher dispatcher(nested_dispatcher);

// TODO(jbates) crbug.com/134753 Find quitters of this RunLoop and have them
// use run_loop.QuitClosure().
Expand Down
5 changes: 2 additions & 3 deletions ash/accelerators/nested_dispatcher_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "base/callback.h"
#include "base/message_loop/message_loop.h"
#include "ui/aura/client/dispatcher_client.h"
#include "ui/aura/window.h"

namespace ash {

Expand All @@ -24,8 +23,8 @@ class ASH_EXPORT NestedDispatcherController
virtual ~NestedDispatcherController();

// aura::client::DispatcherClient:
virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher,
aura::Window* associated_window) OVERRIDE;
virtual void RunWithDispatcher(
base::MessagePumpDispatcher* dispatcher) OVERRIDE;
virtual void QuitNestedMessageLoop() OVERRIDE;

private:
Expand Down
21 changes: 2 additions & 19 deletions ash/accelerators/nested_dispatcher_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,6 @@ void DispatchKeyReleaseA() {

typedef AshTestBase NestedDispatcherTest;

// Aura window below lock screen in z order.
TEST_F(NestedDispatcherTest, AssociatedWindowBelowLockScreen) {
MockDispatcher inner_dispatcher;
scoped_ptr<aura::Window> associated_window(CreateTestWindowInShellWithId(0));

Shell::GetInstance()->session_state_delegate()->LockScreen();
DispatchKeyReleaseA();
aura::Window* root_window = ash::Shell::GetPrimaryRootWindow();
aura::client::GetDispatcherClient(root_window)->RunWithDispatcher(
&inner_dispatcher,
associated_window.get());
EXPECT_EQ(0, inner_dispatcher.num_key_events_dispatched());
Shell::GetInstance()->session_state_delegate()->UnlockScreen();
}

// Aura window above lock screen in z order.
TEST_F(NestedDispatcherTest, AssociatedWindowAboveLockScreen) {
MockDispatcher inner_dispatcher;
Expand All @@ -127,8 +112,7 @@ TEST_F(NestedDispatcherTest, AssociatedWindowAboveLockScreen) {
DispatchKeyReleaseA();
aura::Window* root_window = ash::Shell::GetPrimaryRootWindow();
aura::client::GetDispatcherClient(root_window)->RunWithDispatcher(
&inner_dispatcher,
associated_window.get());
&inner_dispatcher);
EXPECT_EQ(1, inner_dispatcher.num_key_events_dispatched());
}

Expand All @@ -145,8 +129,7 @@ TEST_F(NestedDispatcherTest, AcceleratorsHandled) {

DispatchKeyReleaseA();
aura::client::GetDispatcherClient(root_window)->RunWithDispatcher(
&inner_dispatcher,
root_window);
&inner_dispatcher);
EXPECT_EQ(0, inner_dispatcher.num_key_events_dispatched());
EXPECT_EQ(1, target.accelerator_pressed_count());
}
Expand Down
2 changes: 2 additions & 0 deletions ash/wm/lock_state_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "ui/aura/window_event_dispatcher.h"
#include "ui/compositor/layer_animation_sequence.h"
#include "ui/compositor/scoped_layer_animation_settings.h"
#include "ui/views/controls/menu/menu_controller.h"
#include "ui/wm/core/compound_event_filter.h"

#if defined(OS_CHROMEOS)
Expand Down Expand Up @@ -615,6 +616,7 @@ void LockStateController::PostLockAnimationFinished() {
lock_screen_displayed_callback_.Run();
lock_screen_displayed_callback_.Reset();
}
CHECK(!views::MenuController::GetActiveInstance());
if (shutdown_after_lock_) {
shutdown_after_lock_ = false;
StartLockToShutdownTimer();
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/views/first_run_dialog.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "grit/theme_resources.h"
#include "ui/aura/client/dispatcher_client.h"
#include "ui/aura/env.h"
#include "ui/aura/window.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/views/controls/button/checkbox.h"
Expand Down Expand Up @@ -65,7 +66,7 @@ bool FirstRunDialog::Show(Profile* profile) {
aura::Window* anchor = dialog->GetWidget()->GetNativeWindow();
aura::client::DispatcherClient* client =
aura::client::GetDispatcherClient(anchor->GetRootWindow());
client->RunWithDispatcher(NULL, anchor);
client->RunWithDispatcher(NULL);
dialog_shown = true;
}
#endif // defined(GOOGLE_CHROME_BUILD)
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/views/simple_message_box_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "grit/generated_resources.h"
#include "ui/aura/client/dispatcher_client.h"
#include "ui/aura/env.h"
#include "ui/aura/window.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/native_widget_types.h"
Expand Down Expand Up @@ -224,7 +225,7 @@ MessageBoxResult ShowMessageBoxImpl(gfx::NativeWindow parent,
aura::Window* anchor = dialog->GetWidget()->GetNativeWindow();
aura::client::DispatcherClient* client =
aura::client::GetDispatcherClient(anchor->GetRootWindow());
client->RunWithDispatcher(NULL, anchor);
client->RunWithDispatcher(NULL);
return dialog->result();
}

Expand Down
1 change: 1 addition & 0 deletions ui/aura/client/dispatcher_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "ui/aura/client/dispatcher_client.h"

#include "ui/aura/window.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/aura/window_property.h"

Expand Down
4 changes: 1 addition & 3 deletions ui/aura/client/dispatcher_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "base/message_loop/message_pump_dispatcher.h"
#include "ui/aura/aura_export.h"
#include "ui/aura/window.h"

namespace aura {
class Window;
Expand All @@ -16,8 +15,7 @@ namespace client {
// An interface implemented by an object which handles nested dispatchers.
class AURA_EXPORT DispatcherClient {
public:
virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher,
aura::Window* associated_window) = 0;
virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher) = 0;

virtual void QuitNestedMessageLoop() = 0;
};
Expand Down
1 change: 1 addition & 0 deletions ui/views/controls/menu/menu_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/time/time.h"
#include "ui/aura/client/dispatcher_client.h"
#include "ui/aura/env.h"
#include "ui/aura/window.h"
#include "ui/aura/window_event_dispatcher.h"
#include "ui/base/dragdrop/drag_utils.h"
#include "ui/base/dragdrop/os_exchange_data.h"
Expand Down
3 changes: 1 addition & 2 deletions ui/views/controls/menu/menu_controller_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ void MenuController::RunMessageLoop(bool nested_menu) {
scoped_ptr<ActivationChangeObserverImpl> observer;
if (!nested_menu)
observer.reset(new ActivationChangeObserverImpl(this, root));
aura::client::GetDispatcherClient(root)->
RunWithDispatcher(this, owner_->GetNativeWindow());
aura::client::GetDispatcherClient(root)->RunWithDispatcher(this);
} else {
base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
base::MessageLoop::ScopedNestableTaskAllower allow(loop);
Expand Down
3 changes: 1 addition & 2 deletions ui/views/widget/desktop_aura/desktop_dispatcher_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ DesktopDispatcherClient::~DesktopDispatcherClient() {
}

void DesktopDispatcherClient::RunWithDispatcher(
base::MessagePumpDispatcher* nested_dispatcher,
aura::Window* associated_window) {
base::MessagePumpDispatcher* nested_dispatcher) {
// TODO(erg): This class has been copypastad from
// ash/accelerators/nested_dispatcher_controller.cc. I have left my changes
// commented out because I don't entirely understand the implications of the
Expand Down
4 changes: 2 additions & 2 deletions ui/views/widget/desktop_aura/desktop_dispatcher_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ class VIEWS_EXPORT DesktopDispatcherClient
DesktopDispatcherClient();
virtual ~DesktopDispatcherClient();

virtual void RunWithDispatcher(base::MessagePumpDispatcher* dispatcher,
aura::Window* associated_window) OVERRIDE;
virtual void RunWithDispatcher(
base::MessagePumpDispatcher* dispatcher) OVERRIDE;
virtual void QuitNestedMessageLoop() OVERRIDE;

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ TEST_F(DesktopNativeWidgetAuraTest, WidgetCanBeDestroyedFromNestedLoop) {
// |RunWithDispatcher()| below.
message_loop()->PostTask(FROM_HERE,
base::Bind(&QuitNestedLoopAndCloseWidget, base::Passed(&widget), client));
client->RunWithDispatcher(NULL, NULL);
client->RunWithDispatcher(NULL);
}

} // namespace views

0 comments on commit dfd3484

Please sign in to comment.