Prevent synthetic events from being processed during RootWindowController shutdown.

RootWindowController shutdown occurs in two stages. During the first stage,
the ScreenPositionClient is detached from the root window. During the second
stage, the root window is destroyed. The second stage occurs as a result of
a task posted to the message loop. This CL prevents events from being dispatched
to the root windows after the first stage has completed but before the second
stage has begun (the root window is not yet deleted).

In particular, this CL blocks the delayed synthetic mouse moves generated by
WindowEventDispatcher::OnWindowDestroying() and
WindowEventDispatcher::OnWindowRemovingFromRootWindow() during
RootWindowController shutdown.

The goal is to guarantee that mouse events will only be dispatched to an
ui::EventHandler if it is possible to convert the event's position to screen
coordinates via a aura::client::ScreenPositionClient.

BUG=449863
TEST=ScreenPositionControllerTest.ConvertToScreenWhileRemovingSecondaryDisplay

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

Cr-Commit-Position: refs/heads/master@{#316063}

4 files changed, 111 insertions, 1 deletions

diff --git a/ash/display/screen_position_controller_unittest.cc b/ash/display/screen_position_controller_unittest.cc
index f74f86a..c568b50 100644
--- a/ash/display/screen_position_controller_unittest.cc
+++ b/ash/display/screen_position_controller_unittest.cc
@@ -7,12 +7,15 @@
 #include "ash/display/display_manager.h"
 #include "ash/screen_util.h"
 #include "ash/shell.h"
+#include "ash/shell_window_ids.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/shell_test_api.h"
 #include "ui/aura/env.h"
 #include "ui/aura/test/test_window_delegate.h"
+#include "ui/aura/window_tracker.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/layout.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/gfx/screen.h"
 
 #if defined(OS_WIN)
@@ -22,17 +25,22 @@
 #define MAYBE_ConvertHostPointToScreenHiDPI DISABLED_ConvertHostPointToScreenHiDPI
 #define MAYBE_ConvertHostPointToScreenRotate DISABLED_ConvertHostPointToScreenRotate
 #define MAYBE_ConvertHostPointToScreenUIScale DISABLED_ConvertHostPointToScreenUIScale
+#define MAYBE_ConvertToScreenWhileRemovingSecondaryDisplay \
+  DISABLED_ConvertToScreenWhileRemovingSecondaryDisplay
 #else
 #define MAYBE_ConvertHostPointToScreen ConvertHostPointToScreen
 #define MAYBE_ConvertHostPointToScreenHiDPI ConvertHostPointToScreenHiDPI
 #define MAYBE_ConvertHostPointToScreenRotate ConvertHostPointToScreenRotate
 #define MAYBE_ConvertHostPointToScreenUIScale ConvertHostPointToScreenUIScale
+#define MAYBE_ConvertToScreenWhileRemovingSecondaryDisplay \
+  ConvertToScreenWhileRemovingSecondaryDisplay
 #endif
 
 namespace ash {
 namespace test {
 
 namespace {
+
 void SetSecondaryDisplayLayout(DisplayLayout::Position position) {
   DisplayLayout layout =
       Shell::GetInstance()->display_manager()->GetCurrentDisplayLayout();
@@ -276,5 +284,83 @@ TEST_F(ScreenPositionControllerTest, MAYBE_ConvertHostPointToScreenUIScale) {
   EXPECT_EQ("45,45", ConvertHostPointToScreen(60, -340));
 }
 
+namespace {
+
+// EventHandler which tracks whether it got any MouseEvents whose location could
+// not be converted to screen coordinates.
+class ConvertToScreenEventHandler : public ui::EventHandler {
+ public:
+  ConvertToScreenEventHandler() : could_convert_to_screen_(true) {
+    aura::Env::GetInstance()->AddPreTargetHandler(this);
+  }
+  ~ConvertToScreenEventHandler() override {
+    aura::Env::GetInstance()->RemovePreTargetHandler(this);
+  }
+
+  bool could_convert_to_screen() const { return could_convert_to_screen_; }
+
+ private:
+  void OnMouseEvent(ui::MouseEvent* event) override {
+    if (event->type() == ui::ET_MOUSE_CAPTURE_CHANGED)
+      return;
+
+    aura::Window* root =
+        static_cast<aura::Window*>(event->target())->GetRootWindow();
+    if (!aura::client::GetScreenPositionClient(root))
+      could_convert_to_screen_ = false;
+  }
+
+  bool could_convert_to_screen_;
+
+  DISALLOW_COPY_AND_ASSIGN(ConvertToScreenEventHandler);
+};
+
+}  // namespace
+
+// Test that events are only dispatched when a ScreenPositionClient is available
+// to convert the event to screen coordinates. The ScreenPositionClient is
+// detached from the root window prior to the root window being destroyed. Test
+// that no events are dispatched at this time.
+TEST_F(ScreenPositionControllerTest,
+       MAYBE_ConvertToScreenWhileRemovingSecondaryDisplay) {
+  UpdateDisplay("600x600,600x600");
+  RunAllPendingInMessageLoop();
+
+  // Create a window on the secondary display.
+  window_->SetBoundsInScreen(gfx::Rect(600, 0, 400, 400),
+                             ScreenUtil::GetSecondaryDisplay());
+
+  // Move the mouse cursor over |window_|. Synthetic mouse moves are dispatched
+  // asynchronously when a window which contains the mouse cursor is destroyed.
+  // We want to check that none of these synthetic events are dispatched after
+  // ScreenPositionClient has been detached from the root window.
+  GetEventGenerator().MoveMouseTo(800, 200);
+  EXPECT_TRUE(window_->GetBoundsInScreen().Contains(
+      aura::Env::GetInstance()->last_mouse_location()));
+
+  aura::Window::Windows root_windows =
+      Shell::GetInstance()->GetAllRootWindows();
+  aura::WindowTracker tracker;
+  tracker.Add(root_windows[1]);
+  scoped_ptr<ConvertToScreenEventHandler> event_handler(
+      new ConvertToScreenEventHandler);
+
+  // Remove the secondary monitor.
+  UpdateDisplay("600x600");
+
+  // The secondary root window is not immediately destroyed.
+  EXPECT_TRUE(tracker.Contains(root_windows[1]));
+
+  RunAllPendingInMessageLoop();
+
+  // Check that we waited long enough and that the secondary root window was
+  // destroyed.
+  EXPECT_FALSE(tracker.Contains(root_windows[1]));
+
+  // Check that we could convert all of the mouse events we got to screen
+  // coordinates.
+  EXPECT_TRUE(event_handler->could_convert_to_screen());
+}
+
 }  // namespace test
 }  // namespace ash
diff --git a/ash/extended_desktop_unittest.cc b/ash/extended_desktop_unittest.cc
index 2abea49..e27bf8b 100644
--- a/ash/extended_desktop_unittest.cc
+++ b/ash/extended_desktop_unittest.cc
@@ -878,6 +878,7 @@ TEST_F(ExtendedDesktopTest, KeyEventsOnLockScreen) {
 
   // Deleting 2nd display. The lock window still should get the events.
   UpdateDisplay("100x100");
+  event_generator.set_current_target(root_windows[0]);
   event_generator.PressKey(ui::VKEY_C, 0);
   event_generator.ReleaseKey(ui::VKEY_C, 0);
   EXPECT_EQ(lock_widget->GetNativeView(), focus_client->GetFocusedWindow());
diff --git a/ash/host/ash_window_tree_host_ozone.cc b/ash/host/ash_window_tree_host_ozone.cc
index 860d1ed..7c23cd9 100644
--- a/ash/host/ash_window_tree_host_ozone.cc
+++ b/ash/host/ash_window_tree_host_ozone.cc
@@ -8,7 +8,9 @@
 #include "ash/host/root_window_transformer.h"
 #include "ash/host/transformer_helper.h"
 #include "base/command_line.h"
+#include "ui/aura/window.h"
 #include "ui/aura/window_tree_host_ozone.h"
+#include "ui/events/null_event_targeter.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/transform.h"
 #include "ui/ozone/public/input_controller.h"
@@ -33,6 +35,7 @@ class AshWindowTreeHostOzone : public AshWindowTreeHost,
       scoped_ptr<RootWindowTransformer> transformer) override;
   gfx::Insets GetHostInsets() const override;
   aura::WindowTreeHost* AsWindowTreeHost() override;
+  void PrepareForShutdown() override;
   void SetRootTransform(const gfx::Transform& transform) override;
   gfx::Transform GetRootTransform() const override;
   gfx::Transform GetInverseRootTransform() const override;
@@ -85,6 +88,15 @@ aura::WindowTreeHost* AshWindowTreeHostOzone::AsWindowTreeHost() {
   return this;
 }
 
+void AshWindowTreeHostOzone::PrepareForShutdown() {
+  // Block the root window from dispatching events because it is weird for a
+  // ScreenPositionClient not to be attached to the root window and for
+  // ui::EventHandlers to be unable to convert the event's location to screen
+  // coordinates.
+  window()->SetEventTargeter(
+      scoped_ptr<ui::EventTargeter>(new ui::NullEventTargeter));
+}
+
 void AshWindowTreeHostOzone::SetRootTransform(const gfx::Transform& transform) {
   transformer_helper_.SetTransform(transform);
 }
diff --git a/ash/host/ash_window_tree_host_x11.cc b/ash/host/ash_window_tree_host_x11.cc
index 22bc6c9..fdd8cac 100644
--- a/ash/host/ash_window_tree_host_x11.cc
+++ b/ash/host/ash_window_tree_host_x11.cc
@@ -25,6 +25,7 @@
 #include "ui/events/devices/x11/touch_factory_x11.h"
 #include "ui/events/event.h"
 #include "ui/events/event_utils.h"
+#include "ui/events/null_event_targeter.h"
 #include "ui/events/platform/platform_event_source.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/screen.h"
@@ -131,8 +132,18 @@ void AshWindowTreeHostX11::UpdateDisplayID(int64 id1, int64 id2) {
 }
 
 void AshWindowTreeHostX11::PrepareForShutdown() {
-  if (ui::PlatformEventSource::GetInstance())
+  // Block the root window from dispatching events because it is weird for a
+  // ScreenPositionClient not to be attached to the root window and for
+  // ui::EventHandlers to be unable to convert the event's location to screen
+  // coordinates.
+  window()->SetEventTargeter(
+      scoped_ptr<ui::EventTargeter>(new ui::NullEventTargeter));
+
+  if (ui::PlatformEventSource::GetInstance()) {
+    // Block X events which are not turned into ui::Events from getting
+    // processed. (e.g. ConfigureNotify)
     ui::PlatformEventSource::GetInstance()->RemovePlatformEventDispatcher(this);
+  }
 }
 
 void AshWindowTreeHostX11::SetBounds(const gfx::Rect& bounds) {