Improve consistency in focusAfterDescendants behavior

- focusAfterDescendant ViewGroups that were also clusters would
  continue to be remembered for restoreFocusedInCluster after
  gaining focusable children. This caused undesired cluster-focus
  loops
- focusableViewAvailable wasn't being called in all cases (specifically
  when views were added). This meant focusAfterDescendant views
  wouldn't transfer focus to children in some cases.
- 0-area views which became focusable weren't reporting
  focusableViewAvailable. Since views gaining area doesn't report
  focusableViewAvailable, this was another case of state change not
  being accounted for. Also, this restriction doesn't exist for
  gaining focus so these views are actually focusable despite having
  0 area.
- focusableViewAvailable was reporting focusable views even when
  ancestors (and therefore the new focusables) were not visible.
- restoreFocusNotInCluster tried to focus invisible views
- restoreFocusNotInCluster was sending focus to focusAfterDescendant
  viewgroups which had focusable children IN a cluster.

Bug: 38010274
Bug: 38009598
Bug: 38352147
Test: cycling works in situations reported in bugs.
      Added CTS tests around focusableViewAvailable and
      FOCUS_AFTER_DESCENDANTS

Change-Id: I77f214f5384dcf9092324e08fc1daa3f1155bbad

2 files changed, 39 insertions, 18 deletions

diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
index 07258fc30704..3f3a4949e954 100644
--- a/core/java/android/view/View.java
+++ b/core/java/android/view/View.java
@@ -9957,12 +9957,20 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                 // Going from one cluster to another, so save last-focused.
                 // This covers cluster jumps because they are always FOCUS_DOWN
                 oldFocus.setFocusedInCluster(oldCluster);
+                if (!(oldFocus.mParent instanceof ViewGroup)) {
+                    return;
+                }
                 if (direction == FOCUS_FORWARD || direction == FOCUS_BACKWARD) {
                     // This is a result of ordered navigation so consider navigation through
                     // the previous cluster "complete" and clear its last-focused memory.
-                    if (oldFocus.mParent instanceof ViewGroup) {
-                        ((ViewGroup) oldFocus.mParent).clearFocusedInCluster(oldFocus);
-                    }
+                    ((ViewGroup) oldFocus.mParent).clearFocusedInCluster(oldFocus);
+                } else if (oldFocus instanceof ViewGroup
+                        && ((ViewGroup) oldFocus).getDescendantFocusability()
+                                == ViewGroup.FOCUS_AFTER_DESCENDANTS
+                        && ViewRootImpl.isViewDescendantOf(this, oldFocus)) {
+                    // This means oldFocus is not focusable since it obviously has a focusable
+                    // child (this). Don't restore focus to it in the future.
+                    ((ViewGroup) oldFocus.mParent).clearFocusedInCluster(oldFocus);
                 }
             }
         }
@@ -13009,7 +13017,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
                 // about in case nothing has focus.  even if this specific view
                 // isn't focusable, it may contain something that is, so let
                 // the root view try to give this focus if nothing else does.
-                if ((mParent != null) && (mBottom > mTop) && (mRight > mLeft)) {
+                if ((mParent != null)) {
                     mParent.focusableViewAvailable(this);
                 }
             }
diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
index 50593f28d9e3..b9c611fdf6d9 100644
--- a/core/java/android/view/ViewGroup.java
+++ b/core/java/android/view/ViewGroup.java
@@ -57,6 +57,7 @@ import android.view.animation.Animation;
 import android.view.animation.AnimationUtils;
 import android.view.animation.LayoutAnimationController;
 import android.view.animation.Transformation;
+
 import com.android.internal.R;
 
 import java.util.ArrayList;
@@ -832,8 +833,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
     public void focusableViewAvailable(View v) {
         if (mParent != null
                 // shortcut: don't report a new focusable view if we block our descendants from
-                // getting focus
+                // getting focus or if we're not visible
                 && (getDescendantFocusability() != FOCUS_BLOCK_DESCENDANTS)
+                && ((mViewFlags & VISIBILITY_MASK) == VISIBLE)
                 && (isFocusableInTouchMode() || !shouldBlockFocusForTouchscreen())
                 // shortcut: don't report a new focusable view if we already are focused
                 // (and we don't prefer our descendants)
@@ -1158,18 +1160,25 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
         // Determine whether we have a focused descendant.
         final int descendantFocusability = getDescendantFocusability();
         if (descendantFocusability != FOCUS_BLOCK_DESCENDANTS) {
-            final int count = mChildrenCount;
-            final View[] children = mChildren;
+            return hasFocusableChild(dispatchExplicit);
+        }
 
-            for (int i = 0; i < count; i++) {
-                final View child = children[i];
+        return false;
+    }
 
-                // In case the subclass has overridden has[Explicit]Focusable, dispatch
-                // to the expected one for each child even though we share logic here.
-                if ((dispatchExplicit && child.hasExplicitFocusable())
-                        || (!dispatchExplicit && child.hasFocusable())) {
-                    return true;
-                }
+    boolean hasFocusableChild(boolean dispatchExplicit) {
+        // Determine whether we have a focusable descendant.
+        final int count = mChildrenCount;
+        final View[] children = mChildren;
+
+        for (int i = 0; i < count; i++) {
+            final View child = children[i];
+
+            // In case the subclass has overridden has[Explicit]Focusable, dispatch
+            // to the expected one for each child even though we share logic here.
+            if ((dispatchExplicit && child.hasExplicitFocusable())
+                    || (!dispatchExplicit && child.hasFocusable())) {
+                return true;
             }
         }
 
@@ -3229,7 +3238,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
             // will refer to a view not-in a cluster.
             return restoreFocusInCluster(View.FOCUS_DOWN);
         }
-        if (isKeyboardNavigationCluster()) {
+        if (isKeyboardNavigationCluster() || (mViewFlags & VISIBILITY_MASK) != VISIBLE) {
             return false;
         }
         int descendentFocusability = getDescendantFocusability();
@@ -3247,7 +3256,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
                 return true;
             }
         }
-        if (descendentFocusability == FOCUS_AFTER_DESCENDANTS) {
+        if (descendentFocusability == FOCUS_AFTER_DESCENDANTS && !hasFocusableChild(false)) {
             return super.requestFocus(FOCUS_DOWN, null);
         }
         return false;
@@ -4913,7 +4922,8 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
             child.mParent = this;
         }
 
-        if (child.hasFocus()) {
+        final boolean childHasFocus = child.hasFocus();
+        if (childHasFocus) {
             requestChildFocus(child, child.findFocus());
         }
 
@@ -4926,6 +4936,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager
                 needGlobalAttributesUpdate(true);
             }
             ai.mKeepScreenOn = lastKeepOn;
+            if (!childHasFocus && child.hasFocusable()) {
+                focusableViewAvailable(child);
+            }
         }
 
         if (child.isLayoutDirectionInherited()) {