Code cleanup, move google location settings logic to GoogleLocationUtils

MyTracks/src/com/google/android/apps/mytracks/services/MyTracksLocationManager.java

@@ -23,18 +23,14 @@ import com.google.android.gms.common.GooglePlayServicesClient.OnConnectionFailed
 import com.google.android.gms.location.LocationClient;
 import com.google.android.gms.location.LocationListener;
 import com.google.android.gms.location.LocationRequest;
-import com.google.common.annotations.VisibleForTesting;
 
 import android.content.ContentResolver;
 import android.content.Context;
 import android.database.ContentObserver;
-import android.database.Cursor;
 import android.location.LocationManager;
-import android.net.Uri;
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.Looper;
-import android.util.Log;
 
 /**
  * My Tracks Location Manager. Applies Google location settings before allowing
@@ -57,22 +53,10 @@ public class MyTracksLocationManager {
 
     @Override
     public void onChange(boolean selfChange) {
-      isAllowed = isUseLocationForServicesOn();
+      isAllowed = GoogleLocationUtils.isLocationAccessAllowed(context);
     }
   }
 
-  private static final String TAG = MyTracksLocationManager.class.getSimpleName();
-
-  private static final String GOOGLE_SETTINGS_CONTENT_URI = "content://com.google.settings/partner";
-  private static final String USE_LOCATION_FOR_SERVICES = "use_location_for_services";
-
-  // User has agreed to use location for Google services.
-  @VisibleForTesting
-  static final String USE_LOCATION_FOR_SERVICES_ON = "1";
-
-  private static final String NAME = "name";
-  private static final String VALUE = "value";
-
   private final ConnectionCallbacks connectionCallbacks = new ConnectionCallbacks() {
       @Override
     public void onDisconnected() {}
@@ -105,13 +89,13 @@ public class MyTracksLocationManager {
         public void onConnectionFailed(ConnectionResult connectionResult) {}
       };
 
+  private final Context context;
   private final Handler handler;
   private final LocationClient locationClient;
   private final LocationManager locationManager;
   private final ContentResolver contentResolver;
   private final GoogleSettingsObserver observer;
 
-  private boolean isAvailable;
   private boolean isAllowed;
   private LocationListener requestLastLocation;
   private LocationListener requestLocationUpdates;
@@ -119,6 +103,7 @@ public class MyTracksLocationManager {
   private long requestLocationUpdatesTime;
 
   public MyTracksLocationManager(Context context, Looper looper, boolean enableLocaitonClient) {
+    this.context = context;
     this.handler = new Handler(looper);
 
     if (enableLocaitonClient) {
@@ -132,11 +117,10 @@ public class MyTracksLocationManager {
     contentResolver = context.getContentResolver();
     observer = new GoogleSettingsObserver(handler);
 
-    isAvailable = GoogleLocationUtils.isAvailable(context);
+    isAllowed = GoogleLocationUtils.isLocationAccessAllowed(context);
-    isAllowed = isUseLocationForServicesOn();
 
     contentResolver.registerContentObserver(
-        Uri.parse(GOOGLE_SETTINGS_CONTENT_URI + "/" + USE_LOCATION_FOR_SERVICES), false, observer);
+        GoogleLocationUtils.USE_LOCATION_FOR_SERVICES_URI, false, observer);
   }
 
   /**
@@ -229,29 +213,4 @@ public class MyTracksLocationManager {
       }
     });
   }
-
-  /**
-   * Returns true if the Google location settings for
-   * {@link #USE_LOCATION_FOR_SERVICES} is on.
-   */
-  private boolean isUseLocationForServicesOn() {
-    if (!isAvailable) {
-      return true;
-    }
-    Cursor cursor = null;
-    try {
-      cursor = contentResolver.query(Uri.parse(GOOGLE_SETTINGS_CONTENT_URI), new String[] { VALUE },
-          NAME + "=?", new String[] { USE_LOCATION_FOR_SERVICES }, null);
-      if (cursor != null && cursor.moveToNext()) {
-        return USE_LOCATION_FOR_SERVICES_ON.equals(cursor.getString(0));
-      }
-    } catch (RuntimeException e) {
-      Log.w(TAG, "Failed to read " + USE_LOCATION_FOR_SERVICES, e);
-    } finally {
-      if (cursor != null) {
-        cursor.close();
-      }
-    }
-    return false;
-  }
 }

MyTracks/src/com/google/android/apps/mytracks/util/GoogleLocationUtils.java

@@ -18,11 +18,15 @@ package com.google.android.apps.mytracks.util;
 
 import com.google.android.maps.mytracks.R;
 
+import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
+import android.database.Cursor;
+import android.net.Uri;
 import android.provider.Settings;
