mus: Fix activation to not attempt to activate hidden windows.

It should not be possible to activate hidden or detached windows.

BUG=548422
R=sky@chromium.org

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

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

2 files changed, 73 insertions, 2 deletions

diff --git a/components/mus/ws/focus_controller.cc b/components/mus/ws/focus_controller.cc
index 6f8530e..803b2ef 100644
--- a/components/mus/ws/focus_controller.cc
+++ b/components/mus/ws/focus_controller.cc
@@ -4,6 +4,7 @@
 
 #include "components/mus/ws/focus_controller.h"
 
+#include "components/mus/public/interfaces/window_manager.mojom.h"
 #include "components/mus/ws/focus_controller_delegate.h"
 #include "components/mus/ws/focus_controller_observer.h"
 #include "components/mus/ws/server_window.h"
@@ -138,11 +139,29 @@ bool FocusController::CanBeFocused(ServerWindow* window) const {
 
 bool FocusController::CanBeActivated(ServerWindow* window) const {
   DCHECK(window);
+  // A detached window cannot be activated.
+  if (!root_->Contains(window))
+    return false;
+
   // The parent window must be allowed to have active children.
   if (!delegate_->CanHaveActiveChildren(window->parent()))
     return false;
 
-  // TODO(sad): Implement this.
+  // The window must be drawn, or if it's not drawn, it must be minimized.
+  if (!window->IsDrawn()) {
+    bool is_minimized = false;
+    const ServerWindow::Properties& props = window->properties();
+    if (props.count(mojom::WindowManager::kShowState_Property)) {
+      is_minimized =
+          props.find(mojom::WindowManager::kShowState_Property)->second[0] ==
+          mus::mojom::SHOW_STATE_MINIMIZED;
+    }
+    if (!is_minimized)
+      return false;
+  }
+
+  // TODO(sad): If there's a transient modal window, then this cannot be
+  // activated.
   return true;
 }
 
diff --git a/components/mus/ws/focus_controller_unittest.cc b/components/mus/ws/focus_controller_unittest.cc
index 715e19a4..ee7da03 100644
--- a/components/mus/ws/focus_controller_unittest.cc
+++ b/components/mus/ws/focus_controller_unittest.cc
@@ -175,6 +175,58 @@ TEST(FocusControllerTest, FocusShiftsOnDestroy) {
   focus_controller.RemoveObserver(&focus_observer);
 }
 
-}  // namespace ws
+TEST(FocusControllerTest, ActivationSkipsOverHiddenWindow) {
+  TestServerWindowDelegate server_window_delegate;
+  ServerWindow parent(&server_window_delegate, WindowId());
+  server_window_delegate.set_root_window(&parent);
+  parent.SetVisible(true);
+
+  ServerWindow child_first(&server_window_delegate, WindowId());
+  ServerWindow child_second(&server_window_delegate, WindowId());
+  ServerWindow child_third(&server_window_delegate, WindowId());
+
+  parent.Add(&child_first);
+  parent.Add(&child_second);
+  parent.Add(&child_third);
+
+  child_first.SetVisible(true);
+  child_second.SetVisible(false);
+  child_third.SetVisible(true);
+
+  TestFocusControllerObserver focus_observer;
+  focus_observer.set_ignore_explicit(false);
+  FocusController focus_controller(&focus_observer, &parent);
+  focus_controller.AddObserver(&focus_observer);
+
+  // Since |child_second| is invisible, activation should cycle from
+  // |child_first|, to |child_third|, to |parent|, back to |child_first|.
+  focus_controller.ActivateNextWindow();
+  EXPECT_EQ(nullptr, focus_observer.old_active_window());
+  EXPECT_EQ(&child_first, focus_observer.new_active_window());
+  focus_observer.ClearAll();
+
+  focus_controller.ActivateNextWindow();
+  EXPECT_EQ(&child_first, focus_observer.old_active_window());
+  EXPECT_EQ(&child_third, focus_observer.new_active_window());
+  focus_observer.ClearAll();
+
+  focus_controller.ActivateNextWindow();
+  EXPECT_EQ(&child_third, focus_observer.old_active_window());
+  EXPECT_EQ(&parent, focus_observer.new_active_window());
+  focus_observer.ClearAll();
 
+  focus_controller.ActivateNextWindow();
+  EXPECT_EQ(&parent, focus_observer.old_active_window());
+  EXPECT_EQ(&child_first, focus_observer.new_active_window());
+  focus_observer.ClearAll();
+
+  // Once |child_second| is made visible, activation should go from
+  // |child_first| to |child_second|.
+  child_second.SetVisible(true);
+  focus_controller.ActivateNextWindow();
+  EXPECT_EQ(&child_first, focus_observer.old_active_window());
+  EXPECT_EQ(&child_second, focus_observer.new_active_window());
+}
+
+}  // namespace ws
 }  // namespace mus