overview: Do not show windows if their transient root is also shown.

Overview will show all windows related by transient on the same
thumbnail. Currently we will get blank spaces since the MRU list may
contain two windows related by transient, and the transient ancestor
or descendant of the most recent window will get transformed onto the
transient related windows thumbnail, leaving a blank space.

Introduce a function which removes all transient descendants from a
given list if their transient root exists in the same list. First
usage is overview but may need to extend to alt tab in future.

Test: ash_unittests WindowUtilTest.RemoveTransientDescendants
Bug: 844829
Change-Id: Ic2f9b476bc353b50a197be8fcf7bb3f2e3d44c49
Reviewed-on: https://chromium-review.googlesource.com/c/1355249
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612535}

ash/wm/overview/scoped_transform_overview_window.cc

@@ -50,12 +50,6 @@ bool immediate_close_for_tests = false;
 // Delay closing window to allow it to shrink and fade out.
 constexpr int kCloseWindowDelayInMilliseconds = 150;
 
-aura::Window* GetTransientRoot(aura::Window* window) {
-  while (window && ::wm::GetTransientParent(window))
-    window = ::wm::GetTransientParent(window);
-  return window;
-}
-
 ScopedTransformOverviewWindow::GridWindowFillMode GetWindowDimensionsType(
     aura::Window* window) {
   if (window->bounds().width() >
@@ -446,7 +440,7 @@ void ScopedTransformOverviewWindow::PrepareForOverview() {
 }
 
 void ScopedTransformOverviewWindow::CloseWidget() {
-  aura::Window* parent_window = GetTransientRoot(window_);
+  aura::Window* parent_window = ::wm::GetTransientRoot(window_);
   if (parent_window)
     wm::CloseWidgetForWindow(parent_window);
 }

ash/wm/overview/window_selector_controller.cc

@@ -272,6 +272,10 @@ bool WindowSelectorController::ToggleOverview(
       [](aura::Window* w) { return w->GetProperty(kHideInOverviewKey); });
   hide_windows.resize(end - hide_windows.begin());
   base::EraseIf(windows, wm::ShouldExcludeForOverview);
+  // Overview windows will handle showing their transient related windows, so if
+  // a window in |windows| has a transient root also in |windows|, we can remove
+  // it as the tranisent root will handle showing the window.
+  wm::RemoveTransientDescendants(&windows);
 
   // We may want to slide the overview grid in or out in some cases, even if
   // not explicitly stated.

ash/wm/window_transient_descendant_iterator.cc

@@ -3,22 +3,14 @@
 // found in the LICENSE file.
 
 #include "ash/wm/window_transient_descendant_iterator.h"
-#include <ui/aura/window.h>
-#include <ui/wm/core/window_util.h>
+
+#include "ash/wm/window_util.h"
+#include "ui/aura/window.h"
+#include "ui/wm/core/window_util.h"
 
 namespace ash {
 namespace wm {
 
-namespace {
-
-aura::Window* GetTransientRoot(aura::Window* window) {
-  while (window && ::wm::GetTransientParent(window))
-    window = ::wm::GetTransientParent(window);
-  return window;
-}
-
-}  // namespace
-
 WindowTransientDescendantIterator::WindowTransientDescendantIterator()
     : current_window_(nullptr) {}
 
@@ -76,7 +68,7 @@ WindowTransientDescendantIteratorRange::WindowTransientDescendantIteratorRange(
 WindowTransientDescendantIteratorRange GetTransientTreeIterator(
     aura::Window* window) {
   return WindowTransientDescendantIteratorRange(
-      WindowTransientDescendantIterator(GetTransientRoot(window)));
+      WindowTransientDescendantIterator(::wm::GetTransientRoot(window)));
 }
 
 }  // namespace wm

ash/wm/window_util.cc

@@ -5,7 +5,6 @@
 #include "ash/wm/window_util.h"
 
 #include <memory>
-#include <vector>
 
 #include "ash/public/cpp/ash_constants.h"
 #include "ash/public/cpp/shell_window_ids.h"
@@ -305,5 +304,17 @@ bool ShouldExcludeForOverview(const aura::Window* window) {
   return ShouldExcludeForBothCycleListAndOverview(window);
 }
 
+void RemoveTransientDescendants(std::vector<aura::Window*>* out_window_list) {
+  for (auto it = out_window_list->begin(); it != out_window_list->end();) {
+    aura::Window* transient_root = ::wm::GetTransientRoot(*it);
+    if (*it != transient_root &&
+        base::ContainsValue(*out_window_list, transient_root)) {
+      it = out_window_list->erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
 }  // namespace wm
 }  // namespace ash

ash/wm/window_util.h

@@ -6,6 +6,7 @@
 #define ASH_WM_WINDOW_UTIL_H_
 
 #include <stdint.h>
+#include <vector>
 
 #include "ash/ash_export.h"
 #include "ui/base/ui_base_types.h"
@@ -109,6 +110,12 @@ ASH_EXPORT bool ShouldExcludeForBothCycleListAndOverview(
 ASH_EXPORT bool ShouldExcludeForCycleList(const aura::Window* window);
 ASH_EXPORT bool ShouldExcludeForOverview(const aura::Window* window);
 
+// Removes all windows in |out_window_list| whose transient root is also in
+// |out_window_list|. This is used by overview and window cycler to avoid
+// showing multiple previews for windows linked by transient.
+ASH_EXPORT void RemoveTransientDescendants(
+    std::vector<aura::Window*>* out_window_list);
+
 }  // namespace wm
 }  // namespace ash
 

ash/wm/window_util_unittest.cc

@@ -9,6 +9,7 @@
 #include "ash/wm/window_state.h"
 #include "ui/aura/window.h"
 #include "ui/display/screen.h"
+#include "ui/wm/core/window_util.h"
 
 namespace ash {
 namespace wm {
@@ -141,5 +142,39 @@ TEST_F(WindowUtilTest, MoveWindowToDisplay) {
   EXPECT_EQ(original_root, window->GetRootWindow());
 }
 
+TEST_F(WindowUtilTest, RemoveTransientDescendants) {
+  // Create two windows which have no transient children or parents. Test that
+  // neither of them get removed when running RemoveTransientDescendants.
+  auto window1 = CreateTestWindow();
+  auto window2 = CreateTestWindow();
+  std::vector<aura::Window*> window_list = {window1.get(), window2.get()};
+  RemoveTransientDescendants(&window_list);
+  ASSERT_EQ(2u, window_list.size());
+
+  // Create two windows whose transient roots are |window1|. One is a direct
+  // transient child and one is a transient descendant. Test that both get
+  // removed when calling RemoveTransientDescendants.
+  auto descendant1 = CreateTestWindow();
+  auto descendant2 = CreateTestWindow();
+  ::wm::AddTransientChild(descendant1.get(), descendant2.get());
+  ::wm::AddTransientChild(window1.get(), descendant1.get());
+  window_list.push_back(descendant1.get());
+  window_list.push_back(descendant2.get());
+  RemoveTransientDescendants(&window_list);
+  ASSERT_EQ(2u, window_list.size());
+  ASSERT_TRUE(base::ContainsValue(window_list, window1.get()));
+  ASSERT_TRUE(base::ContainsValue(window_list, window2.get()));
+
+  // Create a window which has a transient parent that is not in |window_list|.
+  // Test that the window is not removed when calling
+  // RemoveTransientDescendants.
+  auto window3 = CreateTestWindow();
+  auto descendant3 = CreateTestWindow();
+  ::wm::AddTransientChild(window3.get(), descendant3.get());
+  window_list.push_back(descendant3.get());
+  RemoveTransientDescendants(&window_list);
+  EXPECT_EQ(3u, window_list.size());
+}
+
 }  // namespace wm
 }  // namespace ash

ui/wm/core/window_util.cc

@@ -178,7 +178,7 @@ aura::Window* GetTransientParent(aura::Window* window) {
 const aura::Window* GetTransientParent(const aura::Window* window) {
   const TransientWindowManager* manager =
       TransientWindowManager::GetIfExists(window);
-  return manager ? manager->transient_parent() : NULL;
+  return manager ? manager->transient_parent() : nullptr;
 }
 
 const std::vector<aura::Window*>& GetTransientChildren(
@@ -192,6 +192,12 @@ const std::vector<aura::Window*>& GetTransientChildren(
   return *shared;
 }
 
+aura::Window* GetTransientRoot(aura::Window* window) {
+  while (window && GetTransientParent(window))
+    window = GetTransientParent(window);
+  return window;
+}
+
 void AddTransientChild(aura::Window* parent, aura::Window* child) {
   TransientWindowManager::GetOrCreate(parent)->AddTransientChild(child);
 }

ui/wm/core/window_util.h

@@ -80,7 +80,7 @@ WM_CORE_EXPORT std::unique_ptr<ui::LayerTreeOwner> MirrorLayers(
 
 // Convenience functions that get the TransientWindowManager for the window and
 // redirect appropriately. These are preferable to calling functions on
-// TransientWindowManager as they handle the appropriate NULL checks.
+// TransientWindowManager as they handle the appropriate null checks.
 WM_CORE_EXPORT aura::Window* GetTransientParent(aura::Window* window);
 WM_CORE_EXPORT const aura::Window* GetTransientParent(
     const aura::Window* window);
@@ -90,6 +90,7 @@ WM_CORE_EXPORT void AddTransientChild(aura::Window* parent,
                                       aura::Window* child);
 WM_CORE_EXPORT void RemoveTransientChild(aura::Window* parent,
                                          aura::Window* child);
+WM_CORE_EXPORT aura::Window* GetTransientRoot(aura::Window* window);
 
 // Returns true if |window| has |ancestor| as a transient ancestor. A transient
 // ancestor is found by following the transient parent chain of the window.