Dynamically enable spatial navigation based on events

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

Index: android_webview/java/src/org/chromium/android_webview/AwSettings.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwSettings.java b/android_webview/java/src/org/chromium/android_webview/AwSettings.java
index deee0a59776e63733066d171cc286f81901e4afe..d22c01fbafea51da1ec7fdd63265766fda7f5e5f 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwSettings.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwSettings.java
@@ -81,6 +81,7 @@ public class AwSettings {
     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;
 
@@ -193,6 +194,10 @@ public class AwSettings {
             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;
@@ -341,6 +346,20 @@ public class AwSettings {
         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}.
      */
@@ -1067,7 +1086,7 @@ public class AwSettings {
     }
 
     @CalledByNative
-    private boolean getPasswordEchoEnabled() {
+    private boolean getPasswordEchoEnabledLocked() {
         return mPasswordEchoEnabled;
     }