Freeing MyTracks from isRecording() calls.

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

@@ -23,6 +23,7 @@ import com.google.android.apps.mytracks.content.TracksColumns;
 import com.google.android.apps.mytracks.content.WaypointCreationRequest;
 import com.google.android.apps.mytracks.io.file.TempFileCleaner;
 import com.google.android.apps.mytracks.services.ITrackRecordingService;
+import com.google.android.apps.mytracks.services.ServiceStateHelper;
 import com.google.android.apps.mytracks.services.TrackRecordingServiceBinder;
 import com.google.android.apps.mytracks.services.tasks.StatusAnnouncerFactory;
 import com.google.android.apps.mytracks.util.ApiFeatures;
@@ -103,38 +104,17 @@ public class MyTracks extends TabActivity implements OnTouchListener {
       synchronized (serviceBinder) {
         ITrackRecordingService service = serviceBinder.getServiceIfBound();
         if (startNewTrackRequested && service != null) {
+          Log.i(TAG, "Starting recording");
+          startNewTrackRequested = false;
           startRecordingNewTrack(service);
+        } else {
+          Log.w(TAG, "Not yet starting recording");
         }
       }
     }
   };
 
-  /**
-   * Checks whether we have a track recording session in progress.
-   * In some cases, when the service has crashed or has been restarted
-   * by the system, we fall back to the shared preferences.
-   *
-   * @return true if the activity is bound to the track recording service and
-   *         the service is recording a track or in case the service is down,
-   *         based on settings from the shared preferences.
-   */
-  public boolean isRecording() {
-    ITrackRecordingService trackRecordingService = serviceBinder.getServiceIfBound();
-    if (trackRecordingService == null) {
-      // Fall back to alternative check method.
-      return dataHub.isRecording();
-    }
-    try {
-      return trackRecordingService.isRecording();
-      // TODO: We catch Exception, because after eliminating the service process
-      // all exceptions it may throw are no longer wrapped in a RemoteException.
-    } catch (Exception e) {
-      Log.e(TAG, "MyTracks: Remote exception.", e);
-
-      // Fall back to alternative check method.
-      return dataHub.isRecording();
-    }
-  }
+  private SharedPreferences preferences;
 
   /*
    * Application lifetime events:
@@ -152,7 +132,8 @@ public class MyTracks extends TabActivity implements OnTouchListener {
     }
 
     providerUtils = MyTracksProviderUtils.Factory.get(this);
-    dataHub = new TrackDataHub(this, providerUtils);
+    preferences = getSharedPreferences(Constants.SETTINGS_NAME, 0);
+    dataHub = new TrackDataHub(this, preferences, providerUtils);
     menuManager = new MenuManager(this);
     dialogManager = new DialogManager(this);
     serviceBinder = TrackRecordingServiceBinder.getInstance(this);
@@ -244,7 +225,7 @@ public class MyTracks extends TabActivity implements OnTouchListener {
     dataHub.start();
 
     // Ensure that service is running if we're supposed to be recording
-    if (dataHub.isRecording()) {
+    if (ServiceStateHelper.isRecording(this, preferences)) {
       serviceBinder.startService();
     }
 
@@ -273,7 +254,8 @@ public class MyTracks extends TabActivity implements OnTouchListener {
   @Override
   public boolean onPrepareOptionsMenu(Menu menu) {
     menuManager.onPrepareOptionsMenu(menu, providerUtils.getLastTrack() != null,
-        isRecording(), dataHub.isATrackSelected());
+        ServiceStateHelper.isRecording(this, preferences),
+        dataHub.isATrackSelected());
     return super.onPrepareOptionsMenu(menu);
   }
 
@@ -291,7 +273,7 @@ public class MyTracks extends TabActivity implements OnTouchListener {
 
   @Override
   public boolean onTrackballEvent(MotionEvent event) {
-    if (isRecording()) {
+    if (ServiceStateHelper.isRecording(this, preferences)) {
       if (event.getAction() == MotionEvent.ACTION_DOWN) {
         try {
           insertWaypoint(WaypointCreationRequest.DEFAULT_STATISTICS);
@@ -436,7 +418,6 @@ public class MyTracks extends TabActivity implements OnTouchListener {
     }
   }
 
-
   private void startRecordingNewTrack(
       ITrackRecordingService trackRecordingService) {
     try {
@@ -509,10 +490,6 @@ public class MyTracks extends TabActivity implements OnTouchListener {
     return dataHub.getSelectedTrackId();
   }
 
-  public DialogManager getDialogManager() {
-    return dialogManager;
-  }
-
   public TrackDataHub getDataHub() {
     return dataHub;
   }

MyTracks/src/com/google/android/apps/mytracks/StatsActivity.java

@@ -22,11 +22,13 @@ import com.google.android.apps.mytracks.content.TrackDataHub;
 import com.google.android.apps.mytracks.content.TrackDataHub.ListenerDataType;
 import com.google.android.apps.mytracks.content.TrackDataListener;
 import com.google.android.apps.mytracks.content.Waypoint;
+import com.google.android.apps.mytracks.services.ServiceStateHelper;
 import com.google.android.apps.mytracks.services.tasks.StatusAnnouncerFactory;
 import com.google.android.apps.mytracks.util.ApiFeatures;
 import com.google.android.maps.mytracks.R;
 
 import android.app.Activity;
+import android.content.SharedPreferences;
 import android.location.Location;
 import android.os.Bundle;
 import android.util.DisplayMetrics;
@@ -46,6 +48,18 @@ import java.util.EnumSet;
  * @author Rodrigo Damazio
  */
 public class StatsActivity extends Activity implements TrackDataListener {
+  /**
+   * A runnable for posting to the UI thread. Will update the total time field.
+   */
+  private final Runnable updateResults = new Runnable() {
+    public void run() {
+      if (dataHub.isRecordingSelected()) {
+        utils.setTime(R.id.total_time_register,
+            System.currentTimeMillis() - startTime);
+      }
+    }
+  };
+
   private StatsUtilities utils;
   private UIUpdateThread thread;
 
@@ -61,18 +75,7 @@ public class StatsActivity extends Activity implements TrackDataListener {
   private boolean showCurrentSegment = false;
 
   private TrackDataHub dataHub;
-
-  /**
-   * A runnable for posting to the UI thread. Will update the total time field.
-   */
-  private final Runnable updateResults = new Runnable() {
-    public void run() {
-      if (dataHub.isRecordingSelected()) {
-        utils.setTime(R.id.total_time_register,
-            System.currentTimeMillis() - startTime);
-      }
-    }
-  };
+  private SharedPreferences preferences;
 
   /**
    * A thread that updates the total time field every second.
@@ -87,7 +90,7 @@ public class StatsActivity extends Activity implements TrackDataListener {
     @Override
     public void run() {
       Log.i(TAG, "Started UI update thread");
-      while (MyTracks.getInstance().isRecording()) {
+      while (ServiceStateHelper.isRecording(StatsActivity.this, preferences)) {
         runOnUiThread(updateResults);
         try {
           Thread.sleep(1000L);
@@ -105,6 +108,7 @@ public class StatsActivity extends Activity implements TrackDataListener {
   public void onCreate(Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
 
+    preferences = getSharedPreferences(Constants.SETTINGS_NAME, 0);
     dataHub = MyTracks.getInstance().getDataHub();
     utils = new StatsUtilities(this);
 

MyTracks/src/com/google/android/apps/mytracks/TrackList.java

@@ -18,6 +18,7 @@ package com.google.android.apps.mytracks;
 import com.google.android.apps.mytracks.content.TracksColumns;
 import com.google.android.apps.mytracks.io.file.SaveActivity;
 import com.google.android.apps.mytracks.io.sendtogoogle.SendActivity;
+import com.google.android.apps.mytracks.services.ServiceStateHelper;
 import com.google.android.apps.mytracks.util.StringUtils;
 import com.google.android.apps.mytracks.util.UnitConversions;
 import com.google.android.maps.mytracks.R;
@@ -78,7 +79,7 @@ public class TrackList extends ListActivity
               R.string.tracklist_show_track);
           menu.add(0, Constants.MENU_EDIT, 0,
               R.string.tracklist_edit_track);
-          if (!MyTracks.getInstance().isRecording()
+          if (!ServiceStateHelper.isRecording(TrackList.this, null)
               || trackId != recordingTrackId) {
             menu.add(0, Constants.MENU_SEND_TO_GOOGLE, 0,
                 R.string.tracklist_send_to_google);
@@ -213,17 +214,18 @@ public class TrackList extends ListActivity
     listView = getListView();
     listView.setOnCreateContextMenuListener(contextMenuListener);
 
+    SharedPreferences preferences =
+        getSharedPreferences(Constants.SETTINGS_NAME, 0);
+
     View deleteAll = findViewById(R.id.tracklist_btn_delete_all);
     View exportAll = findViewById(R.id.tracklist_btn_export_all);
-    boolean notRecording = !MyTracks.getInstance().isRecording();
+    boolean notRecording = !ServiceStateHelper.isRecording(this, preferences);
     deleteAll.setOnClickListener(this);
     deleteAll.setEnabled(notRecording);
     exportAll.setOnClickListener(this);
     exportAll.setEnabled(notRecording);
     findViewById(R.id.tracklist_btn_import_all).setOnClickListener(this);
 
-    SharedPreferences preferences =
-        getSharedPreferences(Constants.SETTINGS_NAME, 0);
     preferences.registerOnSharedPreferenceChangeListener(this);
     metricUnits =
         preferences.getBoolean(getString(R.string.metric_units_key), true);

MyTracks/src/com/google/android/apps/mytracks/content/TrackDataHub.java

@@ -17,12 +17,11 @@ package com.google.android.apps.mytracks.content;
 
 import static com.google.android.apps.mytracks.Constants.DEFAULT_MIN_REQUIRED_ACCURACY;
 import static com.google.android.apps.mytracks.Constants.MAX_DISPLAYED_WAYPOINTS_POINTS;
-import static com.google.android.apps.mytracks.Constants.TARGET_DISPLAYED_TRACK_POINTS;
 import static com.google.android.apps.mytracks.Constants.MAX_LOCATION_AGE_MS;
 import static com.google.android.apps.mytracks.Constants.MAX_NETWORK_AGE_MS;
 import static com.google.android.apps.mytracks.Constants.TAG;
+import static com.google.android.apps.mytracks.Constants.TARGET_DISPLAYED_TRACK_POINTS;
 
-import com.google.android.apps.mytracks.Constants;
 import com.google.android.apps.mytracks.content.DataSourceManager.DataSourceListener;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils.DoubleBufferedLocationFactory;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils.LocationIterator;
@@ -196,14 +195,7 @@ public class TrackDataHub {
   /**
    * Default constructor.
    */
-  public TrackDataHub(Context ctx, MyTracksProviderUtils providerUtils) {
-    this(ctx, ctx.getSharedPreferences(Constants.SETTINGS_NAME, 0), providerUtils);
-  }
-
-  /**
-   * Intermediate constructor, used only for passing preferences into arguments.
-   */
-  private TrackDataHub(Context ctx, SharedPreferences preferences,
+  public TrackDataHub(Context ctx, SharedPreferences preferences,
       MyTracksProviderUtils providerUtils) {
     this(ctx, new DataSourcesWrapperImpl(ctx, preferences), new TrackDataListeners(),
         preferences, providerUtils, TARGET_DISPLAYED_TRACK_POINTS);
@@ -355,14 +347,6 @@ public class TrackDataHub {
     return getSelectedTrackId() > 0;
   }
 
-  /** Returns whether we're currently recording a track. */
-  public boolean isRecording() {
-    if (!started) {
-      loadSharedPreferences();
-    }
-    return preferences.getLong(RECORDING_TRACK_KEY, -1) > 0;
-  }
-
   /** Returns whether the selected track is still being recorded. */
   public boolean isRecordingSelected() {
     if (!started) {

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

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.android.apps.mytracks.services;
+
+import static com.google.android.apps.mytracks.Constants.TAG;
+
+import com.google.android.apps.mytracks.Constants;
+import com.google.android.maps.mytracks.R;
+
+import android.content.Context;
+import android.content.SharedPreferences;
+import android.os.RemoteException;
+import android.util.Log;
+
+/**
+ * Helper for reading service state.
+ *
+ * @author Rodrigo Damazio
+ */
+public class ServiceStateHelper {
+
+  public static boolean isRecording(Context ctx, SharedPreferences preferences) {
+    TrackRecordingServiceBinder serviceBinder = TrackRecordingServiceBinder.getInstance(ctx);
+    ITrackRecordingService service = serviceBinder.getServiceIfBound();
+    if (service != null) {
+      try {
+        return service.isRecording();
+      } catch (RemoteException e) {
+        Log.e(TAG, "Failed to check if service is recording", e);
+      } catch (IllegalStateException e) {
+        Log.e(TAG, "Failed to check if service is recording", e);
+      }
+    }
+
+    if (preferences == null) {
+      preferences = ctx.getSharedPreferences(Constants.SETTINGS_NAME, 0);
+    }
+    return preferences.getLong(ctx.getString(R.string.recording_track_key), -1) > 0;
+  }
+
+  private ServiceStateHelper() {}
+}

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

@@ -968,7 +968,7 @@ public class TrackRecordingService extends Service implements LocationListener {
       this.service = null;
       attachInterface(null, null);
     }
-    
+
     @Override
     public boolean isRecording() {
       checkService();

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

@@ -48,6 +48,7 @@ public class TrackRecordingServiceBinder {
       ITrackRecordingService trackRecordingService =
           ITrackRecordingService.Stub.asInterface(service);
       synchronized (TrackRecordingServiceBinder.this) {
+        TrackRecordingServiceBinder.this.trackRecordingService = trackRecordingService;
         try {
           for (Runnable callback : pendingBindCallbacks.keySet()) {
             if (callback != null) {
@@ -56,7 +57,6 @@ public class TrackRecordingServiceBinder {
           }
         } finally {
           pendingBindCallbacks.clear();
-          TrackRecordingServiceBinder.this.trackRecordingService = trackRecordingService;
         }
       }
     }

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

@@ -15,6 +15,7 @@
  */
 package com.google.android.apps.mytracks;
 
+import com.google.android.apps.mytracks.services.ServiceStateHelper;
 import com.google.android.maps.mytracks.R;
 
 import android.app.Activity;
@@ -55,7 +56,7 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
     assertInitialized();
 
     // Check if not recording.
-    assertFalse(getActivity().isRecording());
+    assertFalse(isRecording());
     assertEquals(-1, getRecordingTrackId());
     long selectedTrackId = getSharedPreferences().getLong(
         getActivity().getString(R.string.selected_track_key), -1);
@@ -71,7 +72,7 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
     assertInitialized();
 
     // Check if not recording.
-    assertFalse(getActivity().isRecording());
+    assertFalse(isRecording());
     assertEquals(-1, getRecordingTrackId());
     long selectedTrackId = getSharedPreferences().getLong(
         getActivity().getString(R.string.selected_track_key), -1);
@@ -92,7 +93,7 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
     assertInitialized();
 
     // Check if not recording.
-    assertFalse(getActivity().isRecording());
+    assertFalse(isRecording());
     assertEquals(-1, getRecordingTrackId());
     long selectedTrackId = getSharedPreferences().getLong(
         getActivity().getString(R.string.selected_track_key), -1);
@@ -112,7 +113,7 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
     assertInitialized();
 
     // Check if not recording.
-    assertFalse(getActivity().isRecording());
+    assertFalse(isRecording());
     assertEquals(-1, getRecordingTrackId());
     long selectedTrackId = getSharedPreferences().getLong(
         getActivity().getString(R.string.selected_track_key), -1);
@@ -128,7 +129,7 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
     clearSelectedAndRecordingTracks();    
     waitForIdle();
 
-    assertFalse(getActivity().isRecording());
+    assertFalse(isRecording());
     assertEquals(-1, getRecordingTrackId());
     long selectedTrackId = getSharedPreferences().getLong(
         getActivity().getString(R.string.selected_track_key), -1);
@@ -227,14 +228,14 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
   private long awaitRecordingStatus(long timeout, boolean isRecording)
       throws TimeoutException, InterruptedException {
     long startTime = System.nanoTime();
-    while (getActivity().isRecording() != isRecording) {
+    while (isRecording() != isRecording) {
       if (System.nanoTime() - startTime > timeout * 1000000) {
         throw new TimeoutException("Timeout while waiting for recording!");
       }
       Thread.sleep(20);
     }
     waitForIdle();
-    assertEquals(isRecording, getActivity().isRecording());
+    assertEquals(isRecording, isRecording());
     return getRecordingTrackId();
   }
 
@@ -248,4 +249,8 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2<MyTracks>{
     }
     return sharedPreferences;
   }
+
+  private boolean isRecording() {
+    return ServiceStateHelper.isRecording(getActivity(), getSharedPreferences());
+  }
 }