Use enums as arguments to FocusSearch to improve readability.

Also replace NULL with nullptr throughout FocusManager and
FocusSearch.

Bug: 764918
Change-Id: Ic90607c470130ac53375b26e601c1226c34a501a
Reviewed-on: https://chromium-review.googlesource.com/1024990Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552907}

ash/login/ui/lock_contents_view.cc

@@ -109,7 +109,11 @@ views::View* FindFirstOrLastFocusableChild(views::View* root, bool reverse) {
   views::FocusTraversable* dummy_focus_traversable;
   views::View* dummy_focus_traversable_view;
   return search.FindNextFocusableView(
-      root, reverse, views::FocusSearch::DOWN, false /*check_starting_view*/,
+      root,
+      reverse ? views::FocusSearch::SearchDirection::kBackwards
+              : views::FocusSearch::SearchDirection::kForwards,
+      views::FocusSearch::TraversalDirection::kDown,
+      views::FocusSearch::StartingViewPolicy::kSkipStartingView,
       &dummy_focus_traversable, &dummy_focus_traversable_view);
 }
 

ash/shelf/shelf_view.cc

@@ -126,12 +126,13 @@ class ShelfFocusSearch : public views::FocusSearch {
   ~ShelfFocusSearch() override = default;
 
   // views::FocusSearch:
-  View* FindNextFocusableView(View* starting_view,
-                              bool reverse,
-                              Direction direction,
-                              bool check_starting_view,
-                              views::FocusTraversable** focus_traversable,
-                              View** focus_traversable_view) override {
+  View* FindNextFocusableView(
+      View* starting_view,
+      FocusSearch::SearchDirection search_direction,
+      FocusSearch::TraversalDirection traversal_direction,
+      FocusSearch::StartingViewPolicy check_starting_view,
+      views::FocusTraversable** focus_traversable,
+      View** focus_traversable_view) override {
     int index = view_model_->GetIndexOfView(starting_view);
     // The back button (item with index 0 on the model) only exists in tablet
     // mode, so punt focus to the app list button (item with index 1 on the
@@ -143,7 +144,7 @@ class ShelfFocusSearch : public views::FocusSearch {
     // Increment or decrement index based on the cycle, unless we are at either
     // edge, then we loop to the back or front. Skip the back button (item with
     // index 0) when not in tablet mode.
-    if (reverse) {
+    if (search_direction == FocusSearch::SearchDirection::kBackwards) {
       --index;
       if (index < 0 || (index == 0 && !tablet_mode))
         index = view_model_->view_size() - 1;

ash/shelf/shelf_widget.cc

@@ -43,9 +43,12 @@ views::View* FindFirstOrLastFocusableChild(views::View* root,
   views::FocusTraversable* dummy_focus_traversable;
   views::View* dummy_focus_traversable_view;
   return search.FindNextFocusableView(
-      root, find_last_child, views::FocusSearch::DOWN,
-      false /*check_starting_view*/, &dummy_focus_traversable,
-      &dummy_focus_traversable_view);
+      root,
+      find_last_child ? views::FocusSearch::SearchDirection::kBackwards
+                      : views::FocusSearch::SearchDirection::kForwards,
+      views::FocusSearch::TraversalDirection::kDown,
+      views::FocusSearch::StartingViewPolicy::kSkipStartingView,
+      &dummy_focus_traversable, &dummy_focus_traversable_view);
 }
 
 }  // namespace

chrome/browser/ui/views/payments/payment_request_sheet_controller.cc

@@ -92,7 +92,9 @@ class SheetView : public views::View, public views::FocusTraversable {
       views::FocusTraversable* dummy_focus_traversable;
       views::View* dummy_focus_traversable_view;
       first_focusable = focus_search_->FindNextFocusableView(
-          nullptr, false, views::FocusSearch::DOWN, false,
+          nullptr, views::FocusSearch::SearchDirection::kForwards,
+          views::FocusSearch::TraversalDirection::kDown,
+          views::FocusSearch::StartingViewPolicy::kSkipStartingView,
           &dummy_focus_traversable, &dummy_focus_traversable_view);
     }
 

ui/views/accessible_pane_view.cc

@@ -25,8 +25,9 @@ class AccessiblePaneViewFocusSearch : public FocusSearch {
 
  protected:
   View* GetParent(View* v) override {
-    return accessible_pane_view_->ContainsForFocusSearch(root(), v) ?
-        accessible_pane_view_->GetParentForFocusSearch(v) : NULL;
+    return accessible_pane_view_->ContainsForFocusSearch(root(), v)
+               ? accessible_pane_view_->GetParentForFocusSearch(v)
+               : nullptr;
   }
 
   // Returns true if |v| is contained within the hierarchy rooted at |root|.
@@ -43,7 +44,7 @@ class AccessiblePaneViewFocusSearch : public FocusSearch {
 AccessiblePaneView::AccessiblePaneView()
     : pane_has_focus_(false),
       allow_deactivate_on_esc_(false),
-      focus_manager_(NULL),
+      focus_manager_(nullptr),
       home_key_(ui::VKEY_HOME, ui::EF_NONE),
       end_key_(ui::VKEY_END, ui::EF_NONE),
       escape_key_(ui::VKEY_ESCAPE, ui::EF_NONE),
@@ -109,7 +110,7 @@ bool AccessiblePaneView::SetPaneFocusAndFocusDefault() {
 }
 
 views::View* AccessiblePaneView::GetDefaultFocusableChild() {
-  return NULL;
+  return nullptr;
 }
 
 View* AccessiblePaneView::GetParentForFocusSearch(View* v) {
@@ -135,7 +136,9 @@ views::View* AccessiblePaneView::GetFirstFocusableChild() {
   FocusTraversable* dummy_focus_traversable;
   views::View* dummy_focus_traversable_view;
   return focus_search_->FindNextFocusableView(
-      NULL, false, views::FocusSearch::DOWN, false,
+      nullptr, FocusSearch::SearchDirection::kForwards,
+      FocusSearch::TraversalDirection::kDown,
+      FocusSearch::StartingViewPolicy::kSkipStartingView,
       &dummy_focus_traversable, &dummy_focus_traversable_view);
 }
 
@@ -143,7 +146,9 @@ views::View* AccessiblePaneView::GetLastFocusableChild() {
   FocusTraversable* dummy_focus_traversable;
   views::View* dummy_focus_traversable_view;
   return focus_search_->FindNextFocusableView(
-      this, true, views::FocusSearch::DOWN, false,
+      this, FocusSearch::SearchDirection::kBackwards,
+      FocusSearch::TraversalDirection::kDown,
+      FocusSearch::StartingViewPolicy::kSkipStartingView,
       &dummy_focus_traversable, &dummy_focus_traversable_view);
 }
 
@@ -154,7 +159,7 @@ views::FocusTraversable* AccessiblePaneView::GetPaneFocusTraversable() {
   if (pane_has_focus_)
     return this;
   else
-    return NULL;
+    return nullptr;
 }
 
 bool AccessiblePaneView::AcceleratorPressed(
@@ -247,12 +252,12 @@ views::FocusSearch* AccessiblePaneView::GetFocusSearch() {
 
 views::FocusTraversable* AccessiblePaneView::GetFocusTraversableParent() {
   DCHECK(pane_has_focus_);
-  return NULL;
+  return nullptr;
 }
 
 views::View* AccessiblePaneView::GetFocusTraversableParentView() {
   DCHECK(pane_has_focus_);
-  return NULL;
+  return nullptr;
 }
 
 }  // namespace views

ui/views/focus/focus_manager.cc

@@ -113,7 +113,7 @@ bool FocusManager::ContainsView(View* view) {
 }
 
 void FocusManager::AdvanceFocus(bool reverse) {
-  View* v = GetNextFocusableView(focused_view_, NULL, reverse, false);
+  View* v = GetNextFocusableView(focused_view_, nullptr, 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
   // the TabContentsContainerView is the only focusable view (possible in
@@ -199,9 +199,9 @@ View* FocusManager::GetNextFocusableView(View* original_starting_view,
   DCHECK(!focused_view_ || ContainsView(focused_view_))
       << " focus_view=" << focused_view_;
 
-  FocusTraversable* focus_traversable = NULL;
+  FocusTraversable* focus_traversable = nullptr;
 
-  View* starting_view = NULL;
+  View* starting_view = nullptr;
   if (original_starting_view) {
     // Search up the containment hierarchy to see if a view is acting as
     // a pane, and wants to implement its own focus traversable to keep
@@ -254,9 +254,14 @@ View* FocusManager::GetNextFocusableView(View* original_starting_view,
     FocusTraversable* new_focus_traversable = nullptr;
     View* new_starting_view = nullptr;
     // When we are going backward, the parent view might gain the next focus.
-    bool check_starting_view = reverse;
+    auto check_starting_view =
+        reverse ? FocusSearch::StartingViewPolicy::kCheckStartingView
+                : FocusSearch::StartingViewPolicy::kSkipStartingView;
     v = parent_focus_traversable->GetFocusSearch()->FindNextFocusableView(
-        starting_view, reverse, FocusSearch::UP, check_starting_view,
+        starting_view,
+        reverse ? FocusSearch::SearchDirection::kBackwards
+                : FocusSearch::SearchDirection::kForwards,
+        FocusSearch::TraversalDirection::kUp, check_starting_view,
         &new_focus_traversable, &new_starting_view);
 
     if (new_focus_traversable) {
@@ -281,7 +286,7 @@ View* FocusManager::GetNextFocusableView(View* original_starting_view,
     return nullptr;
 
   // Easy, just clear the selection and press tab again.
-  // By calling with NULL as the starting view, we'll start from either
+  // By calling with nullptr as the starting view, we'll start from either
   // the starting views widget or |widget_|.
   Widget* widget = original_starting_view->GetWidget();
   if (widget->widget_delegate()->ShouldAdvanceFocusToTopLevelWidget())
@@ -351,10 +356,10 @@ void FocusManager::SetFocusedViewWithReason(View* view,
 }
 
 void FocusManager::ClearFocus() {
-  // SetFocusedView(NULL) is going to clear out the stored view to. We need to
-  // persist it in this case.
+  // SetFocusedView(nullptr) is going to clear out the stored view to. We need
+  // to persist it in this case.
   views::View* focused_view = GetStoredFocusView();
-  SetFocusedView(NULL);
+  SetFocusedView(nullptr);
   ClearNativeFocus();
   SetStoredFocusView(focused_view);
 }
@@ -376,8 +381,8 @@ void FocusManager::AdvanceFocusIfNecessary() {
 
 void FocusManager::StoreFocusedView(bool clear_native_focus) {
   View* focused_view = focused_view_;
-  // Don't do anything if no focused view. Storing the view (which is NULL), in
-  // this case, would clobber the view that was previously saved.
+  // Don't do anything if no focused view. Storing the view (which is nullptr),
+  // in this case, would clobber the view that was previously saved.
   if (!focused_view_)
     return;
 
@@ -392,7 +397,7 @@ void FocusManager::StoreFocusedView(bool clear_native_focus) {
     // ClearFocus() also stores the focused view.
     ClearFocus();
   } else {
-    SetFocusedView(NULL);
+    SetFocusedView(nullptr);
     SetStoredFocusView(focused_view);
   }
 
@@ -440,20 +445,28 @@ View* FocusManager::GetStoredFocusView() {
 View* FocusManager::FindFocusableView(FocusTraversable* focus_traversable,
                                       View* starting_view,
                                       bool reverse) {
-  FocusTraversable* new_focus_traversable = NULL;
-  View* new_starting_view = NULL;
+  FocusTraversable* new_focus_traversable = nullptr;
+  View* new_starting_view = nullptr;
   View* v = focus_traversable->GetFocusSearch()->FindNextFocusableView(
-      starting_view, reverse, FocusSearch::DOWN, false, &new_focus_traversable,
-      &new_starting_view);
+      starting_view,
+      reverse ? FocusSearch::SearchDirection::kBackwards
+              : FocusSearch::SearchDirection::kForwards,
+      FocusSearch::TraversalDirection::kDown,
+      FocusSearch::StartingViewPolicy::kSkipStartingView,
+      &new_focus_traversable, &new_starting_view);
 
   // Let's go down the FocusTraversable tree as much as we can.
   while (new_focus_traversable) {
     DCHECK(!v);
     focus_traversable = new_focus_traversable;
-    new_focus_traversable = NULL;
-    starting_view = NULL;
+    new_focus_traversable = nullptr;
+    starting_view = nullptr;
     v = focus_traversable->GetFocusSearch()->FindNextFocusableView(
-        starting_view, reverse, FocusSearch::DOWN, false,
+        starting_view,
+        reverse ? FocusSearch::SearchDirection::kBackwards
+                : FocusSearch::SearchDirection::kForwards,
+        FocusSearch::TraversalDirection::kDown,
+        FocusSearch::StartingViewPolicy::kSkipStartingView,
         &new_focus_traversable, &new_starting_view);
   }
   return v;
@@ -499,7 +512,7 @@ void FocusManager::ViewRemoved(View* removed) {
   // be called while the top level widget is being destroyed.
   DCHECK(removed);
   if (removed->Contains(focused_view_))
-    SetFocusedView(NULL);
+    SetFocusedView(nullptr);
 }
 
 void FocusManager::AddFocusChangeListener(FocusChangeListener* listener) {

ui/views/focus/focus_search.h

@@ -21,9 +21,19 @@ class VIEWS_EXPORT FocusSearch {
   // goal is to switch to focusable views on the same level when using the arrow
   // keys (ala Windows: in a dialog box, arrow keys typically move between the
   // dialog OK, Cancel buttons).
-  enum Direction {
-    UP = 0,
-    DOWN
+  enum class TraversalDirection {
+    kUp,
+    kDown,
+  };
+
+  enum class SearchDirection {
+    kForwards,
+    kBackwards,
+  };
+
+  enum class StartingViewPolicy {
+    kSkipStartingView,
+    kCheckStartingView,
   };
 
   // Constructor.
@@ -52,20 +62,22 @@ class VIEWS_EXPORT FocusSearch {
   // - |starting_view| is the view that should be used as the starting point
   //   when looking for the previous/next view. It may be NULL (in which case
   //   the first/last view should be used depending if normal/reverse).
-  // - |reverse| whether we should find the next (reverse is false) or the
-  //   previous (reverse is true) view.
-  // - |direction| specifies whether we are traversing down (meaning we should
-  //   look into child views) or traversing up (don't look at child views).
-  // - |check_starting_view| is true if starting_view may obtain the next focus.
+  // - |search_direction| whether we should find the next (kForwards) or
+  //   previous (kReverse) view.
+  // - |traversal_direction| specifies whether we are traversing down (meaning
+  //   we should look into child views) or traversing up (don't look at
+  //   child views).
+  // - |check_starting_view| indicated if starting_view may obtain the next
+  //   focus.
   // - |focus_traversable| is set to the focus traversable that should be
   //   traversed if one is found (in which case the call returns NULL).
   // - |focus_traversable_view| is set to the view associated with the
   //   FocusTraversable set in the previous parameter (it is used as the
   //   starting view when looking for the next focusable view).
   virtual View* FindNextFocusableView(View* starting_view,
-                                      bool reverse,
-                                      Direction direction,
-                                      bool check_starting_view,
+                                      SearchDirection search_direction,
+                                      TraversalDirection traversal_direction,
+                                      StartingViewPolicy check_starting_view,
                                       FocusTraversable** focus_traversable,
                                       View** focus_traversable_view);
 
@@ -102,7 +114,7 @@ class VIEWS_EXPORT FocusSearch {
   // traversal of the views hierarchy. |skip_group_id| specifies a group_id,
   // -1 means no group. All views from a group are traversed in one pass.
   View* FindNextFocusableViewImpl(View* starting_view,
-                                  bool check_starting_view,
+                                  StartingViewPolicy check_starting_view,
                                   bool can_go_up,
                                   bool can_go_down,
                                   int skip_group_id,
@@ -111,7 +123,7 @@ class VIEWS_EXPORT FocusSearch {
 
   // Same as FindNextFocusableViewImpl but returns the previous focusable view.
   View* FindPreviousFocusableViewImpl(View* starting_view,
-                                      bool check_starting_view,
+                                      StartingViewPolicy check_starting_view,
                                       bool can_go_up,
                                       bool can_go_down,
                                       int skip_group_id,