Keep the window tree host until MoveWindowsTo() is finished.

Some handlers in MoveWindowsTo() can GetRootWindowForDisplayId().
This causes a crash because the removing gfx::Display is still
valid but the corresponding window tree host is already detached.

This CL delays the detaching, window tree host is still valid
during OnDisplayRemoved().

BUG=415222
R=oshima@chromium.org
TEST=the new test case covers

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

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

2 files changed, 62 insertions, 6 deletions

diff --git a/ash/display/display_controller.cc b/ash/display/display_controller.cc
index cc5a8b4..2b5a130 100644
--- a/ash/display/display_controller.cc
+++ b/ash/display/display_controller.cc
@@ -648,21 +648,25 @@ void DisplayController::OnDisplayRemoved(const gfx::Display& display) {
   AshWindowTreeHost* host_to_delete = window_tree_hosts_[display.id()];
   CHECK(host_to_delete) << display.ToString();
 
-  // Display for root window will be deleted when the Primary RootWindow
-  // is deleted by the Shell.
-  window_tree_hosts_.erase(display.id());
-
   // When the primary root window's display is removed, move the primary
   // root to the other display.
   if (primary_display_id == display.id()) {
     // Temporarily store the primary root window in
     // |primary_root_window_for_replace_| when replacing the display.
-    if (window_tree_hosts_.size() == 0) {
+    if (window_tree_hosts_.size() == 1) {
       primary_display_id = gfx::Display::kInvalidDisplayID;
       primary_tree_host_for_replace_ = host_to_delete;
+      // Display for root window will be deleted when the Primary RootWindow
+      // is deleted by the Shell.
+      window_tree_hosts_.erase(display.id());
       return;
     }
-    primary_display_id = window_tree_hosts_.begin()->first;
+    for (const auto& pair : window_tree_hosts_) {
+      if (pair.first != display.id()) {
+        primary_display_id = pair.first;
+        break;
+      }
+    }
     CHECK_NE(gfx::Display::kInvalidDisplayID, primary_display_id);
 
     AshWindowTreeHost* primary_host = host_to_delete;
@@ -688,6 +692,11 @@ void DisplayController::OnDisplayRemoved(const gfx::Display& display) {
   // root window itself yet because the stack may be using it.
   controller->Shutdown();
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, controller);
+
+  // The window tree host should be erased at last because some handlers can
+  // access to the host through GetRootWindowForDisplayId() during
+  // MoveWindowsTo(). See http://crbug.com/415222
+  window_tree_hosts_.erase(display.id());
 }
 
 void DisplayController::OnDisplayMetricsChanged(const gfx::Display& display,
diff --git a/ash/display/display_controller_unittest.cc b/ash/display/display_controller_unittest.cc
index a753d1b..6945fac2 100644
--- a/ash/display/display_controller_unittest.cc
+++ b/ash/display/display_controller_unittest.cc
@@ -29,6 +29,9 @@
 #include "ui/events/test/event_generator.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/screen.h"
+#include "ui/views/mouse_watcher.h"
+#include "ui/views/mouse_watcher_view_host.h"
+#include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/wm/public/activation_change_observer.h"
 #include "ui/wm/public/activation_client.h"
@@ -375,6 +378,17 @@ std::string GetXWindowName(aura::WindowTreeHost* host) {
 }
 #endif
 
+class TestMouseWatcherListener : public views::MouseWatcherListener {
+ public:
+  TestMouseWatcherListener() {}
+
+ private:
+  // views::MouseWatcherListener:
+  void MouseMovedOutOfHost() override {}
+
+  DISALLOW_COPY_AND_ASSIGN(TestMouseWatcherListener);
+};
+
 }  // namespace
 
 typedef test::AshTestBase DisplayControllerTest;
@@ -1460,4 +1474,37 @@ TEST_F(DisplayControllerTest,
   EXPECT_EQ(gfx::Display::ROTATE_90, test_api.GetCurrentCursorRotation());
 }
 
+// GetRootWindowForDisplayId() for removed gfx::Display during
+// OnDisplayRemoved() should not cause crash. See http://crbug.com/415222
+TEST_F(DisplayControllerTest,
+       GetRootWindowForDisplayIdDuringDisplayDisconnection) {
+  if (!SupportsMultipleDisplays())
+    return;
+
+  UpdateDisplay("300x300,200x200");
+  aura::Window* root2 =
+      Shell::GetInstance()->display_controller()->GetRootWindowForDisplayId(
+          ScreenUtil::GetSecondaryDisplay().id());
+  views::Widget* widget = views::Widget::CreateWindowWithContextAndBounds(
+      nullptr, root2, gfx::Rect(350, 0, 100, 100));
+  views::View* view = new views::View();
+  widget->GetContentsView()->AddChildView(view);
+  view->SetBounds(0, 0, 100, 100);
+  widget->Show();
+
+  TestMouseWatcherListener listener;
+  views::MouseWatcher watcher(
+      new views::MouseWatcherViewHost(view, gfx::Insets()), &listener);
+  watcher.Start();
+
+  ui::test::EventGenerator event_generator(
+      widget->GetNativeWindow()->GetRootWindow());
+  event_generator.MoveMouseToCenterOf(widget->GetNativeWindow());
+
+  UpdateDisplay("300x300");
+  watcher.Stop();
+
+  widget->CloseNow();
+}
+
 }  // namespace ash