Implementing geolocation for the Android Webview

android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java

Index: android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java b/android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java
index 133f5970eb5a12a778701aba59b3f329e39941e0..2c5fb7184988abc2ce2bcd237031050ff6bfa761 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwGeolocationPermissions.java
@@ -24,10 +24,44 @@ public final class AwGeolocationPermissions {
             AwGeolocationPermissions.class.getCanonicalName() + "%";
     private final SharedPreferences mSharedPreferences;
 
+    private static AwGeolocationPermissions sInstance;
+
+    // TODO(kristianm): Make this private once framework is updated to use getInstance
     public AwGeolocationPermissions(SharedPreferences sharedPreferences) {
         mSharedPreferences = sharedPreferences;
+        sInstance = this;
+    }
+
+    /**
+     * Get the static instance after it has been created
+     */
+    public static AwGeolocationPermissions getInstance() {
+        synchronized (AwGeolocationPermissions.class) {
+            if (sInstance == null) {
+                throw new IllegalStateException("This should only be called after createInstance.");
+            }
+        }
+        return sInstance;
+    }
+
+    /**
+     * Create the static instance of this class
+     */
+    public static AwGeolocationPermissions createInstance(
+            SharedPreferences sharedPreferences) {
+        synchronized (AwGeolocationPermissions.class) {
+            if (sInstance != null) {
+                throw new IllegalStateException("This should only be called once.");
+            }
+            // sInstance set in the constructor
+            new AwGeolocationPermissions(sharedPreferences);
+            return sInstance;
+        }
     }
 
+    /**
+     * Set one origin to be allowed.
+     */
     public void allow(String origin) {
         String key = getOriginKey(origin);
         if (key != null) {
@@ -35,6 +69,9 @@ public final class AwGeolocationPermissions {
         }
     }
 
+    /**
+     * Set one origin to be denied.
+     */
     public void deny(String origin) {
         String key = getOriginKey(origin);
         if (key != null) {
@@ -42,6 +79,9 @@ public final class AwGeolocationPermissions {
         }
     }
 
+    /**
+     * Clear one origin from being allowed and denied.

[benm (inactive), 2013/02/07 17:22:39]

nit: Does "Clear the stored permission for a particular origin." read better?

[Kristian Monsen, 2013/02/08 00:07:44]

Done.

+     */
     public void clear(String origin) {

[benm (inactive), 2013/02/07 17:22:39]

Should we document the format for origin anywhere?

[Kristian Monsen, 2013/02/08 00:07:44]

I don't think that is needed. It is a regular concept for url's, and if anything
else than a origin is passed in we just get the origin part of it.

         String key = getOriginKey(origin);
         if (key != null) {
@@ -49,6 +89,9 @@ public final class AwGeolocationPermissions {
         }
     }
 
+    /**
+     * Clear all origins set to allowed or denied.

[benm (inactive), 2013/02/07 17:22:39]

nit: Does "Clear stored permissions for all origins." read better?

[Kristian Monsen, 2013/02/08 00:07:44]

Done.

+     */
     public void clearAll() {
         SharedPreferences.Editor editor = null;
         for (String name : mSharedPreferences.getAll().keySet()) {
@@ -64,17 +107,29 @@ public final class AwGeolocationPermissions {
         }
     }
 
-    public void getAllowed(String origin, final ValueCallback<Boolean> callback) {
-        boolean allowed = false;
+    /**
+     * Sync method to get if an origin is set to be allowed.
+     */
+    public boolean isOriginAllowed(String origin) {
         try {
-            String key = getOriginKey(origin);
-            if (key != null) {
-                allowed = mSharedPreferences.getBoolean(key, false);
-            }
+            return mSharedPreferences.getBoolean(getOriginKey(origin), false);
         } catch (ClassCastException e) {

[benm (inactive), 2013/02/07 17:22:39]

Do you need to catch this?

[Kristian Monsen, 2013/02/08 00:07:44]

I think it is best. If somehow there got to be another pref with the same name
it would raise an exception all the way out. Don't think we want that on release
builds. This is not optimal either so I could be convinced to remove. Might make
sense to remove the bad pref as well.

-            // Want to return false in this case, do nothing here
+            return false;
         }
-        final boolean finalAllowed = allowed;
+    }
+
+    /**
+     * Returns true if the origin is either set to allowed or denied.
+     */
+    public boolean hasOrigin(String origin) {
+        return mSharedPreferences.contains(getOriginKey(origin));
+    }
+
+    /**
+     * Async method to get if an origin set to be allowed.
+     */
+    public void getAllowed(String origin, final ValueCallback<Boolean> callback) {
+        final boolean finalAllowed = isOriginAllowed(origin);
         ThreadUtils.postOnUiThread(new Runnable() {
             @Override
             public void run() {
@@ -83,6 +138,9 @@ public final class AwGeolocationPermissions {
         });
     }
 
+    /**
+     * ASync method to get the domains currently allowed or denied.
+     */
     public void getOrigins(final ValueCallback<Set<String>> callback) {
         final Set<String> origins = new HashSet<String>();
         for (String name : mSharedPreferences.getAll().keySet()) {
@@ -98,6 +156,9 @@ public final class AwGeolocationPermissions {
         });
     }
 
+    /**
+     * Get the domain of an URL using the GURL library.
+     */
     private String getOriginKey(String url) {
         String origin = GURLUtils.getOrigin(url);
         if (origin.isEmpty()) {