+import android.util.Log;
 
 /**
  * Utilities for Google location settings
@@ -31,17 +35,48 @@ import android.provider.Settings;
  */
 public class GoogleLocationUtils {
 
+  private static final String TAG = GoogleLocationUtils.class.getSimpleName();
+
+  // Action to launch the google location settings
   private static final String
       ACTION_GOOGLE_LOCATION_SETTINGS = "com.google.android.gsf.GOOGLE_LOCATION_SETTINGS";
+
+  // Action to check if the google apps locations settings exists
   private static final String
       ACTION_GOOGLE_APPS_LOCATION_SETTINGS = "com.google.android.gsf.GOOGLE_APPS_LOCATION_SETTINGS";
 
+  /**
+   * User has disagreed to use location for Google services.
+   */
+  private static final int USE_LOCATION_FOR_SERVICES_OFF = 0;
+
+  /**
+   * User has agreed to use location for Google services.
+   */
+  public static final int USE_LOCATION_FOR_SERVICES_ON = 1;
+
+  /**
+   * The user has neither agreed nor disagreed to use location for Google
+   * services yet.
+   */
+  private static final int USE_LOCATION_FOR_SERVICES_NOT_SET = 2;
+
+  private static final String GOOGLE_SETTINGS_AUTHORITY = "com.google.settings";
+  private static final Uri GOOGLE_SETTINGS_CONTENT_URI = Uri.parse(
+      "content://" + GOOGLE_SETTINGS_AUTHORITY + "/partner");
+  private static final String NAME = "name";
+  private static final String VALUE = "value";
+  private static final String USE_LOCATION_FOR_SERVICES = "use_location_for_services";
+
+  public static final Uri USE_LOCATION_FOR_SERVICES_URI = Uri.parse(
+      "content://" + GOOGLE_SETTINGS_AUTHORITY + "/partner/" + USE_LOCATION_FOR_SERVICES);
+
   private GoogleLocationUtils() {}
 
   /**
-   * Returns true if the Google location settings is available.
+   * Returns true if the Google location settings is enforceable.
    */
-  public static boolean isAvailable(Context context) {
+  public static boolean isEnforceable(Context context) {
     Intent intent = new Intent(ACTION_GOOGLE_APPS_LOCATION_SETTINGS);
     ResolveInfo resolveInfo = context.getPackageManager()
         .resolveActivity(intent, PackageManager.MATCH_DEFAULT_ONLY);
@@ -54,7 +89,7 @@ public class GoogleLocationUtils {
    * @param context the context
    */
   public static String getLocationSettingsName(Context context) {
-    return context.getString(isAvailable(context) ? R.string.gps_google_location_settings
+    return context.getString(isEnforceable(context) ? R.string.gps_google_location_settings
         : R.string.gps_location_access);
   }
 
@@ -64,10 +99,58 @@ public class GoogleLocationUtils {
    * @param context the context
    */
   public static Intent newLocationSettingsIntent(Context context) {
-    Intent intent = isAvailable(context) ? new Intent(
+    Intent intent = isEnforceable(context) ? new Intent(
         GoogleLocationUtils.ACTION_GOOGLE_LOCATION_SETTINGS)
         : new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS);
     intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
     return intent;
   }
+
+  /**
+   * Returns true if location access is allowed.
+   * 
+   * @param context the context
+   */
+  public static boolean isLocationAccessAllowed(Context context) {
+    if (!isEnforceable(context)) {
+      return true;
+    }
+    return getUseLocationForServices(context) == USE_LOCATION_FOR_SERVICES_ON;
+  }
+
+  /**
+   * Get the current value for the 'Use value for location' setting.
+   * 
+   * @return One of {@link #USE_LOCATION_FOR_SERVICES_NOT_SET},
+   *         {@link #USE_LOCATION_FOR_SERVICES_OFF} or
+   *         {@link #USE_LOCATION_FOR_SERVICES_ON}.
+   */
+  private static int getUseLocationForServices(Context context) {
+    ContentResolver contentResolver = context.getContentResolver();
+    Cursor cursor = null;
+    String stringValue = null;
+    try {
+      cursor = contentResolver.query(GOOGLE_SETTINGS_CONTENT_URI, new String[] { VALUE },
+          NAME + "=?", new String[] { USE_LOCATION_FOR_SERVICES }, null);
+      if (cursor != null && cursor.moveToNext()) {
+        stringValue = cursor.getString(0);
+      }
+    } catch (RuntimeException e) {
+      Log.w(TAG, "Failed to get 'Use My Location' setting", e);
+    } finally {
+      if (cursor != null) {
+        cursor.close();
+      }
+    }
+    if (stringValue == null) {
+      return USE_LOCATION_FOR_SERVICES_NOT_SET;
+    }
+    int value;
+    try {
+      value = Integer.parseInt(stringValue);
+    } catch (NumberFormatException nfe) {
+      value = USE_LOCATION_FOR_SERVICES_NOT_SET;
+    }
+    return value;
+  }
 }

MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java

@@ -24,6 +24,7 @@ import com.google.android.apps.mytracks.content.Waypoint.WaypointType;
 import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.stats.TripStatistics;
 import com.google.android.apps.mytracks.util.ApiAdapterFactory;
+import com.google.android.apps.mytracks.util.GoogleLocationUtils;
 import com.google.android.apps.mytracks.util.PreferencesUtils;
 import com.google.android.maps.mytracks.R;
 
@@ -177,7 +178,7 @@ public class TrackRecordingServiceTest extends ServiceTestCase<TestRecordingServ
 
             @Override
           public String getString(int columnIndex) {
-            return MyTracksLocationManager.USE_LOCATION_FOR_SERVICES_ON;
+            return String.valueOf(GoogleLocationUtils.USE_LOCATION_FOR_SERVICES_ON);
           }
 
             @Override

MyTracksTest_E2E/src/com/google/android/apps/mytracks/endtoendtest/common/GoToMyLocationTest.java

@@ -53,7 +53,7 @@ public class GoToMyLocationTest extends ActivityInstrumentationTestCase2<TrackLi
     EndToEndTestUtils.findAndClickMyLocation(activityMyTracks);
     if (EndToEndTestUtils.isEmulator) {
       String setting = activityMyTracks.getString(
-          GoogleLocationUtils.isAvailable(activityMyTracks) ? R.string.gps_google_location_settings
+          GoogleLocationUtils.isEnforceable(activityMyTracks) ? R.string.gps_google_location_settings
               : R.string.gps_location_access);
       EndToEndTestUtils.SOLO.waitForText(
           activityMyTracks.getString(R.string.my_location_no_gps, setting), 1,