Prevent non-visible views from getting focused.

BUG=48719
TEST=FocusManagerTest.TraversalWithNonVisibleViews

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

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

2 files changed, 56 insertions, 4 deletions

diff --git a/views/focus/focus_manager_unittest.cc b/views/focus/focus_manager_unittest.cc
index ae585a7..11e982a 100644
--- a/views/focus/focus_manager_unittest.cc
+++ b/views/focus/focus_manager_unittest.cc
@@ -1114,8 +1114,59 @@ TEST_F(FocusTraversalTest, TraversalWithNonEnabledViews) {
   for (size_t i = 0; i < arraysize(kDisabledIDs); i++) {
     View* v = FindViewByID(kDisabledIDs[i]);
     ASSERT_TRUE(v != NULL);
-    if (v)
-      v->SetEnabled(false);
+    v->SetEnabled(false);
+  }
+
+  // Uncomment the following line if you want to test manually the UI of this
+  // test.
+  // MessageLoopForUI::current()->Run(new AcceleratorHandler());
+
+  View* focused_view;
+  // Let's do one traversal (several times, to make sure it loops ok).
+  GetFocusManager()->ClearFocus();
+  for (int i = 0; i < 3; ++i) {
+    for (size_t j = 0; j < arraysize(kTraversalIDs); j++) {
+      GetFocusManager()->AdvanceFocus(false);
+      focused_view = GetFocusManager()->GetFocusedView();
+      EXPECT_TRUE(focused_view != NULL);
+      if (focused_view)
+        EXPECT_EQ(kTraversalIDs[j], focused_view->GetID());
+    }
+  }
+
+  // Same thing in reverse.
+  GetFocusManager()->ClearFocus();
+  for (int i = 0; i < 3; ++i) {
+    for (int j = arraysize(kTraversalIDs) - 1; j >= 0; --j) {
+      GetFocusManager()->AdvanceFocus(true);
+      focused_view = GetFocusManager()->GetFocusedView();
+      EXPECT_TRUE(focused_view != NULL);
+      if (focused_view)
+        EXPECT_EQ(kTraversalIDs[j], focused_view->GetID());
+    }
+  }
+}
+
+TEST_F(FocusTraversalTest, TraversalWithInvisibleViews) {
+  const int kInvisibleIDs[] = { kTopCheckBoxID, kOKButtonID,
+      kThumbnailContainerID };
+
+  const int kTraversalIDs[] = { kAppleTextfieldID, kOrangeTextfieldID,
+      kBananaTextfieldID, kKiwiTextfieldID, kFruitButtonID, kFruitCheckBoxID,
+      kComboboxID, kBroccoliButtonID, kRosettaLinkID,
+      kStupeurEtTremblementLinkID, kDinerGameLinkID, kRidiculeLinkID,
+      kClosetLinkID, kVisitingLinkID, kAmelieLinkID, kJoyeuxNoelLinkID,
+      kCampingLinkID, kBriceDeNiceLinkID, kTaxiLinkID, kAsterixLinkID,
+      kCancelButtonID, kHelpButtonID, kStyleContainerID, kBoldCheckBoxID,
+      kItalicCheckBoxID, kUnderlinedCheckBoxID, kStyleHelpLinkID,
+      kStyleTextEditID, kSearchTextfieldID, kSearchButtonID, kHelpLinkID };
+
+
+  // Let's make some views invisible.
+  for (size_t i = 0; i < arraysize(kInvisibleIDs); i++) {
+    View* v = FindViewByID(kInvisibleIDs[i]);
+    ASSERT_TRUE(v != NULL);
+    v->SetVisible(false);
   }
 
   // Uncomment the following line if you want to test manually the UI of this
diff --git a/views/view.cc b/views/view.cc
index d3a9b98..69cf67f 100644
--- a/views/view.cc
+++ b/views/view.cc
@@ -264,7 +264,7 @@ void View::SetEnabled(bool state) {
 }
 
 bool View::IsFocusable() const {
-  return focusable_ && IsEnabled() && IsVisible();
+  return focusable_ && IsEnabled() && IsVisibleInRootView();
 }
 
 void View::SetFocusable(bool focusable) {
@@ -272,7 +272,8 @@ void View::SetFocusable(bool focusable) {
 }
 
 bool View::IsAccessibilityFocusable() const {
-  return (focusable_ || accessibility_focusable_) && IsEnabled() && IsVisible();
+  return (focusable_ || accessibility_focusable_) && IsEnabled() &&
+      IsVisibleInRootView();
 }
 
 FocusManager* View::GetFocusManager() {