Handle WebKit selection changes synchronously.

This ensures that performing an accessibility navigation action correctly
returns true or false based on the new selected element.

Bug: 8027669
Change-Id: Ib7b3e1ec92ef522b23b073d32878dc1dc05723e9

4 files changed, 143 insertions, 89 deletions

diff --git a/core/java/android/webkit/AccessibilityInjector.java b/core/java/android/webkit/AccessibilityInjector.java
index 008a615b5bcb..8008a6bffc8e 100644
--- a/core/java/android/webkit/AccessibilityInjector.java
+++ b/core/java/android/webkit/AccessibilityInjector.java
@@ -318,12 +318,15 @@ class AccessibilityInjector {
     /**
      * Attempts to handle selection change events when accessibility is using a
      * non-JavaScript method.
+     * <p>
+     * This must not be called from the main thread.
      *
-     * @param selectionString The selection string.
+     * @param selection The selection string.
+     * @param token The selection request token.
      */
-    public void handleSelectionChangedIfNecessary(String selectionString) {
+    public void onSelectionStringChangedWebCoreThread(String selection, int token) {
         if (mAccessibilityInjectorFallback != null) {
-            mAccessibilityInjectorFallback.onSelectionStringChange(selectionString);
+            mAccessibilityInjectorFallback.onSelectionStringChangedWebCoreThread(selection, token);
         }
     }
 
diff --git a/core/java/android/webkit/AccessibilityInjectorFallback.java b/core/java/android/webkit/AccessibilityInjectorFallback.java
index 783b3db3b385..6417527b9630 100644
--- a/core/java/android/webkit/AccessibilityInjectorFallback.java
+++ b/core/java/android/webkit/AccessibilityInjectorFallback.java
@@ -27,8 +27,9 @@ import android.view.accessibility.AccessibilityManager;
 import android.view.accessibility.AccessibilityNodeInfo;
 import android.webkit.WebViewCore.EventHub;
 
+import com.android.internal.os.SomeArgs;
+
 import java.util.ArrayList;
-import java.util.Stack;
 
 /**
  * This class injects accessibility into WebViews with disabled JavaScript or
@@ -48,8 +49,7 @@ import java.util.Stack;
  * </p>
  * The possible actions are invocations to
  * {@link #setCurrentAxis(int, boolean, String)}, or
- * {@link #traverseCurrentAxis(int, boolean, String)}
- * {@link #traverseGivenAxis(int, int, boolean, String)}
+ * {@link #traverseGivenAxis(int, int, boolean, String, boolean)}
  * {@link #performAxisTransition(int, int, boolean, String)}
  * referred via the values of:
  * {@link #ACTION_SET_CURRENT_AXIS},
@@ -74,6 +74,9 @@ class AccessibilityInjectorFallback {
     private static final int ACTION_PERFORM_AXIS_TRANSITION = 3;
     private static final int ACTION_TRAVERSE_DEFAULT_WEB_VIEW_BEHAVIOR_AXIS = 4;
 
+    /** Timeout after which asynchronous granular movement is aborted. */
+    private static final int MODIFY_SELECTION_TIMEOUT = 500;
+
     // WebView navigation axes from WebViewCore.h, plus an additional axis for
     // the default behavior.
     private static final int NAVIGATION_AXIS_CHARACTER = 0;
@@ -81,7 +84,8 @@ class AccessibilityInjectorFallback {
     private static final int NAVIGATION_AXIS_SENTENCE = 2;
     @SuppressWarnings("unused")
     private static final int NAVIGATION_AXIS_HEADING = 3;
-    private static final int NAVIGATION_AXIS_SIBLING = 5;
+    @SuppressWarnings("unused")
+    private static final int NAVIGATION_AXIS_SIBLING = 4;
     @SuppressWarnings("unused")
     private static final int NAVIGATION_AXIS_PARENT_FIRST_CHILD = 5;
     private static final int NAVIGATION_AXIS_DOCUMENT = 6;
@@ -99,8 +103,11 @@ class AccessibilityInjectorFallback {
     private final WebViewClassic mWebView;
     private final WebView mWebViewInternal;
 
-    // events scheduled for sending as soon as we receive the selected text
-    private final Stack<AccessibilityEvent> mScheduledEventStack = new Stack<AccessibilityEvent>();
+    // Event scheduled for sending as soon as we receive the selected text.
+    private AccessibilityEvent mScheduledEvent;
+
+    // Token required to send the scheduled event.
+    private int mScheduledToken = 0;
 
     // the current traversal axis
     private int mCurrentAxis = 2; // sentence
@@ -114,6 +121,15 @@ class AccessibilityInjectorFallback {
     // keep track of last direction
     private int mLastDirection;
 
+    // Lock used for asynchronous selection callback.
+    private final Object mCallbackLock = new Object();
+
+    // Whether the asynchronous selection callback was received.
+    private boolean mCallbackReceived;
+
+    // Whether the asynchronous selection callback succeeded.
+    private boolean mCallbackResult;
+
     /**
      * Creates a new injector associated with a given {@link WebViewClassic}.
      *
@@ -174,8 +190,8 @@ class AccessibilityInjectorFallback {
                     }
                     mLastDirection = direction;
                     sendEvent = (binding.getSecondArgument(i) == 1);
-                    mLastDownEventHandled = traverseCurrentAxis(direction, sendEvent,
-                            contentDescription);
+                    mLastDownEventHandled = traverseGivenAxis(
+                            direction, mCurrentAxis, sendEvent, contentDescription, false);
                     break;
                 case ACTION_TRAVERSE_GIVEN_AXIS:
                     direction = binding.getFirstArgument(i);
@@ -187,7 +203,7 @@ class AccessibilityInjectorFallback {
                     mLastDirection = direction;
                     axis =  binding.getSecondArgument(i);
                     sendEvent = (binding.getThirdArgument(i) == 1);
-                    traverseGivenAxis(direction, axis, sendEvent, contentDescription);
+                    traverseGivenAxis(direction, axis, sendEvent, contentDescription, false);
                     mLastDownEventHandled = true;
                     break;
                 case ACTION_PERFORM_AXIS_TRANSITION:
@@ -207,7 +223,7 @@ class AccessibilityInjectorFallback {
                         mLastDirection = binding.getFirstArgument(i);
                         sendEvent = (binding.getSecondArgument(i) == 1);
                         traverseGivenAxis(mLastDirection, NAVIGATION_AXIS_DEFAULT_WEB_VIEW_BEHAVIOR,
-                            sendEvent, contentDescription);
+                            sendEvent, contentDescription, false);
                         mLastDownEventHandled = false;
                     } else {
                         mLastDownEventHandled = true;
@@ -222,8 +238,7 @@ class AccessibilityInjectorFallback {
     }
 
     /**
-     * Set the current navigation axis which will be used while
-     * calling {@link #traverseCurrentAxis(int, boolean, String)}.
+     * Set the current navigation axis.
      *
      * @param axis The axis to set.
      * @param sendEvent Whether to send an accessibility event to
@@ -255,20 +270,6 @@ class AccessibilityInjectorFallback {
         }
     }
 
-    /**
-     * Traverse the document along the current navigation axis.
-     *
-     * @param direction The direction of traversal.
-     * @param sendEvent Whether to send an accessibility event to
-     *        announce the change.
-     * @param contentDescription A description of the performed action.
-     * @see #setCurrentAxis(int, boolean, String)
-     */
-    private boolean traverseCurrentAxis(int direction, boolean sendEvent,
-            String contentDescription) {
-        return traverseGivenAxis(direction, mCurrentAxis, sendEvent, contentDescription);
-    }
-    
     boolean performAccessibilityAction(int action, Bundle arguments) {
         switch (action) {
             case AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY:
@@ -276,14 +277,14 @@ class AccessibilityInjectorFallback {
                 final int direction = getDirectionForAction(action);
                 final int axis = getAxisForGranularity(arguments.getInt(
                         AccessibilityNodeInfo.ACTION_ARGUMENT_MOVEMENT_GRANULARITY_INT));
-                return traverseGivenAxis(direction, axis, true, null);
+                return traverseGivenAxis(direction, axis, true, null, true);
             }
             case AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT:
             case AccessibilityNodeInfo.ACTION_PREVIOUS_HTML_ELEMENT: {
                 final int direction = getDirectionForAction(action);
                 // TODO: Add support for moving by object.
                 final int axis = NAVIGATION_AXIS_SENTENCE;
-                return traverseGivenAxis(direction, axis, true, null);
+                return traverseGivenAxis(direction, axis, true, null, true);
             }
             default:
                 return false;
@@ -293,7 +294,7 @@ class AccessibilityInjectorFallback {
     /**
      * Returns the {@link WebView}-defined direction for the given
      * {@link AccessibilityNodeInfo}-defined action.
-     * 
+     *
      * @param action An accessibility action identifier.
      * @return A web view navigation direction.
      */
@@ -313,7 +314,7 @@ class AccessibilityInjectorFallback {
     /**
      * Returns the {@link WebView}-defined axis for the given
      * {@link AccessibilityNodeInfo}-defined granularity.
-     * 
+     *
      * @param granularity An accessibility granularity identifier.
      * @return A web view navigation axis.
      */
@@ -345,20 +346,20 @@ class AccessibilityInjectorFallback {
      * @param contentDescription A description of the performed action.
      */
     private boolean traverseGivenAxis(int direction, int axis, boolean sendEvent,
-            String contentDescription) {
-        WebViewCore webViewCore = mWebView.getWebViewCore();
+            String contentDescription, boolean sychronous) {
+        final WebViewCore webViewCore = mWebView.getWebViewCore();
         if (webViewCore == null) {
             return false;
         }
 
-        AccessibilityEvent event = null;
         if (sendEvent) {
-            event = getPartialyPopulatedAccessibilityEvent(
+            final AccessibilityEvent event = getPartialyPopulatedAccessibilityEvent(
                     AccessibilityEvent.TYPE_VIEW_TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY);
-            // the text will be set upon receiving the selection string
+            // The text will be set upon receiving the selection string.
             event.setContentDescription(contentDescription);
+            mScheduledEvent = event;
+            mScheduledToken++;
         }
-        mScheduledEventStack.push(event);
 
         // if the axis is the default let WebView handle the event which will
         // result in cursor ring movement and selection of its content
@@ -366,27 +367,78 @@ class AccessibilityInjectorFallback {
             return false;
         }
 
-        webViewCore.sendMessage(EventHub.MODIFY_SELECTION, direction, axis);
-        return true;
+        final SomeArgs args = SomeArgs.obtain();
+        args.argi1 = direction;
+        args.argi2 = axis;
+        args.argi3 = mScheduledToken;
+
+        // If we don't need synchronous results, just return true.
+        if (!sychronous) {
+            webViewCore.sendMessage(EventHub.MODIFY_SELECTION, args);
+            return true;
+        }
+
+        final boolean callbackResult;
+
+        synchronized (mCallbackLock) {
+            mCallbackReceived = false;
+
+            // Asynchronously changes the selection in WebView, which responds by
+            // calling onSelectionStringChanged().
+            webViewCore.sendMessage(EventHub.MODIFY_SELECTION, args);
+
+            try {
+                mCallbackLock.wait(MODIFY_SELECTION_TIMEOUT);
+            } catch (InterruptedException e) {
+                // Do nothing.
+            }
+
+            callbackResult = mCallbackResult;
+        }
+
+        return (mCallbackReceived && callbackResult);
     }
 
-    /**
-     * Called when the <code>selectionString</code> has changed.
-     */
-    public void onSelectionStringChange(String selectionString) {
+    /* package */ void onSelectionStringChangedWebCoreThread(
+            final String selection, final int token) {
+        synchronized (mCallbackLock) {
+            mCallbackReceived = true;
+            mCallbackResult = (selection != null);
+            mCallbackLock.notifyAll();
+        }
+
+        // Managing state and sending events must take place on the UI thread.
+        mWebViewInternal.post(new Runnable() {
+            @Override
+            public void run() {
+                onSelectionStringChangedMainThread(selection, token);
+            }
+        });
+    }
+
+    private void onSelectionStringChangedMainThread(String selection, int token) {
         if (DEBUG) {
-            Log.d(LOG_TAG, "Selection string: " + selectionString);
+            Log.d(LOG_TAG, "Selection string: " + selection);
         }
-        mIsLastSelectionStringNull = (selectionString == null);
-        if (mScheduledEventStack.isEmpty()) {
+
+        if (token != mScheduledToken) {
+            if (DEBUG) {
+                Log.d(LOG_TAG, "Selection string has incorrect token: " + token);
+            }
             return;
         }
-        AccessibilityEvent event = mScheduledEventStack.pop();
-        if ((event != null) && (selectionString != null)) {
-            event.getText().add(selectionString);
+
+        mIsLastSelectionStringNull = (selection == null);
+
+        final AccessibilityEvent event = mScheduledEvent;
+        mScheduledEvent = null;
+
+        if ((event != null) && (selection != null)) {
+            event.getText().add(selection);
             event.setFromIndex(0);
-            event.setToIndex(selectionString.length());
+            event.setToIndex(selection.length());
             sendAccessibilityEvent(event);
+            event.recycle();
         }
     }
 
diff --git a/core/java/android/webkit/WebViewClassic.java b/core/java/android/webkit/WebViewClassic.java
index a867d39d158e..a4347a4829f5 100644
--- a/core/java/android/webkit/WebViewClassic.java
+++ b/core/java/android/webkit/WebViewClassic.java
@@ -1024,30 +1024,26 @@ public final class WebViewClassic implements WebViewProvider, WebViewProvider.Sc
     static final int UPDATE_MATCH_COUNT                 = 126;
     static final int CENTER_FIT_RECT                    = 127;
     static final int SET_SCROLLBAR_MODES                = 129;
-    static final int SELECTION_STRING_CHANGED           = 130;
-    static final int HIT_TEST_RESULT                    = 131;
-    static final int SAVE_WEBARCHIVE_FINISHED           = 132;
-
-    static final int SET_AUTOFILLABLE                   = 133;
-    static final int AUTOFILL_COMPLETE                  = 134;
-
-    static final int SCREEN_ON                          = 136;
-    static final int UPDATE_ZOOM_DENSITY                = 139;
-    static final int EXIT_FULLSCREEN_VIDEO              = 140;
-
-    static final int COPY_TO_CLIPBOARD                  = 141;
-    static final int INIT_EDIT_FIELD                    = 142;
-    static final int REPLACE_TEXT                       = 143;
-    static final int CLEAR_CARET_HANDLE                 = 144;
-    static final int KEY_PRESS                          = 145;
-    static final int RELOCATE_AUTO_COMPLETE_POPUP       = 146;
-    static final int FOCUS_NODE_CHANGED                 = 147;
-    static final int AUTOFILL_FORM                      = 148;
-    static final int SCROLL_EDIT_TEXT                   = 149;
-    static final int EDIT_TEXT_SIZE_CHANGED             = 150;
-    static final int SHOW_CARET_HANDLE                  = 151;
-    static final int UPDATE_CONTENT_BOUNDS              = 152;
-    static final int SCROLL_HANDLE_INTO_VIEW            = 153;
+    static final int HIT_TEST_RESULT                    = 130;
+    static final int SAVE_WEBARCHIVE_FINISHED           = 131;
+    static final int SET_AUTOFILLABLE                   = 132;
+    static final int AUTOFILL_COMPLETE                  = 133;
+    static final int SCREEN_ON                          = 134;
+    static final int UPDATE_ZOOM_DENSITY                = 135;
+    static final int EXIT_FULLSCREEN_VIDEO              = 136;
+    static final int COPY_TO_CLIPBOARD                  = 137;
+    static final int INIT_EDIT_FIELD                    = 138;
+    static final int REPLACE_TEXT                       = 139;
+    static final int CLEAR_CARET_HANDLE                 = 140;
+    static final int KEY_PRESS                          = 141;
+    static final int RELOCATE_AUTO_COMPLETE_POPUP       = 142;
+    static final int FOCUS_NODE_CHANGED                 = 143;
+    static final int AUTOFILL_FORM                      = 144;
+    static final int SCROLL_EDIT_TEXT                   = 145;
+    static final int EDIT_TEXT_SIZE_CHANGED             = 146;
+    static final int SHOW_CARET_HANDLE                  = 147;
+    static final int UPDATE_CONTENT_BOUNDS              = 148;
+    static final int SCROLL_HANDLE_INTO_VIEW            = 149;
 
     private static final int FIRST_PACKAGE_MSG_ID = SCROLL_TO_MSG_ID;
     private static final int LAST_PACKAGE_MSG_ID = HIT_TEST_RESULT;
@@ -1766,6 +1762,12 @@ public final class WebViewClassic implements WebViewProvider, WebViewProvider.Sc
         event.setMaxScrollY(Math.max(convertedContentHeight - adjustedViewHeight, 0));
     }
 
+    /* package */ void handleSelectionChangedWebCoreThread(String selection, int token) {
+        if (isAccessibilityInjectionEnabled()) {
+            getAccessibilityInjector().onSelectionStringChangedWebCoreThread(selection, token);
+        }
+    }
+
     private boolean isAccessibilityInjectionEnabled() {
         final AccessibilityManager manager = AccessibilityManager.getInstance(mContext);
         if (!manager.isEnabled()) {
@@ -7496,13 +7498,6 @@ public final class WebViewClassic implements WebViewProvider, WebViewProvider.Sc
                     mVerticalScrollBarMode = msg.arg2;
                     break;
 
-                case SELECTION_STRING_CHANGED:
-                    if (isAccessibilityInjectionEnabled()) {
-                        getAccessibilityInjector()
-                                .handleSelectionChangedIfNecessary((String) msg.obj);
-                    }
-                    break;
-
                 case FOCUS_NODE_CHANGED:
                     mIsEditingText = (msg.arg1 == mFieldPointer);
                     if (mAutoCompletePopup != null && !mIsEditingText) {
diff --git a/core/java/android/webkit/WebViewCore.java b/core/java/android/webkit/WebViewCore.java
index ccb84e6a02cd..4a09636aae4e 100644
--- a/core/java/android/webkit/WebViewCore.java
+++ b/core/java/android/webkit/WebViewCore.java
@@ -41,6 +41,8 @@ import android.view.View;
 import android.webkit.WebViewClassic.FocusNodeHref;
 import android.webkit.WebViewInputDispatcher.WebKitCallbacks;
 
+import com.android.internal.os.SomeArgs;
+
 import junit.framework.Assert;
 
 import java.io.OutputStream;
@@ -1545,12 +1547,14 @@ public final class WebViewCore {
                         case MODIFY_SELECTION:
                             mTextSelectionChangeReason
                                     = TextSelectionData.REASON_ACCESSIBILITY_INJECTOR;
-                            String modifiedSelectionString =
-                                nativeModifySelection(mNativeClass, msg.arg1,
-                                        msg.arg2);
-                            mWebViewClassic.mPrivateHandler.obtainMessage(
-                                    WebViewClassic.SELECTION_STRING_CHANGED,
-                                    modifiedSelectionString).sendToTarget();
+                            final SomeArgs args = (SomeArgs) msg.obj;
+                            final String modifiedSelectionString = nativeModifySelection(
+                                    mNativeClass, args.argi1, args.argi2);
+                            // If accessibility is on, the main thread may be
+                            // waiting for a response. Send on webcore thread.
+                            mWebViewClassic.handleSelectionChangedWebCoreThread(
+                                    modifiedSelectionString, args.argi3);
+                            args.recycle();
                             mTextSelectionChangeReason
                                     = TextSelectionData.REASON_UNKNOWN;
                             break;