overview: Fix DCHECK failure and crash, in clamshell/tablet transition

The present CL fixes what happens if you switch to tablet mode when the
following criteria are met:
1. There are at least two displays.
2. There is only one window.
3. The window is active.
4. The window is snapped on the primary display.

Without the present CL, NOTREACHED is hit (Issue 1024309), and there is
also a crash if DCHECKs are disabled (Issue 1024325).

Fixed: 1024309, 1024325
Change-Id: Ie1a6e378067076c7dbd8ec3ed388e70cd9382611
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1938463
Commit-Queue: Avery Musbach <amusbach@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721061}

ash/root_window_controller.cc

@@ -56,6 +56,8 @@
 #include "ash/wm/lock_action_handler_layout_manager.h"
 #include "ash/wm/lock_layout_manager.h"
 #include "ash/wm/overlay_layout_manager.h"
+#include "ash/wm/overview/overview_controller.h"
+#include "ash/wm/overview/overview_session.h"
 #include "ash/wm/root_window_layout_manager.h"
 #include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/splitview/split_view_utils.h"
@@ -618,6 +620,10 @@ void RootWindowController::CloseChildWindows() {
   // down associated layout managers.
   Shell::Get()->keyboard_controller()->OnRootWindowClosing(root);
 
+  OverviewController* overview_controller = Shell::Get()->overview_controller();
+  if (overview_controller && overview_controller->InOverviewSession())
+    overview_controller->overview_session()->OnRootWindowClosing(root);
+
   shelf_->ShutdownShelfWidget();
 
   ClearWorkspaceControllers(root);

ash/wm/overview/overview_session.cc

@@ -788,14 +788,18 @@ void OverviewSession::OnHighlightedItemClosed(OverviewItem* item) {
   item->CloseWindow();
 }
 
-void OverviewSession::OnDisplayAdded(const display::Display& display) {
-  EndOverview();
+void OverviewSession::OnRootWindowClosing(aura::Window* root) {
+  auto iter = std::find_if(grid_list_.begin(), grid_list_.end(),
+                           [root](std::unique_ptr<OverviewGrid>& grid) {
+                             return grid->root_window() == root;
+                           });
+  DCHECK(iter != grid_list_.end());
+  (*iter)->Shutdown();
+  grid_list_.erase(iter);
 }
 
-void OverviewSession::OnDisplayRemoved(const display::Display& display) {
-  // Removing a display causes a window activation which will end overview mode
-  // so that |OnDisplayRemoved| is never called.
-  NOTREACHED();
+void OverviewSession::OnDisplayAdded(const display::Display& display) {
+  EndOverview();
 }
 
 void OverviewSession::OnDisplayMetricsChanged(const display::Display& display,

ash/wm/overview/overview_session.h

@@ -274,9 +274,16 @@ class ASH_EXPORT OverviewSession : public display::DisplayObserver,
   void OnHighlightedItemActivated(OverviewItem* item);
   void OnHighlightedItemClosed(OverviewItem* item);
 
+  // Called explicitly (with no list of observers) by the |RootWindowController|
+  // of |root|, so that the associated grid is properly removed and destroyed.
+  // Note: Usually, when a display is removed, it causes a window activation
+  // which ends overview mode, and then this function does not get called. This
+  // function is only needed for when overview mode cannot be ended (see
+  // |OverviewController::CanEndOverview| and https://crbug.com/1024325).
+  void OnRootWindowClosing(aura::Window* root);
+
   // display::DisplayObserver:
   void OnDisplayAdded(const display::Display& display) override;
-  void OnDisplayRemoved(const display::Display& display) override;
   void OnDisplayMetricsChanged(const display::Display& display,
                                uint32_t metrics) override;
 

ash/wm/tablet_mode/tablet_mode_controller_unittest.cc

@@ -1407,6 +1407,24 @@ TEST_P(TabletModeControllerTest, StartTabletActiveLeftSnapPreviousLeftSnap) {
   EXPECT_EQ(window1.get(), window_util::GetActiveWindow());
 }
 
+// Like TabletModeControllerTest.StartTabletActiveLeftSnap, but with an extra
+// display which has no relevant windows on it.
+TEST_P(TabletModeControllerTest,
+       StartTabletActiveLeftSnapPlusExtraneousDisplay) {
+  UpdateDisplay("800x600,800x600");
+  std::unique_ptr<aura::Window> window = CreateDesktopWindowSnappedLeft();
+  tablet_mode_controller()->SetEnabledForTest(true);
+  EXPECT_EQ(2u, Shell::GetAllRootWindows().size());
+  // Make sure display mirroring triggers without any crashes.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1u, Shell::GetAllRootWindows().size());
+  EXPECT_EQ(SplitViewController::State::kLeftSnapped,
+            split_view_controller()->state());
+  EXPECT_EQ(window.get(), split_view_controller()->left_window());
+  EXPECT_TRUE(Shell::Get()->overview_controller()->InOverviewSession());
+  EXPECT_EQ(window.get(), window_util::GetActiveWindow());
+}
+
 TEST_P(TabletModeControllerTest, StartTabletActiveLeftSnapOnSecondaryDisplay) {
   UpdateDisplay("800x600,800x600");
   std::unique_ptr<aura::Window> window =