cros: Handle window destruction for visible on all desks windows.

Currently when a window destroys, it is not removed from
DesksController.visible_on_all_desks_windows_. This can cause bugs
where a destroyed window is returned.

This CL changes the WorkspaceLayoutManager to remove visible on all
desks windows when they are destroyed.

Test: manual + added
Bug: 1023794
Change-Id: If2b50fe87b5356044070dfc740809f80d268c24b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622396
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842333}

ash/wm/desks/desks_controller.cc

@@ -550,7 +550,7 @@ void DesksController::AddVisibleOnAllDesksWindow(aura::Window* window) {
   DCHECK(added);
 }
 
-void DesksController::RemoveVisibleOnAllDesksWindow(aura::Window* window) {
+void DesksController::MaybeRemoveVisibleOnAllDesksWindow(aura::Window* window) {
   visible_on_all_desks_windows_.erase(window);
 }
 

ash/wm/desks/desks_controller.h

@@ -165,7 +165,7 @@ class ASH_EXPORT DesksController : public DesksHelper,
   void AddVisibleOnAllDesksWindow(aura::Window* window);
 
   // Removes |window| if it is in |visible_on_all_desks_windows_|.
-  void RemoveVisibleOnAllDesksWindow(aura::Window* window);
+  void MaybeRemoveVisibleOnAllDesksWindow(aura::Window* window);
 
   // Reverts the name of the given |desk| to the default value (i.e. "Desk 1",
   // "Desk 2", ... etc.) according to its position in the |desks_| list, as if

ash/wm/desks/desks_unittests.cc

@@ -4064,6 +4064,30 @@ TEST_F(DesksBentoTest, VisibleOnAllDesksMoveWindowToDeskViaContextMenu) {
   EXPECT_TRUE(base::Contains(desk_2->windows(), window.get()));
 }
 
+// Tests that when a window that is visible on all desks is destroyed it is
+// removed from DesksController.visible_on_all_desks_windows_.
+TEST_F(DesksBentoTest, VisibleOnAllDesksWindowDestruction) {
+  auto* controller = DesksController::Get();
+  NewDesk();
+  const Desk* desk_1 = controller->desks()[0].get();
+  auto* root = Shell::GetPrimaryRootWindow();
+
+  auto window = CreateAppWindow(gfx::Rect(0, 0, 100, 100));
+  auto* widget = views::Widget::GetWidgetForNativeWindow(window.get());
+
+  // Assign |window| to all desks.
+  widget->SetVisibleOnAllWorkspaces(true);
+  ASSERT_TRUE(window->GetProperty(aura::client::kVisibleOnAllWorkspacesKey));
+  EXPECT_EQ(1u, controller->visible_on_all_desks_windows().size());
+  EXPECT_EQ(1u, desk_1->GetDeskContainerForRoot(root)->children().size());
+
+  // Destroy |window|. It should be removed from
+  // DesksController.visible_on_all_desks_windows_.
+  window.reset();
+  EXPECT_EQ(0u, controller->visible_on_all_desks_windows().size());
+  EXPECT_EQ(0u, desk_1->GetDeskContainerForRoot(root)->children().size());
+}
+
 // TODO(afakhry): Add more tests:
 // - Always on top windows are not tracked by any desk.
 // - Reusing containers when desks are removed and created.

ash/wm/workspace/workspace_layout_manager.cc

@@ -329,7 +329,7 @@ void WorkspaceLayoutManager::OnWindowPropertyChanged(aura::Window* window,
     if (window->GetProperty(aura::client::kVisibleOnAllWorkspacesKey))
       desks_controller->AddVisibleOnAllDesksWindow(window);
     else
-      desks_controller->RemoveVisibleOnAllDesksWindow(window);
+      desks_controller->MaybeRemoveVisibleOnAllDesksWindow(window);
   }
 }
 
@@ -348,6 +348,7 @@ void WorkspaceLayoutManager::OnWindowDestroying(aura::Window* window) {
     settings_bubble_container_ = nullptr;
   if (accessibility_bubble_container_ == window)
     accessibility_bubble_container_ = nullptr;
+  Shell::Get()->desks_controller()->MaybeRemoveVisibleOnAllDesksWindow(window);
 }
 
 void WorkspaceLayoutManager::OnWindowBoundsChanged(