Port more focus controller unit tests and fix our focus rules.

mojo/services/window_manager/basic_focus_rules.cc

Index: mojo/services/window_manager/basic_focus_rules.cc
diff --git a/mojo/services/window_manager/basic_focus_rules.cc b/mojo/services/window_manager/basic_focus_rules.cc
index 656abf2abc4acaee8a2e97e22cf75024c84354b8..6dbb8361e8aa6dc53f18f6c2f7a8c11ffbaa42c3 100644
--- a/mojo/services/window_manager/basic_focus_rules.cc
+++ b/mojo/services/window_manager/basic_focus_rules.cc
@@ -9,52 +9,160 @@
 
 namespace mojo {
 
+namespace {
+
+// TODO(erg): aura::Window::CanFocus() exists. mojo::View::CanFocus() does
+// not. This is a hack that does everything that Window::CanFocus() currently
+// does that doesn't require a delegate or an EventClient.
+bool ViewCanFocus(View* view) {

[sky, 2014/11/24 21:49:27]

This name is awful. CanFocusView is much better, but conflicts with the same
name in this class. How about CanFocusViewImpl?

+  // TODO(erg): In unit tests, views will never be drawn, so we can't rely on
+  // IsDrawn() here.
+  if (view->GetRoot() == view)

[sky, 2014/11/24 21:49:27]

GetRoot() is not the same as aura::Window::IsRootWindow. The equivalent of
aura::Window::IsRootWindow is something like:

view->view_manager()->GetRoots().contains(view);

[Elliot Glaysher, 2014/11/25 00:15:26]

Done.

To support this, I extracted the actual ViewManager interface from
ViewManagerClientImpl. Most of the usage of the interface was previously
static_cast<ViewManagerClientImpl*>(view_manager_), which obviously doesn't work
when you have a testing subclass of your interface.

+    return view->visible();
+
+  // TODO(erg): Add the intermediary delegate and event client checks once we

[sky, 2014/11/24 21:49:27]

This TODO makes it clearer why you have this structure, thanks.

+  // have those.
+
+  return ViewCanFocus(view->parent());
+}
+
+}  // namespace
+
 BasicFocusRules::BasicFocusRules(mojo::View* window_container)
     : window_container_(window_container) {
 }
 
 BasicFocusRules::~BasicFocusRules() {}
 
+bool BasicFocusRules::SupportsChildActivation(mojo::View* view) const {
+  return true;
+}
+
 bool BasicFocusRules::IsToplevelView(mojo::View* view) const {
-  return view->parent() == window_container_;
+  if (view->parent() != window_container_)
+    return false;
+
+  // The window must exist within a container that supports activation.
+  // The window cannot be blocked by a modal transient.
+  return SupportsChildActivation(view->parent());
 }
 
 bool BasicFocusRules::CanActivateView(mojo::View* view) const {
-  // TODO(erg): This needs to check visibility, along with focus, and several
-  // other things (see wm::BaseFocusRules).
-  return view->parent() == window_container_;
+  if (!view)
+    return true;
+
+  // Only toplevel windows can be activated
+  if (!IsToplevelView(view))
+    return false;
+
+  // The view must be visible.
+  if (!view->visible())
+    return false;
+
+  // TODO(erg): The aura version of this class asks the aura::Window's
+  // ActivationDelegate whether the window is activatable.
+
+  // A window must be focusable to be activatable. We don't call
+  // CanFocusWindow() from here because it will call back to us via
+  // GetActivatableWindow().
+  if (!ViewCanFocus(view))
+    return false;
+
+  // TODO(erg): In the aura version, we also check whether the window is
+  // blocked by a modal transient window.
+
+  return true;
 }
 
 bool BasicFocusRules::CanFocusView(mojo::View* view) const {
-  return true;
+  // It is possible to focus a NULL window, it is equivalent to clearing focus.
+  if (!view)
+    return true;
+
+  // The focused view is always inside the active view, so views that aren't
+  // activatable can't contain the focused view.
+  View* activatable = GetActivatableView(view);
+  if (!activatable || !activatable->Contains(view))
+    return false;
+  return ViewCanFocus(view);
 }
 
 mojo::View* BasicFocusRules::GetToplevelView(mojo::View* view) const {
-  while (view->parent() != window_container_) {
-    view = view->parent();
-    // Unparented hierarchy, there is no "top level" window.
-    if (!view)
-      return nullptr;
+  View* parent = view->parent();
+  View* child = view;
+  while (parent) {
+    if (IsToplevelView(child))
+      return child;
+
+    parent = parent->parent();
+    child = child->parent();
   }
 
-  return view;
+  return nullptr;
 }
 
 mojo::View* BasicFocusRules::GetActivatableView(mojo::View* view) const {
-  return GetToplevelView(view);
+  View* parent = view->parent();
+  View* child = view;
+  while (parent) {
+    if (CanActivateView(child))
+      return child;
+
+    // TODO(erg): In the aura version of this class, we have a whole bunch of
+    // checks to support modal transient windows, and transient parents.
+
+    parent = parent->parent();
+    child = child->parent();
+  }
+
+  return nullptr;
 }
 
 mojo::View* BasicFocusRules::GetFocusableView(mojo::View* view) const {
+  if (CanFocusView(view))
+    return view;
+
+  // |view| may be in a hierarchy that is non-activatable, in which case we
+  // need to cut over to the activatable hierarchy.
+  View* activatable = GetActivatableView(view);
+  if (!activatable) {
+    // There may not be a related activatable hierarchy to cut over to, in which
+    // case we try an unrelated one.
+    View* toplevel = GetToplevelView(view);
+    if (toplevel)
+      activatable = GetNextActivatableView(toplevel);
+    if (!activatable)
+      return nullptr;
+  }
+
+  if (!activatable->Contains(view)) {
+    // If there's already a child window focused in the activatable hierarchy,
+    // just use that (i.e. don't shift focus), otherwise we need to at least cut
+    // over to the activatable hierarchy.
+    View* focused = GetFocusableView(activatable);
+    return activatable->Contains(focused) ? focused : activatable;
+  }
+
+  while (view && !CanFocusView(view))
+    view = view->parent();
   return view;
 }
 
 mojo::View* BasicFocusRules::GetNextActivatableView(
     mojo::View* activatable) const {
-  const mojo::View::Children& children = activatable->parent()->children();
-  for (mojo::View::Children::const_reverse_iterator it = children.rbegin();
-       it != children.rend(); ++it) {
-    if (*it != activatable)
-      return *it;
+  DCHECK(activatable);
+
+  // In the basic scenarios handled by BasicFocusRules, the pool of activatable
+  // windows is limited to the |ignore|'s siblings.
+  const View::Children& siblings = activatable->parent()->children();
+  DCHECK(!siblings.empty());
+
+  for (auto rit = siblings.rbegin(); rit != siblings.rend(); ++rit) {
+    View* cur = *rit;
+    if (cur == activatable)
+      continue;
+    if (CanActivateView(cur))
+      return cur;
   }
   return nullptr;
 }