Dynamically enable spatial navigation based on events

android_webview/java/src/org/chromium/android_webview/AwSettings.java

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.android_webview;
 
 import android.content.Context;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
@@ skipping 63 matching lines @@
     private boolean mSupportMultipleWindows = false;
     private PluginState mPluginState = PluginState.OFF;
     private boolean mAppCacheEnabled = false;
     private boolean mDomStorageEnabled = false;
     private boolean mDatabaseEnabled = false;
     private boolean mUseWideViewport = false;
     private boolean mLoadWithOverviewMode = false;
     private boolean mMediaPlaybackRequiresUserGesture = true;
     private String mDefaultVideoPosterURL;
     private float mInitialPageScalePercent = 0;
+    private boolean mSpatialNavigationEnabled;  // Default depends on device feautres.

[sgurun-gerrit only, 2013/10/22 17:18:00]

s/feautres/features/

[joth, 2013/10/22 22:02:41]

Done.

 
     private final boolean mSupportLegacyQuirks;
 
     private final boolean mPasswordEchoEnabled;
 
     // Not accessed by the native side.
     private boolean mBlockNetworkLoads;  // Default depends on permission of embedding APK.
     private boolean mAllowContentUrlAccess = true;
     private boolean mAllowFileUrlAccess = true;
     private int mCacheMode = WebSettings.LOAD_DEFAULT;
@@ skipping 92 matching lines @@
             mBlockNetworkLoads = !hasInternetPermission;
 
             if (isAccessFromFileURLsGrantedByDefault) {
                 mAllowUniversalAccessFromFileURLs = true;
                 mAllowFileAccessFromFileURLs = true;
             }
 
             mUserAgent = LazyDefaultUserAgent.sInstance;
             onGestureZoomSupportChanged(supportsGestureZoomLocked());
 
+            // Best-guess  a sensible initial value based on the features supported on the device.

[dmazzoni, 2013/10/22 06:47:55]

nit: delete extra space after "guess"

[joth, 2013/10/22 22:02:41]

Done.

+            mSpatialNavigationEnabled = !context.getPackageManager().hasSystemFeature(
+                    PackageManager.FEATURE_TOUCHSCREEN);
+
             // Respect the system setting for password echoing.
             mPasswordEchoEnabled = Settings.System.getInt(context.getContentResolver(),
                     Settings.System.TEXT_SHOW_PASSWORD, 1) == 1;
 
             mSupportLegacyQuirks = supportsLegacyQuirks;
         }
         // Defer initializing the native side until a native WebContents instance is set.
     }
 
     @CalledByNative
@@ skipping 128 matching lines @@
                 });
             }
         }
     }
 
     @CalledByNative
     private float getInitialPageScalePercentLocked() {
         return mInitialPageScalePercent;
     }
 
+    void setSpatialNavigationEnabled(boolean enable) {
+        synchronized (mAwSettingsLock) {
+            if (mSpatialNavigationEnabled != enable) {
+                mSpatialNavigationEnabled = enable;
+                mEventHandler.updateWebkitPreferencesLocked();

[dmazzoni, 2013/10/22 06:47:55]

Does this have the right effect, visually? I didn't see code in Blink that
triggers when the pref changes, so visual artifacts might remain when
disabling it.

[joth, 2013/10/22 22:02:41]

I think you're right that when transitioning enabled -> disabled there's no
specific code in place to ensure the  focus ring is not left on an item that is
now no-longer clickable.

In practice it seems to work OK. The main issues of spatial nav == on are in
erroneous focus changes from touch events, and having touch-down event turn off
spat-nav avoids those.

I also toyed with:
- making spatial nav be a one-way latch (off->on)
- calling  RenderViewHost::ClearFocusedNode() when transitioning to off

neither those looked like an improvement, and having additional
ClearFocusedNode() may have other unintended consequences. So I think I'll go
for this strategy for this CL and see about following up with something more
elaborate depending on the outcome of ongoing discussion on the bug.

+            }
+        }
+    }
+
+    @CalledByNative
+    private boolean getSpatialNavigationLocked() {
+        return mSpatialNavigationEnabled;
+    }
+
     /**
      * See {@link android.webkit.WebSettings#setNeedInitialFocus}.
      */
     public boolean shouldFocusFirstNode() {
         synchronized(mAwSettingsLock) {
             return mShouldFocusFirstNode;
         }
     }
 
     /**
@@ skipping 706 matching lines @@
             return mUseWideViewport;
         }
     }
 
     @CalledByNative
     private boolean getUseWideViewportLocked() {
         return mUseWideViewport;
     }
 
     @CalledByNative
-    private boolean getPasswordEchoEnabled() {
+    private boolean getPasswordEchoEnabledLocked() {
         return mPasswordEchoEnabled;
     }
 
     /**
      * See {@link android.webkit.WebSettings#setAppCacheEnabled}.
      */
     public void setAppCacheEnabled(boolean flag) {
         synchronized (mAwSettingsLock) {
             if (mAppCacheEnabled != flag) {
                 mAppCacheEnabled = flag;
@@ skipping 292 matching lines @@
     private native void nativeUpdateInitialPageScaleLocked(int nativeAwSettings);
 
     private native void nativeUpdateUserAgentLocked(int nativeAwSettings);
 
     private native void nativeUpdateWebkitPreferencesLocked(int nativeAwSettings);
 
     private static native String nativeGetDefaultUserAgent();
 
     private native void nativeUpdateFormDataPreferencesLocked(int nativeAwSettings);
 }