Complete the Focus Manager hookup.

- Temporarily patch in old Focus Sibling initialization code.
- Improve the constness of FocusSearch 
- Simplify ContainsView method in FocusManager

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/6349080

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@73632 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 154 insertions, 69 deletions

diff --git a/ui/views/focus/accelerator_handler_win.cc b/ui/views/focus/accelerator_handler_win.cc
index 02bd097..5efdab8 100644
--- a/ui/views/focus/accelerator_handler_win.cc
+++ b/ui/views/focus/accelerator_handler_win.cc
@@ -7,7 +7,7 @@
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/keycodes/keyboard_code_conversion_win.h"
 #include "ui/views/events/event.h"
-//#include "ui/views/focus/focus_manager.h"
+#include "ui/views/focus/focus_manager.h"
 
 namespace ui {
 
@@ -18,7 +18,6 @@ bool AcceleratorHandler::Dispatch(const MSG& msg) {
   bool process_message = true;
 
   if (msg.message >= WM_KEYFIRST && msg.message <= WM_KEYLAST) {
-    /*
     FocusManager* focus_manager =
         FocusManager::GetFocusManagerForNativeView(msg.hwnd);
     if (focus_manager) {
@@ -46,7 +45,6 @@ bool AcceleratorHandler::Dispatch(const MSG& msg) {
         }
       }
     }
-    */
   }
 
   if (process_message) {
diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc
index 79c1206..93c299f 100644
--- a/ui/views/focus/focus_manager.cc
+++ b/ui/views/focus/focus_manager.cc
@@ -161,35 +161,26 @@ void FocusManager::ValidateFocusedView() {
   }
 }
 
-// Tests whether a view is valid, whether it still belongs to the window
-// hierarchy of the FocusManager.
-bool FocusManager::ContainsView(View* view) {
-  DCHECK(view);
+bool FocusManager::ContainsView(View* view) const {
   Widget* widget = view->GetWidget();
-  if (!widget)
-    return false;
+  return widget ? widget->GetFocusManager() == this : false;
+}
 
-  gfx::NativeView top_window = widget_->native_widget()->GetNativeView();
-  gfx::NativeView window = widget->native_widget()->GetNativeView();
-  while (window) {
-    if (window == top_window)
-      return true;
-#if defined(OS_WIN)
-    window = ::GetParent(window);
-#else
-    window = gtk_widget_get_parent(window);
-#endif
-  }
-  return false;
+void FocusManager::RemoveView(View* view) {
+  // Clear focus if the removed child was focused.
+  if (focused_view_ == view)
+    ClearFocus();
+
+  ViewStorage::GetInstance()->RemoveView(view);
 }
 
 void FocusManager::AdvanceFocus(bool reverse) {
   View* v = GetNextFocusableView(focused_view_, reverse, false);
   // Note: Do not skip this next block when v == focused_view_.  If the user
-  // tabs past the last focusable element in a webpage, we'll get here, and if
+  // tabs past the last focusable element in a web page, we'll get here, and if
   // the TabContentsContainerView is the only focusable view (possible in
-  // fullscreen mode), we need to run this block in order to cycle around to the
-  // first element on the page.
+  // full-screen mode), we need to run this block in order to cycle around to
+  // the first element on the page.
   if (v) {
     v->OnFocus(); // TODO(beng): AboutToRequestFocusFromTabTraversal(reverse);
     SetFocusedViewWithReason(v, kReasonFocusTraversal);
@@ -201,7 +192,7 @@ View* FocusManager::GetNextFocusableView(View* original_starting_view,
                                          bool dont_loop) {
   FocusTraversable* focus_traversable = NULL;
 
-  // Let's revalidate the focused view.
+  // Let's re-validate the focused view.
   ValidateFocusedView();
 
   View* starting_view = NULL;
@@ -219,8 +210,6 @@ View* FocusManager::GetNextFocusableView(View* original_starting_view,
       pane_search = pane_search->parent();
     }
 
-    /*
-    TODO(beng): figure out traversal
     if (!focus_traversable) {
       if (!reverse) {
         // If the starting view has a focus traversable, use it.
@@ -229,22 +218,20 @@ View* FocusManager::GetNextFocusableView(View* original_starting_view,
 
         // Otherwise default to the root view.
         if (!focus_traversable) {
-          focus_traversable = original_starting_view->GetRootView();
+          focus_traversable =
+              original_starting_view->GetWidget()->GetFocusTraversable();
           starting_view = original_starting_view;
         }
       } else {
         // When you are going back, starting view's FocusTraversable
         // should not be used.
-        focus_traversable = original_starting_view->GetRootView();
+        focus_traversable =
+            original_starting_view->GetWidget()->GetFocusTraversable();
         starting_view = original_starting_view;
       }
     }
-    */
   } else {
-    /*
-    TODO(beng): figure out traversal
-    focus_traversable = widget_->GetRootView();
-    */
+    focus_traversable = widget_->GetFocusTraversable();
   }
 
   // Traverse the FocusTraversable tree down to find the focusable view.
@@ -339,7 +326,7 @@ void FocusManager::StoreFocusedView() {
   // TODO (jcampan): when a TabContents containing a popup is closed, the focus
   // is stored twice causing an assert. We should find a better alternative than
   // removing the view from the storage explicitly.
-  view_storage->RemoveView(stored_focused_view_storage_id_);
+  view_storage->RemoveViewByID(stored_focused_view_storage_id_);
 
   if (!focused_view_)
     return;
@@ -402,7 +389,7 @@ void FocusManager::ClearStoredFocusedView() {
     NOTREACHED();
     return;
   }
-  view_storage->RemoveView(stored_focused_view_storage_id_);
+  view_storage->RemoveViewByID(stored_focused_view_storage_id_);
 }
 
 // Find the next (previous if reverse is true) focusable view for the specified
diff --git a/ui/views/focus/focus_manager.h b/ui/views/focus/focus_manager.h
index f1bb19e..863b7d3 100644
--- a/ui/views/focus/focus_manager.h
+++ b/ui/views/focus/focus_manager.h
@@ -83,16 +83,16 @@ class FocusTraversable {
  public:
   // Return a FocusSearch object that implements the algorithm to find
   // the next or previous focusable view.
-  virtual FocusSearch* GetFocusSearch() = 0;
+  virtual const FocusSearch* GetFocusSearch() const = 0;
 
   // Should return the parent FocusTraversable.
   // The top RootView which is the top FocusTraversable returns NULL.
-  virtual FocusTraversable* GetFocusTraversableParent() = 0;
+  virtual FocusTraversable* GetFocusTraversableParent() const = 0;
 
   // This should return the View this FocusTraversable belongs to.
   // It is used when walking up the view hierarchy tree to find which view
   // should be used as the starting view for finding the next/previous view.
-  virtual View* GetFocusTraversableParentView() = 0;
+  virtual View* GetFocusTraversableParentView() const = 0;
 
  protected:
   virtual ~FocusTraversable() {}
@@ -188,7 +188,11 @@ class FocusManager {
 
   // Returns true is the specified is part of the hierarchy of the window
   // associated with this FocusManager.
-  bool ContainsView(View* view);
+  bool ContainsView(View* view) const;
+
+  // Stops tracking this View in the focus manager. If the View is focused,
+  // focus is cleared.
+  void RemoveView(View* view);
 
   // Advances the focus (backward if reverse is true).
   void AdvanceFocus(bool reverse);
diff --git a/ui/views/focus/focus_search.cc b/ui/views/focus/focus_search.cc
index 2050b1d..c8093fd0 100644
--- a/ui/views/focus/focus_search.cc
+++ b/ui/views/focus/focus_search.cc
@@ -20,7 +20,7 @@ View* FocusSearch::FindNextFocusableView(View* starting_view,
                                          Direction direction,
                                          bool check_starting_view,
                                          FocusTraversable** focus_traversable,
-                                         View** focus_traversable_view) {
+                                         View** focus_traversable_view) const {
   *focus_traversable = NULL;
   *focus_traversable_view = NULL;
 
@@ -94,20 +94,20 @@ View* FocusSearch::FindNextFocusableView(View* starting_view,
   return NULL;
 }
 
-bool FocusSearch::IsViewFocusableCandidate(View* v, int skip_group_id) {
+bool FocusSearch::IsViewFocusableCandidate(View* v, int skip_group_id) const {
   return IsFocusable(v) &&
       (v->IsGroupFocusTraversable() || skip_group_id == -1 ||
        v->group() != skip_group_id);
 }
 
-bool FocusSearch::IsFocusable(View* v) {
+bool FocusSearch::IsFocusable(View* v) const {
   if (accessibility_mode_)
     return v && v->IsAccessibilityFocusableInRootView();
 
   return v && v->IsFocusableInRootView();
 }
 
-View* FocusSearch::FindSelectedViewForGroup(View* view) {
+View* FocusSearch::FindSelectedViewForGroup(View* view) const {
   // No group for that view.
   if (view->IsGroupFocusTraversable() || view->group() == -1)
     return view;
@@ -120,7 +120,7 @@ View* FocusSearch::FindSelectedViewForGroup(View* view) {
   return view;
 }
 
-View* FocusSearch::GetParent(View* v) {
+View* FocusSearch::GetParent(View* v) const {
   return root_->Contains(v) ? v->parent() : NULL;
 }
 
@@ -139,7 +139,7 @@ View* FocusSearch::FindNextFocusableViewImpl(
     bool can_go_down,
     int skip_group_id,
     FocusTraversable** focus_traversable,
-    View** focus_traversable_view) {
+    View** focus_traversable_view) const {
   if (check_starting_view) {
     if (IsViewFocusableCandidate(starting_view, skip_group_id)) {
       View* v = FindSelectedViewForGroup(starting_view);
@@ -212,7 +212,7 @@ View* FocusSearch::FindPreviousFocusableViewImpl(
     bool can_go_down,
     int skip_group_id,
     FocusTraversable** focus_traversable,
-    View** focus_traversable_view) {
+    View** focus_traversable_view) const {
   // Let's go down and right as much as we can.
   if (can_go_down) {
     // Before we go into the direct children, we have to check if this view has
diff --git a/ui/views/focus/focus_search.h b/ui/views/focus/focus_search.h
index 0b0c749..3cda790 100644
--- a/ui/views/focus/focus_search.h
+++ b/ui/views/focus/focus_search.h
@@ -67,26 +67,26 @@ class FocusSearch {
                                       Direction direction,
                                       bool check_starting_view,
                                       FocusTraversable** focus_traversable,
-                                      View** focus_traversable_view);
+                                      View** focus_traversable_view) const;
 
  private:
   // Convenience method that returns true if a view is focusable and does not
   // belong to the specified group.
-  bool IsViewFocusableCandidate(View* v, int skip_group_id);
+  bool IsViewFocusableCandidate(View* v, int skip_group_id) const;
 
   // Convenience method; returns true if a view is not NULL and is focusable
   // (checking IsAccessibilityFocusableInRootView() if accessibility_mode_ is
   // true).
-  bool IsFocusable(View* v);
+  bool IsFocusable(View* v) const;
 
   // Returns the view selected for the group of the selected view. If the view
   // does not belong to a group or if no view is selected in the group, the
   // specified view is returned.
-  View* FindSelectedViewForGroup(View* view);
+  View* FindSelectedViewForGroup(View* view) const;
 
   // Get the parent, but stay within the root. Returns NULL if asked for
   // the parent of root_.
-  View* GetParent(View* view);
+  View* GetParent(View* view) const;
 
   // Returns the next focusable view or view containing a FocusTraversable
   // (NULL if none was found), starting at the starting_view.
@@ -99,7 +99,7 @@ class FocusSearch {
                                   bool can_go_down,
                                   int skip_group_id,
                                   FocusTraversable** focus_traversable,
-                                  View** focus_traversable_view);
+                                  View** focus_traversable_view) const;
 
   // Same as FindNextFocusableViewImpl but returns the previous focusable view.
   View* FindPreviousFocusableViewImpl(View* starting_view,
@@ -108,7 +108,7 @@ class FocusSearch {
                                       bool can_go_down,
                                       int skip_group_id,
                                       FocusTraversable** focus_traversable,
-                                      View** focus_traversable_view);
+                                      View** focus_traversable_view) const;
 
   View* root_;
   bool cycle_;
diff --git a/ui/views/focus/view_storage.cc b/ui/views/focus/view_storage.cc
index 504787a..e9c9799 100644
--- a/ui/views/focus/view_storage.cc
+++ b/ui/views/focus/view_storage.cc
@@ -34,7 +34,7 @@ void ViewStorage::StoreView(int storage_id, View* view) {
 
   if (iter != id_to_view_.end()) {
     NOTREACHED();
-    RemoveView(storage_id);
+    RemoveViewByID(storage_id);
   }
 
   id_to_view_[storage_id] = view;
@@ -58,14 +58,14 @@ View* ViewStorage::RetrieveView(int storage_id) {
   return iter->second;
 }
 
-void ViewStorage::RemoveView(int storage_id) {
+void ViewStorage::RemoveViewByID(int storage_id) {
   EraseView(storage_id, false);
 }
 
-void ViewStorage::ViewRemoved(View* parent, View* removed) {
+void ViewStorage::RemoveView(View* view) {
   // Let's first retrieve the ids for that view.
   std::map<View*, std::vector<int>*>::iterator ids_iter =
-      view_to_ids_.find(removed);
+      view_to_ids_.find(view);
 
   if (ids_iter == view_to_ids_.end()) {
     // That view is not in the view storage.
diff --git a/ui/views/focus/view_storage.h b/ui/views/focus/view_storage.h
index 40ecf18..1923f0b 100644
--- a/ui/views/focus/view_storage.h
+++ b/ui/views/focus/view_storage.h
@@ -38,10 +38,10 @@ class ViewStorage {
   View* RetrieveView(int storage_id);
 
   // Removes the view associated with |storage_id| if any.
-  void RemoveView(int storage_id);
+  void RemoveViewByID(int storage_id);
 
-  // Notifies the ViewStorage that a view was removed from its parent somewhere.
-  void ViewRemoved(View* parent, View* removed);
+  // Removes the specified View.
+  void RemoveView(View* view);
 
 #ifdef UNIT_TEST
   size_t view_count() const { return view_to_ids_.size(); }
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 77bd585..671692c 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -56,6 +56,8 @@ View::View()
       id_(-1),
       group_(-1),
       focusable_(false),
+      next_focusable_view_(NULL),
+      prev_focusable_view_(NULL),
       context_menu_controller_(NULL),
       drag_controller_(NULL) {
 }
@@ -223,6 +225,9 @@ void View::AddChildViewAt(View* view, size_t index) {
   if (view->parent())
     view->parent()->RemoveChildView(view);
 
+  // TODO(beng): Move focus initialization to FocusManager.
+  InitFocusSiblings(view, index);
+
   children_.insert(children_.begin() + index, view);
   view->parent_ = this;
 
@@ -532,6 +537,50 @@ void View::DragInfo::PossibleDrag(const gfx::Point& point) {
   press_point = point;
 }
 
+// Focus -----------------------------------------------------------------------
+
+// TODO(beng): Move to FocusManager.
+void View::InitFocusSiblings(View* view, size_t index) {
+  if (child_count() == 0) {
+    view->next_focusable_view_ = NULL;
+    view->prev_focusable_view_ = NULL;
+  } else {
+    if (index == child_count()) {
+      // We are inserting at the end, but the end of the child list may not be
+      // the last focusable element. Let's try to find an element with no next
+      // focusable element to link to.
+      View* last_focusable_view = NULL;
+      for (std::vector<View*>::iterator iter = children_.begin();
+           iter != children_.end(); ++iter) {
+          if (!(*iter)->next_focusable_view_) {
+            last_focusable_view = *iter;
+            break;
+          }
+      }
+      if (last_focusable_view == NULL) {
+        // Hum... there is a cycle in the focus list. Let's just insert ourself
+        // after the last child.
+        View* prev = children_[index - 1];
+        view->prev_focusable_view_ = prev;
+        view->next_focusable_view_ = prev->next_focusable_view_;
+        prev->next_focusable_view_->prev_focusable_view_ = view;
+        prev->next_focusable_view_ = view;
+      } else {
+        last_focusable_view->next_focusable_view_ = view;
+        view->next_focusable_view_ = NULL;
+        view->prev_focusable_view_ = last_focusable_view;
+      }
+    } else {
+      View* prev = children_[index]->GetPreviousFocusableView();
+      view->prev_focusable_view_ = prev;
+      view->next_focusable_view_ = children_[index];
+      if (prev)
+        prev->next_focusable_view_ = view;
+      children_[index]->prev_focusable_view_ = view;
+    }
+  }
+}
+
 // Painting --------------------------------------------------------------------
 
 void View::Paint(gfx::Canvas* canvas) {
diff --git a/ui/views/view.h b/ui/views/view.h
index aa51ce8..870ef92 100644
--- a/ui/views/view.h
+++ b/ui/views/view.h
@@ -311,6 +311,13 @@ class View {
     gfx::Point press_point;
   };
 
+  // Focus ---------------------------------------------------------------------
+
+  // Called when |child| is inserted into this View's children_ at |index|.
+  // Sets up next/previous focus views
+  // TODO(beng): Move this to FocusManager.
+  void InitFocusSiblings(View* child, size_t index);
+
   // Painting ------------------------------------------------------------------
 
   // Called by the framework to paint a View. Performs translation and clipping
@@ -343,6 +350,8 @@ class View {
                             bool is_add,
                             bool has_widget);
 
+  // TODO(beng): sort this section.
+
   // The View's parent view. This is set and reset when the View is added and
   // removed from a hierarchy.
   View* parent_;
@@ -376,6 +385,10 @@ class View {
   // True if this View is focusable by the FocusManager.
   bool focusable_;
 
+  // Focus siblings for this View.
+  View* next_focusable_view_;
+  View* prev_focusable_view_;
+
   // An optional helper that handles layout for child views.
   scoped_ptr<LayoutManager> layout_manager_;
 
@@ -395,7 +408,6 @@ class View {
 /*
 
 TODO(beng):
-- focus
 - accessibility
 - scrolling
 - cursors
@@ -408,6 +420,5 @@ TODO(beng):
 - investigate why assorted notifications are necessary
 - native_widget_views
 - native_widget_gtk
-- pick a name
 
 */
\ No newline at end of file
diff --git a/ui/views/widget/root_view.cc b/ui/views/widget/root_view.cc
index 1e11ee0..8a4f053 100644
--- a/ui/views/widget/root_view.cc
+++ b/ui/views/widget/root_view.cc
@@ -17,7 +17,10 @@ namespace internal {
 RootView::RootView(Widget* widget, View* contents_view)
     : widget_(widget),
       mouse_pressed_handler_(NULL),
-      mouse_move_handler_(NULL) {
+      mouse_move_handler_(NULL),
+      ALLOW_THIS_IN_INITIALIZER_LIST(focus_search_(this, false, false)),
+      focus_traversable_parent_(NULL),
+      focus_traversable_parent_view_(NULL) {
   SetLayoutManager(new FillLayout);
   AddChildView(contents_view);
 }
@@ -32,10 +35,7 @@ void RootView::OnViewRemoved(View* parent, View* child) {
   if (child == mouse_pressed_handler_)
     mouse_pressed_handler_ = NULL;
 
-  // Clear focus if the removed child was focused.
-  FocusManager* focus_manager = GetFocusManager();
-  if (focus_manager && focus_manager->focused_view() == child)
-    focus_manager->ClearFocus();
+  GetFocusManager()->RemoveView(child);
 }
 
 bool RootView::OnKeyPressed(const KeyEvent& event) {
@@ -145,6 +145,22 @@ Widget* RootView::GetWidget() const {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// RootView, FocusTraversable implementation:
+
+const FocusSearch* RootView::GetFocusSearch() const {
+  return &focus_search_;
+}
+
+FocusTraversable* RootView::GetFocusTraversableParent() const {
+  return focus_traversable_parent_;
+}
+
+View* RootView::GetFocusTraversableParentView() const {
+  return focus_traversable_parent_view_;
+}
+
+
+////////////////////////////////////////////////////////////////////////////////
 // RootView, private:
 
 }  // namespace internal
diff --git a/ui/views/widget/root_view.h b/ui/views/widget/root_view.h
index 308717c..2fe5c66 100644
--- a/ui/views/widget/root_view.h
+++ b/ui/views/widget/root_view.h
@@ -6,6 +6,8 @@
 #define UI_VIEWS_WIDGET_ROOT_VIEW_H_
 #pragma once
 
+#include "ui/views/focus/focus_manager.h"
+#include "ui/views/focus/focus_search.h"
 #include "ui/views/view.h"
 
 namespace ui {
@@ -17,7 +19,8 @@ namespace internal {
 //  A View subclass that owns a View hierarchy. Used by the Widget to perform
 //  View-specific event tracking.
 //
-class RootView : public View {
+class RootView : public View,
+                 public FocusTraversable {
  public:
   RootView(Widget* widget, View* contents_view);
   virtual ~RootView();
@@ -38,6 +41,11 @@ class RootView : public View {
   virtual Widget* GetWidget() const;
 
  private:
+  // Overridden from FocusTraversable:
+  virtual const FocusSearch* GetFocusSearch() const;
+  virtual FocusTraversable* GetFocusTraversableParent() const;
+  virtual View* GetFocusTraversableParentView() const;
+
   Widget* widget_;
 
   // The View that the mouse was pressed down on. Used to track drag operations
@@ -53,6 +61,12 @@ class RootView : public View {
   // operation.
   DragInfo drag_info_;
 
+  //
+  FocusSearch focus_search_;
+
+  FocusTraversable* focus_traversable_parent_;
+  View* focus_traversable_parent_view_;
+
   DISALLOW_COPY_AND_ASSIGN(RootView);
 };
 
diff --git a/ui/views/widget/widget.cc b/ui/views/widget/widget.cc
index 1759c6b..6fb70b3 100644
--- a/ui/views/widget/widget.cc
+++ b/ui/views/widget/widget.cc
@@ -123,6 +123,10 @@ FocusManager* Widget::GetFocusManager() const {
   return GetTopLevelWidget()->focus_manager_.get();
 }
 
+FocusTraversable* Widget::GetFocusTraversable() const {
+  return root_view_.get();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Widget, NativeWidgetListener implementation:
 
diff --git a/ui/views/widget/widget.h b/ui/views/widget/widget.h
index 42c5c32..56515d1 100644
--- a/ui/views/widget/widget.h
+++ b/ui/views/widget/widget.h
@@ -94,6 +94,8 @@ class Widget : public internal::NativeWidgetListener {
   // FocusManagers.
   FocusManager* GetFocusManager() const;
 
+  FocusTraversable* GetFocusTraversable() const;
+
   NativeWidget* native_widget() const { return native_widget_.get(); }
 
  private: