Don't alter accessibility JS APIs unless a page is about to load.

Previously we were adding and removing the APIs on window attach and
detach, but a page's life cycle is not related to attach/detach. This
patch also ensures that ChromeVox is not null before calling. Includes
formatting fixes and extra comments in the waitForResultTimedLocked
method to improve readability. Fixes parsing of integer resultId, which
was being parsed using Long.

Bug: 7328348
Change-Id: I6b81a8e4d8209f8e99419da9b8250abe57e25048

Author: alanv

core/java/android/webkit/AccessibilityInjector.java

@@ -97,9 +97,12 @@ class AccessibilityInjector {
     // Template for JavaScript that performs AndroidVox actions.
     private static final String ACCESSIBILITY_ANDROIDVOX_TEMPLATE =
             "(function() {" +
-                    "  if ((typeof(cvox) != 'undefined')"+
+                    "  if ((typeof(cvox) != 'undefined')" +
+                    "      && (cvox != null)" +
                     "      && (typeof(cvox.ChromeVox) != 'undefined')" +
+                    "      && (cvox.ChromeVox != null)" +
                     "      && (typeof(cvox.AndroidVox) != 'undefined')" +
+                    "      && (cvox.AndroidVox != null)" +
                     "      && cvox.ChromeVox.isActive) {" +
                     "    return cvox.AndroidVox.performAction('%1s');" +
                     "  } else {" +
@@ -110,9 +113,12 @@ class AccessibilityInjector {
     // JS code used to shut down an active AndroidVox instance.
     private static final String TOGGLE_CVOX_TEMPLATE =
             "javascript:(function() {" +
-                    "  if ((typeof(cvox) != 'undefined')"+
+                    "  if ((typeof(cvox) != 'undefined')" +
+                    "      && (cvox != null)" +
                     "      && (typeof(cvox.ChromeVox) != 'undefined')" +
-                    "      && (typeof(cvox.ChromeVox.host) != 'undefined')) {" +
+                    "      && (cvox.ChromeVox != null)" +
+                    "      && (typeof(cvox.ChromeVox.host) != 'undefined')" +
+                    "      && (cvox.ChromeVox.host != null)) {" +
                     "    cvox.ChromeVox.host.activateOrDeactivateChromeVox(%b);" +
                     "  }" +
                     "})();";
@@ -131,34 +137,61 @@ public AccessibilityInjector(WebViewClassic webViewClassic) {
         mAccessibilityManager = AccessibilityManager.getInstance(mContext);
     }
 
+    /**
+     * If JavaScript is enabled, pauses or resumes AndroidVox.
+     *
+     * @param enabled Whether feedback should be enabled.
+     */
+    public void toggleAccessibilityFeedback(boolean enabled) {
+        if (!isAccessibilityEnabled() || !isJavaScriptEnabled()) {
+            return;
+        }
+
+        toggleAndroidVox(enabled);
+
+        if (!enabled && (mTextToSpeech != null)) {
+            mTextToSpeech.stop();
+        }
+    }
+
     /**
      * Attempts to load scripting interfaces for accessibility.
      * <p>
-     * This should be called when the window is attached.
-     * </p>
+     * This should only be called before a page loads.
      */
-    public void addAccessibilityApisIfNecessary() {
+    private void addAccessibilityApisIfNecessary() {
         if (!isAccessibilityEnabled() || !isJavaScriptEnabled()) {
             return;
         }
 
         addTtsApis();
         addCallbackApis();
-        toggleAndroidVox(true);
     }
 
     /**
      * Attempts to unload scripting interfaces for accessibility.
      * <p>
-     * This should be called when the window is detached.
-     * </p>
+     * This should only be called before a page loads.
      */
-    public void removeAccessibilityApisIfNecessary() {
-        toggleAndroidVox(false);
+    private void removeAccessibilityApisIfNecessary() {
         removeTtsApis();
         removeCallbackApis();
     }
 
+    /**
+     * Destroys this accessibility injector.
+     */
+    public void destroy() {
+        if (mTextToSpeech != null) {
+            mTextToSpeech.shutdown();
+            mTextToSpeech = null;
+        }
+
+        if (mCallback != null) {
+            mCallback = null;
+        }
+    }
+
     private void toggleAndroidVox(boolean state) {
         if (!mAccessibilityScriptInjected) {
             return;
@@ -517,7 +550,12 @@ private String getScreenReaderInjectionUrl() {
      *         settings.
      */
     private boolean isJavaScriptEnabled() {
-        return mWebView.getSettings().getJavaScriptEnabled();
+        final WebSettings settings = mWebView.getSettings();
+        if (settings == null) {
+            return false;
+        }
+
+        return settings.getJavaScriptEnabled();
     }
 
     /**
@@ -732,7 +770,7 @@ private static class CallbackHandler {
         private final String mInterfaceName;
 
         private boolean mResult = false;
-        private long mResultId = -1;
+        private int mResultId = -1;
 
         private CallbackHandler(String interfaceName) {
             mInterfaceName = interfaceName;
@@ -784,34 +822,46 @@ private void clearResultLocked() {
          * @return Whether the result was received.
          */
         private boolean waitForResultTimedLocked(int resultId) {
-            if (DEBUG)
-                Log.d(TAG, "Waiting for CVOX result...");
-            long waitTimeMillis = RESULT_TIMEOUT;
             final long startTimeMillis = SystemClock.uptimeMillis();
+
+            if (DEBUG)
+                Log.d(TAG, "Waiting for CVOX result with ID " + resultId + "...");
+
             while (true) {
+                // Fail if we received a callback from the future.
+                if (mResultId > resultId) {
+                    if (DEBUG)
+                        Log.w(TAG, "Aborted CVOX result");
+                    return false;
+                }
+
+                final long elapsedTimeMillis = (SystemClock.uptimeMillis() - startTimeMillis);
+
+                // Succeed if we received the callback we were expecting.
+                if (DEBUG)
+                    Log.w(TAG, "Check " + mResultId + " versus expected " + resultId);
+                if (mResultId == resultId) {
+                    if (DEBUG)
+                        Log.w(TAG, "Received CVOX result after " + elapsedTimeMillis + " ms");
+                    return true;
+                }
+
+                final long waitTimeMillis = (RESULT_TIMEOUT - elapsedTimeMillis);
+
+                // Fail if we've already exceeded the timeout.
+                if (waitTimeMillis <= 0) {
+                    if (DEBUG)
+                        Log.w(TAG, "Timed out while waiting for CVOX result");
+                    return false;
+                }
+
                 try {
-                    if (mResultId == resultId) {
-                        if (DEBUG)
-                            Log.w(TAG, "Received CVOX result");
-                        return true;
-                    }
-                    if (mResultId > resultId) {
-                        if (DEBUG)
-                            Log.w(TAG, "Obsolete CVOX result");
-                        return false;
-                    }
-                    final long elapsedTimeMillis = SystemClock.uptimeMillis() - startTimeMillis;
-                    waitTimeMillis = RESULT_TIMEOUT - elapsedTimeMillis;
-                    if (waitTimeMillis <= 0) {
-                        if (DEBUG)
-                            Log.w(TAG, "Timed out while waiting for CVOX result");
-                        return false;
-                    }
+                    if (DEBUG)
+                        Log.w(TAG, "Start waiting...");
                     mResultLock.wait(waitTimeMillis);
                 } catch (InterruptedException ie) {
                     if (DEBUG)
                         Log.w(TAG, "Interrupted while waiting for CVOX result");
-                    /* ignore */
                 }
             }
         }
@@ -827,11 +877,11 @@ private boolean waitForResultTimedLocked(int resultId) {
         @SuppressWarnings("unused")
         public void onResult(String id, String result) {
             if (DEBUG)
-                Log.w(TAG, "Saw CVOX result of '" + result + "'");
-            final long resultId;
+                Log.w(TAG, "Saw CVOX result of '" + result + "' for ID " + id);
+            final int resultId;
 
             try {
-                resultId = Long.parseLong(id);
+                resultId = Integer.parseInt(id);
             } catch (NumberFormatException e) {
                 return;
             }
@@ -840,6 +890,9 @@ public void onResult(String id, String result) {
                 if (resultId > mResultId) {
                     mResult = Boolean.parseBoolean(result);
                     mResultId = resultId;
+                } else {
+                    if (DEBUG)
+                        Log.w(TAG, "Result with ID " + resultId + " was stale vesus " + mResultId);
                 }
                 mResultLock.notifyAll();
             }

core/java/android/webkit/WebViewClassic.java

@@ -2132,6 +2132,10 @@ private void ensureFunctorDetached() {
 
     private void destroyJava() {
         mCallbackProxy.blockMessages();
+        if (mAccessibilityInjector != null) {
+            mAccessibilityInjector.destroy();
+            mAccessibilityInjector = null;
+        }
         if (mWebViewCore != null) {
             // Tell WebViewCore to destroy itself
             synchronized (this) {
@@ -3967,8 +3971,6 @@ private boolean setContentScrollBy(int cx, int cy, boolean animate) {
         // null, and that will be the case
         mWebView.setCertificate(null);
 
-        // reset the flag since we set to true in if need after
-        // loading is see onPageFinished(Url)
         if (isAccessibilityInjectionEnabled()) {
             getAccessibilityInjector().onPageStarted(url);
         }
@@ -5384,7 +5386,7 @@ public void onAttachedToWindow() {
         if (mWebView.hasWindowFocus()) setActive(true);
 
         if (isAccessibilityInjectionEnabled()) {
-            getAccessibilityInjector().addAccessibilityApisIfNecessary();
+            getAccessibilityInjector().toggleAccessibilityFeedback(true);
         }
 
         updateHwAccelerated();
@@ -5397,11 +5399,7 @@ public void onDetachedFromWindow() {
         if (mWebView.hasWindowFocus()) setActive(false);
 
         if (isAccessibilityInjectionEnabled()) {
-            getAccessibilityInjector().removeAccessibilityApisIfNecessary();
-        } else {
-            // Ensure the injector is cleared if we're detaching from the window
-            // and accessibility is disabled.
-            mAccessibilityInjector = null;
+            getAccessibilityInjector().toggleAccessibilityFeedback(false);
         }
 
         updateHwAccelerated();