Merge "Incorrect behavior of View clear focus v2.0."

sganov · Android (Google) Code Review · commit 5dd53e1f121e · 2012-02-14T19:09:54.000-08:00
diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java
@@ -3788,6 +3788,11 @@ void clearFocusForRemoval() {
 
             onFocusChanged(false, 0, null);
             refreshDrawableState();
+
+            // The view cleared focus and invoked the callbacks, so  now is the
+            // time to give focus to the the first focusable from the top to
+            // ensure that the gain focus is announced after clear focus.
+            getRootView().requestFocus(FOCUS_FORWARD);
         }
     }
 
diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java
@@ -675,11 +675,14 @@ public void clearChildFocus(View child) {
      */
     @Override
     public void clearFocus() {
-        super.clearFocus();
-
-        // clear any child focus if it exists
-        if (mFocused != null) {
+        if (DBG) {
+            System.out.println(this + " clearFocus()");
+        }
+        if (mFocused == null) {
+            super.clearFocus();
+        } else {
             mFocused.clearFocus();
+            mFocused = null;
         }
     }
 
@@ -691,12 +694,12 @@ void unFocus() {
         if (DBG) {
             System.out.println(this + " unFocus()");
         }
-
-        super.unFocus();
-        if (mFocused != null) {
+        if (mFocused == null) {
+            super.unFocus();
+        } else {
             mFocused.unFocus();
+            mFocused = null;
         }
-        mFocused = null;
     }
 
     /**
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
@@ -174,6 +174,7 @@ public final class ViewRootImpl extends Handler implements ViewParent,
     View mView;
     View mFocusedView;
     View mRealFocusedView;  // this is not set to null in touch mode
+    View mOldFocusedView;
     int mViewVisibility;
     boolean mAppVisible = true;
     int mOrigWindowType = -1;
@@ -2272,32 +2273,33 @@ boolean scrollToRectOrFocus(Rect rectangle, boolean immediate) {
 
     public void requestChildFocus(View child, View focused) {
         checkThread();
-        if (mFocusedView != focused) {
-            mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(mFocusedView, focused);
-            scheduleTraversals();
+
+        if (DEBUG_INPUT_RESIZE) {
+            Log.v(TAG, "Request child focus: focus now " + focused);
         }
+
+        mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(mOldFocusedView, focused);
+        scheduleTraversals();
+
         mFocusedView = mRealFocusedView = focused;
-        if (DEBUG_INPUT_RESIZE) Log.v(TAG, "Request child focus: focus now "
-                + mFocusedView);
     }
 
     public void clearChildFocus(View child) {
         checkThread();
 
-        View oldFocus = mFocusedView;
+        if (DEBUG_INPUT_RESIZE) {
+            Log.v(TAG, "Clearing child focus");
+        }
+
+        mOldFocusedView = mFocusedView;
 
-        if (DEBUG_INPUT_RESIZE) Log.v(TAG, "Clearing child focus");
-        mFocusedView = mRealFocusedView = null;
-        if (mView != null && !mView.hasFocus()) {
-            // If a view gets the focus, the listener will be invoked from requestChildFocus()
-            if (!mView.requestFocus(View.FOCUS_FORWARD)) {
-                mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(oldFocus, null);
-            }
-        } else if (oldFocus != null) {
-            mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(oldFocus, null);
+        // Invoke the listener only if there is no view to take focus
+        if (focusSearch(null, View.FOCUS_FORWARD) == null) {
+            mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(mOldFocusedView, null);
         }
-    }
 
+        mFocusedView = mRealFocusedView = null;
+    }
 
     public void focusableViewAvailable(View v) {
         checkThread();
@@ -2770,6 +2772,7 @@ private boolean enterTouchMode() {
                         mView.unFocus();
                         mAttachInfo.mTreeObserver.dispatchOnGlobalFocusChange(focused, null);
                         mFocusedView = null;
+                        mOldFocusedView = null;
                         return true;
                     }
                 }
diff --git a/core/tests/coretests/Android.mk b/core/tests/coretests/Android.mk
@@ -22,7 +22,7 @@ LOCAL_SRC_FILES := \
 	$(call all-java-files-under, EnabledTestApp/src)
 
 LOCAL_DX_FLAGS := --core-library
-LOCAL_STATIC_JAVA_LIBRARIES := core-tests android-common frameworks-core-util-lib mockwebserver guava
+LOCAL_STATIC_JAVA_LIBRARIES := core-tests android-common frameworks-core-util-lib mockwebserver guava littlemock
 LOCAL_JAVA_LIBRARIES := android.test.runner
 LOCAL_PACKAGE_NAME := FrameworksCoreTests
 
diff --git a/core/tests/coretests/src/android/widget/focus/RequestFocus.java b/core/tests/coretests/src/android/widget/focus/RequestFocus.java
@@ -21,9 +21,7 @@
 import android.app.Activity;
 import android.os.Bundle;
 import android.os.Handler;
-import android.widget.LinearLayout;
 import android.widget.Button;
-import android.view.View;
 
 /**
  * Exercises cases where elements of the UI are requestFocus()ed.
diff --git a/core/tests/coretests/src/android/widget/focus/RequestFocusTest.java b/core/tests/coretests/src/android/widget/focus/RequestFocusTest.java
@@ -16,21 +16,28 @@
 
 package android.widget.focus;
 
-import android.widget.focus.RequestFocus;
-import com.android.frameworks.coretests.R;
+import static com.google.testing.littlemock.LittleMock.inOrder;
+import static com.google.testing.littlemock.LittleMock.mock;
 
 import android.os.Handler;
-import android.test.ActivityInstrumentationTestCase;
+import android.test.ActivityInstrumentationTestCase2;
+import android.test.UiThreadTest;
 import android.test.suitebuilder.annotation.LargeTest;
 import android.test.suitebuilder.annotation.MediumTest;
-import android.widget.Button;
 import android.util.AndroidRuntimeException;
+import android.view.View;
+import android.view.View.OnFocusChangeListener;
+import android.view.ViewTreeObserver.OnGlobalFocusChangeListener;
+import android.widget.Button;
+
+import com.android.frameworks.coretests.R;
+import com.google.testing.littlemock.LittleMock.InOrder;
 
 /**
  * {@link RequestFocusTest} is set up to exercise cases where the views that
  * have focus become invisible or GONE.
  */
-public class RequestFocusTest extends ActivityInstrumentationTestCase<RequestFocus> {
+public class RequestFocusTest extends ActivityInstrumentationTestCase2<RequestFocus> {
 
     private Button mTopLeftButton;
     private Button mBottomLeftButton;
@@ -39,7 +46,7 @@ public class RequestFocusTest extends ActivityInstrumentationTestCase<RequestFoc
     private Handler mHandler;
 
     public RequestFocusTest() {
-        super("com.android.frameworks.coretests", RequestFocus.class);
+        super(RequestFocus.class);
     }
 
     @Override
@@ -94,4 +101,90 @@ public void testWrongThreadRequestFocusFails() throws Exception {
                          e.getClass().getName());
         }
     }
+
+    /**
+     * This tests checks the case in which the first focusable View clears focus.
+     * In such a case the framework tries to give the focus to another View starting
+     * from the top. Hence, the framework will try to give focus to the view that
+     * wants to clear its focus.
+     *
+     * @throws Exception If an error occurs.
+     */
+    @UiThreadTest
+    public void testOnFocusChangeCallbackOrderWhenClearingFocusOfFirstFocusable()
+            throws Exception {
+        // Get the first focusable.
+        Button clearingFocusButton = mTopLeftButton;
+        Button gainingFocusButton = mTopLeftButton;
+
+        // Make sure that the clearing focus View is the first focusable.
+        View focusCandidate = clearingFocusButton.getRootView().getParent().focusSearch(null,
+                View.FOCUS_FORWARD);
+        assertSame("The clearing focus button is the first focusable.",
+                clearingFocusButton, focusCandidate);
+        assertSame("The gaining focus button is the first focusable.",
+                gainingFocusButton, focusCandidate);
+
+        // Focus the clearing focus button.
+        clearingFocusButton.requestFocus();
+        assertTrue(clearingFocusButton.hasFocus());
+
+        // Register the invocation order checker.
+        CombinedListeners mock = mock(CombinedListeners.class);
+        clearingFocusButton.setOnFocusChangeListener(mock);
+        gainingFocusButton.setOnFocusChangeListener(mock);
+        clearingFocusButton.getViewTreeObserver().addOnGlobalFocusChangeListener(mock);
+
+        // Try to clear focus.
+        clearingFocusButton.clearFocus();
+
+        // Check that no callback was invoked since focus did not move.
+        InOrder inOrder = inOrder(mock);
+        inOrder.verify(mock).onFocusChange(clearingFocusButton, false);
+        inOrder.verify(mock).onGlobalFocusChanged(clearingFocusButton, gainingFocusButton);
+        inOrder.verify(mock).onFocusChange(gainingFocusButton, true);
+    }
+
+    public interface CombinedListeners extends OnFocusChangeListener, OnGlobalFocusChangeListener {}
+
+    /**
+     * This tests check whether the on focus change callbacks are invoked in
+     * the proper order when a View loses focus and the framework gives it to
+     * the fist focusable one.
+     *
+     * @throws Exception
+     */
+    @UiThreadTest
+    public void testOnFocusChangeCallbackOrderWhenClearingFocusOfNotFirstFocusable()
+            throws Exception {
+        Button clearingFocusButton = mTopRightButton;
+        Button gainingFocusButton = mTopLeftButton;
+
+        // Make sure that the clearing focus View is not the first focusable.
+        View focusCandidate = clearingFocusButton.getRootView().getParent().focusSearch(null,
+                View.FOCUS_FORWARD);
+        assertNotSame("The clearing focus button is not the first focusable.",
+                clearingFocusButton, focusCandidate);
+        assertSame("The gaining focus button is the first focusable.",
+                gainingFocusButton, focusCandidate);
+
+        // Focus the clearing focus button.
+        clearingFocusButton.requestFocus();
+        assertTrue(clearingFocusButton.hasFocus());
+
+        // Register the invocation order checker.
+        CombinedListeners mock = mock(CombinedListeners.class);
+        clearingFocusButton.setOnFocusChangeListener(mock);
+        gainingFocusButton.setOnFocusChangeListener(mock);
+        clearingFocusButton.getViewTreeObserver().addOnGlobalFocusChangeListener(mock);
+
+        // Try to clear focus.
+        clearingFocusButton.clearFocus();
+
+        // Check that no callback was invoked since focus did not move.
+        InOrder inOrder = inOrder(mock);
+        inOrder.verify(mock).onFocusChange(clearingFocusButton, false);
+        inOrder.verify(mock).onGlobalFocusChanged(clearingFocusButton, gainingFocusButton);
+        inOrder.verify(mock).onFocusChange(gainingFocusButton, true);
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -3788,6 +3788,11 @@ void clearFocusForRemoval() {`
`3788`	`3788`
`3789`	`3789`	`onFocusChanged(false, 0, null);`
`3790`	`3790`	`refreshDrawableState();`
	`3791`	`+`
	`3792`	`+ // The view cleared focus and invoked the callbacks, so now is the`
	`3793`	`+ // time to give focus to the the first focusable from the top to`
	`3794`	`+ // ensure that the gain focus is announced after clear focus.`
	`3795`	`+ getRootView().requestFocus(FOCUS_FORWARD);`
`3791`	`3796`	`}`
`3792`	`3797`	`}`
`3793`	`3798`