Several fixes to the service and more test coverage.

Notably:
1) Fixed isRecording to not return false if service is down (for instance, it may be being restarted)
2) Added more checks to prevent from invalid starting/stopping (if in already in progress/not started).

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

@@ -298,13 +298,13 @@ public class ChartActivity extends Activity implements
       public void onChange(boolean selfChange) {
         Log.d(MyTracksConstants.TAG, "ChartActivity: ContentObserver.onChange");
         // Check for any new locations and append them to the currently
-        // recording track:
+        // recording track.
-        if (!MyTracks.getInstance().isRecording()) {
+        if (recordingTrackId < 0) {
           // No track is being recorded. We should not be here.
           return;
         }
         if (selectedTrackId != recordingTrackId) {
-          // no track, or one other than the recording track is selected, don't
+          // No track, or one other than the recording track is selected, don't
           // bother.
           return;
         }

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

@@ -26,9 +26,9 @@ import com.google.android.apps.mytracks.io.AuthManagerFactory;
 import com.google.android.apps.mytracks.io.GpxImporter;
 import com.google.android.apps.mytracks.io.SendToDocs;
 import com.google.android.apps.mytracks.io.SendToMyMaps;
-import com.google.android.apps.mytracks.io.SendToMyMaps.OnSendCompletedListener;
 import com.google.android.apps.mytracks.io.TrackWriter;
 import com.google.android.apps.mytracks.io.TrackWriterFactory;
+import com.google.android.apps.mytracks.io.SendToMyMaps.OnSendCompletedListener;
 import com.google.android.apps.mytracks.io.TrackWriterFactory.TrackFileFormat;
 import com.google.android.apps.mytracks.services.ITrackRecordingService;
 import com.google.android.apps.mytracks.services.StatusAnnouncerFactory;
@@ -65,9 +65,9 @@ import android.view.Menu;
 import android.view.MenuItem;
 import android.view.MotionEvent;
 import android.view.View;
+import android.view.Window;
 import android.view.View.OnTouchListener;
 import android.view.ViewGroup.LayoutParams;
-import android.view.Window;
 import android.view.WindowManager.BadTokenException;
 import android.widget.RelativeLayout;
 import android.widget.TabHost;
@@ -233,20 +233,35 @@ public class MyTracks extends TabActivity implements OnTouchListener,
   }
 
   /**
+   * 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.
+   *         the service is recording a track or in case the service is down,
+   *         based on settings from the shared preferences. 
    */
   public boolean isRecording() {
     if (trackRecordingService == null) {
-      return false;
+      // Fall back to alternative check method.
+      return isRecordingBasedOnSharedPreferences();
     }
     try {
       return trackRecordingService.isRecording();
     } catch (RemoteException e) {
       Log.e(MyTracksConstants.TAG, "MyTracks: Remote exception.", e);
-      return false;
+  
+      // Fall back to alternative check method.
+      return isRecordingBasedOnSharedPreferences();
     }
   }
+  
+  private boolean isRecordingBasedOnSharedPreferences() {
+    // TrackRecordingServices guarantees that recordingTrackId is set to
+    // -1 if the track has been stopped.
+    // TODO: Refresh recordingTrackId.
+    return recordingTrackId >= 0;
+  }
 
   /*
    * Application lifetime events:
@@ -303,6 +318,12 @@ public class MyTracks extends TabActivity implements OnTouchListener,
       recordingTrackId =
           prefs.getLong(getString(R.string.recording_track_key), -1);
       prefs.registerOnSharedPreferenceChangeListener(this);
+      Log.d(MyTracksConstants.TAG, "recordingTrackId: " + recordingTrackId
+          + ", selectedTrackId: " + selectedTrackId);
+      if (recordingTrackId > 0) {
+        Intent startIntent = new Intent(this, TrackRecordingService.class);
+        startService(startIntent);
+      }
     }
 
     // This will show the eula until the user accepts or quits the app.

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

@@ -331,12 +331,7 @@ public class MyTracksMap extends MapActivity
       @Override
       public void onChange(boolean selfChange) {
         Log.d(MyTracksConstants.TAG, "MyTracksMap: ContentObserver.onChange");
-        // Check for any new locations and append them to the currently
+
-        // recording track:
-        if (!MyTracks.getInstance().isRecording()) {
-          // No track is being recorded. We should not be here.
-          return;
-        }
         if (!isRecordingSelected()) {
           // No track, or one other than the recording track is selected,
           // don't bother.
@@ -451,6 +446,10 @@ public class MyTracksMap extends MapActivity
     Log.d(MyTracksConstants.TAG, "MyTracksMap.onResume");
     super.onResume();
 
+    // Reload all preferences as they might have changed meanwhile.
+    reloadSharedPreferences(
+        getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0), null);
+    
     // Make sure any updates that might have happened are propagated to the
     // Map overlay:
     observer.onChange(false);
@@ -786,7 +785,7 @@ public class MyTracksMap extends MapActivity
           menu.setHeaderTitle(R.string.tracklist_this_track);
           menu.add(0, MyTracksConstants.MENU_EDIT, 0,
               R.string.tracklist_edit_track);
-          if (!MyTracks.getInstance().isRecording() || !isRecordingSelected()) {
+          if (!isRecordingSelected()) {
             menu.add(0, MyTracksConstants.MENU_SEND_TO_GOOGLE, 0,
                 R.string.tracklist_send_to_google);
             SubMenu share = menu.addSubMenu(0, MyTracksConstants.MENU_SHARE, 0,
@@ -910,26 +909,7 @@ public class MyTracksMap extends MapActivity
       uiHandler.post(new Runnable() {
         @Override
         public void run() {
-          if (key.equals(getString(R.string.min_required_accuracy_key))) {
+          reloadSharedPreferences(sharedPreferences, key);
-            minRequiredAccuracy = sharedPreferences.getInt(
-                getString(R.string.min_required_accuracy_key),
-                MyTracksSettings.DEFAULT_MIN_REQUIRED_ACCURACY);
-          } else if (key.equals(getString(R.string.selected_track_key))) {
-            long selectedTrackId =
-                sharedPreferences.getLong(
-                    getString(R.string.selected_track_key),
-                    -1);
-            setSelectedTrack(selectedTrackId);
-          } else if (key.equals(getString(R.string.recording_track_key))) {
-            recordingTrackId =
-                sharedPreferences.getLong(
-                    getString(R.string.recording_track_key),
-                    -1);
-            if (isATrackSelected()) {
-              mapOverlay.setShowEndMarker(!isRecordingSelected());
-              mapView.postInvalidate();
-            }
-          }
         }
       });
     }
@@ -1009,6 +989,29 @@ public class MyTracksMap extends MapActivity
     }
   };
 
+  private void reloadSharedPreferences(SharedPreferences sharedPreferences,
+      String key) {
+    if (key == null ||
+        key.equals(getString(R.string.min_required_accuracy_key))) {
+      minRequiredAccuracy = sharedPreferences.getInt(
+          getString(R.string.min_required_accuracy_key),
+          MyTracksSettings.DEFAULT_MIN_REQUIRED_ACCURACY);
+    }
+    if (key == null || key.equals(getString(R.string.selected_track_key))) {
+      long selectedTrackId = sharedPreferences.getLong(
+          getString(R.string.selected_track_key), -1);
+      setSelectedTrack(selectedTrackId);
+    }
+    if (key == null || key.equals(getString(R.string.recording_track_key))) {
+      recordingTrackId = sharedPreferences.getLong(
+          getString(R.string.recording_track_key), -1);
+      if (isATrackSelected()) {
+        mapOverlay.setShowEndMarker(!isRecordingSelected());
+        mapView.postInvalidate();
+      }
+    }
+  }
+
   private void readAllNewTrackPoints() {
     int numPoints = mapOverlay.getNumLocations();
     if (numPoints >= MyTracksConstants.MAX_DISPLAYED_TRACK_POINTS) {

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

@@ -745,6 +745,7 @@ public class TrackRecordingService extends Service implements LocationListener {
             "TrackRecordingService: Not resuming, because the previous track ("
             + recordingTrack + ") doesn't exist or is too old");
         isRecording = false;
+        prefManager.setRecordingTrack(recordingTrackId = -1); 
         stopSelfResult(startId);
         return;
       }
@@ -805,6 +806,11 @@ public class TrackRecordingService extends Service implements LocationListener {
   }
 
   public long insertWaypointMarker(Waypoint waypoint) {
+    if (!isRecording()) {
+      throw new IllegalStateException(
+          "Unable to insert waypoint marker while not recording!");
+    }
+    
     if (waypoint.getLocation() != null) {
       waypoint.setLength(length);
       waypoint.setDuration(waypoint.getLocation().getTime()
@@ -823,6 +829,11 @@ public class TrackRecordingService extends Service implements LocationListener {
    * @return the unique id of the inserted marker
    */
   public long insertStatisticsMarker(Location location) {
+    if (!isRecording()) {
+      throw new IllegalStateException(
+          "Unable to insert statistics marker while not recording!");
+    }
+    
     StringUtils utils = new StringUtils(TrackRecordingService.this);
 
     // Create a new waypoint to save
@@ -930,13 +941,15 @@ public class TrackRecordingService extends Service implements LocationListener {
                 "_id=" + recordingTrack.getId(), null);
           }
           showNotification();
-          recordingTrackId = -1;
+          prefManager.setRecordingTrack(recordingTrackId = -1);
-          prefManager.setRecordingTrack(recordingTrackId);
         }
 
         @Override
         public void deleteAllTracks() {
-          endCurrentTrack();
+          if (isRecording()) {
+            throw new IllegalStateException(
+                "Cannot delete all tracks while recording!");
+          }
           providerUtils.deleteAllTracks();
         }
 
@@ -969,12 +982,13 @@ public class TrackRecordingService extends Service implements LocationListener {
     recordingTrackId = Long.parseLong(trackUri.getLastPathSegment());
     track.setId(recordingTrackId);
     track.setName(String.format(getString(R.string.new_track), recordingTrackId));
+    isRecording = true;
+    isMoving = true;
+    
     providerUtils.updateTrack(track);
     statsBuilder = new TripStatisticsBuilder(startTime);
     waypointStatsBuilder = new TripStatisticsBuilder(startTime);
     currentWaypointId = insertStatisticsMarker(null);
-    isRecording = true;
-    isMoving = true;
     setUpAnnouncer();
     length = 0;
     showNotification();

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

@@ -21,6 +21,7 @@ import com.google.android.apps.mytracks.MyTracksSettings;
 import com.google.android.apps.mytracks.content.MyTracksProvider;
 import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
 import com.google.android.apps.mytracks.content.Track;
+import com.google.android.apps.mytracks.content.Waypoint;
 import com.google.android.apps.mytracks.stats.TripStatistics;
 import com.google.android.maps.mytracks.R;
 
@@ -30,8 +31,8 @@ import android.content.ContextWrapper;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.content.SharedPreferences.Editor;
+import android.location.Location;
 import android.os.IBinder;
-import android.os.RemoteException;
 import android.test.RenamingDelegatingContext;
 import android.test.ServiceTestCase;
 import android.test.mock.MockContentResolver;
@@ -122,6 +123,8 @@ public class TrackRecordingServiceTest
     assertNotNull(getService());
 
     assertTrue(getService().isRecording());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertEquals(123, service.getRecordingTrackId());
   }
 
   @MediumTest
@@ -139,6 +142,8 @@ public class TrackRecordingServiceTest
     assertNotNull(getService());
     
     assertFalse(getService().isRecording());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertEquals(-1, service.getRecordingTrackId());
   }
   
   @MediumTest
@@ -156,6 +161,8 @@ public class TrackRecordingServiceTest
     assertNotNull(getService());
     
     assertFalse(getService().isRecording());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertEquals(-1, service.getRecordingTrackId());
   }
 
   @MediumTest
@@ -174,6 +181,8 @@ public class TrackRecordingServiceTest
     assertNotNull(getService());
     
     assertFalse(getService().isRecording());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertEquals(-1, service.getRecordingTrackId());
   }
 
   @MediumTest
@@ -181,17 +190,19 @@ public class TrackRecordingServiceTest
     List<Track> tracks = providerUtils.getAllTracks();
     assertTrue(tracks.isEmpty());
     
-    ITrackRecordingService service = startAndGetService(createStartIntent());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
     // Test if we start in no-recording mode by default. 
     assertFalse(service.isRecording());
+    assertEquals(-1, service.getRecordingTrackId());
   }
   
   @MediumTest
   public void testRecording_oldTracks() throws Exception {
     createDummyTrack(123, -1, false);
     
-    ITrackRecordingService service = startAndGetService(createStartIntent());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertFalse(service.isRecording());
+    assertEquals(-1, service.getRecordingTrackId());
   }
 
   @MediumTest
@@ -199,7 +210,7 @@ public class TrackRecordingServiceTest
     List<Track> tracks = providerUtils.getAllTracks();
     assertTrue(tracks.isEmpty());
     
-    ITrackRecordingService service = startAndGetService(createStartIntent());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertFalse(service.isRecording());
     
     long id = service.startNewTrack();
@@ -210,30 +221,32 @@ public class TrackRecordingServiceTest
     assertEquals(id, track.getId());
     assertEquals(id, sharedPreferences.getLong(
         context.getString(R.string.recording_track_key), -1));
+    assertEquals(id, service.getRecordingTrackId());
   }
   
   @MediumTest
   public void testStartNewTrack_alreadyRecording() throws Exception {
     createDummyTrack(123, -1, true);
     
-    ITrackRecordingService service = startAndGetService(createStartIntent());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertTrue(service.isRecording());
     
     try {
       service.startNewTrack();
-      fail("Expecting RemoteException");
+      fail("Expecting IllegalStateException");
     } catch (IllegalStateException e) {
       // Expected.
     }
     assertEquals(123, sharedPreferences.getLong(
         context.getString(R.string.recording_track_key), 0));
+    assertEquals(123, service.getRecordingTrackId());
   }  
   
   @MediumTest
   public void testEndCurrentTrack_alreadyRecording() throws Exception {
     createDummyTrack(123, -1, true);
     
-    ITrackRecordingService service = startAndGetService(createStartIntent());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertTrue(service.isRecording());
 
     // End the current track.
@@ -241,25 +254,170 @@ public class TrackRecordingServiceTest
     assertFalse(service.isRecording());
     assertEquals(-1, sharedPreferences.getLong(
         context.getString(R.string.recording_track_key), 0));
+    assertEquals(-1, service.getRecordingTrackId());
   }
   
   @MediumTest
   public void testEndCurrentTrack_noRecording() throws Exception {
-    ITrackRecordingService service = startAndGetService(createStartIntent());
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
     assertFalse(service.isRecording());
 
     // End the current track.
     try {
       service.endCurrentTrack();
-      fail("Expecting RemoteException");
+      fail("Expecting IllegalStateException");
     } catch (IllegalStateException e) {
       // Expected.
     }
     assertEquals(-1, sharedPreferences.getLong(
         context.getString(R.string.recording_track_key), 0));
+    assertEquals(-1, service.getRecordingTrackId());
   }
   
-  private ITrackRecordingService startAndGetService(Intent intent) {
+  @MediumTest
+  public void testDeleteAllTracks_noRecording() throws Exception {
+    createDummyTrack(123, -1, false);
+    assertEquals(1, providerUtils.getAllTracks().size());
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+
+    // Deleting all tracks should succeed.
+    service.deleteAllTracks();
+    assertFalse(service.isRecording());
+    assertTrue(providerUtils.getAllTracks().isEmpty());
+  }
+
+  @MediumTest
+  public void testDeleteAllTracks_noTracks() throws Exception {
+    assertTrue(providerUtils.getAllTracks().isEmpty());
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+
+    // Deleting all tracks should succeed.
+    service.deleteAllTracks();
+    assertFalse(service.isRecording());
+    assertTrue(providerUtils.getAllTracks().isEmpty());
+  }
+  
+  @MediumTest
+  public void testDeleteAllTracks_trackInProgress() throws Exception {
+    createDummyTrack(123, -1, true);
+    assertEquals(1, providerUtils.getAllTracks().size());
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+
+    // Since we have a track in progress, we expect to fail.
+    try {
+      service.deleteAllTracks();
+      fail("Expecting IllegalStateException");
+    } catch (IllegalStateException e) {
+      // Expected.
+    }
+    assertTrue(service.isRecording());
+    assertEquals(1, providerUtils.getAllTracks().size());
+  }
+  
+  @MediumTest
+  public void testHasRecorded_noTracks() throws Exception {
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+    assertFalse(service.hasRecorded());
+  }
+  
+  @MediumTest
+  public void testHasRecorded_trackInProgress() throws Exception {
+    createDummyTrack(123, -1, true);
+    assertEquals(1, providerUtils.getAllTracks().size());
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+    assertTrue(service.hasRecorded());
+  }
+  
+  @MediumTest
+  public void testHasRecorded_oldTracks() throws Exception {
+    createDummyTrack(123, -1, false);
+    assertEquals(1, providerUtils.getAllTracks().size());
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+    assertTrue(service.hasRecorded());
+  }
+  
+  @MediumTest
+  public void testInsertStatisticsMarker_noRecordingTrack() throws Exception {
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+    
+    Location loc = new Location("gps");
+    try {
+      service.insertStatisticsMarker(loc);
+      fail("Expecting IllegalStateException");
+    } catch (IllegalStateException e) {
+      // Expected.
+    }
+  }
+  
+  @MediumTest
+  public void testInsertStatisticsMarker_validLocation() throws Exception {
+    createDummyTrack(123, -1, true);
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+    
+    Location loc = new Location("gps");
+    assertEquals(1, service.insertStatisticsMarker(loc));
+    assertEquals(2, service.insertStatisticsMarker(loc));
+    
+    // TODO: Add more checks.
+  }
+
+  @MediumTest
+  public void testInsertWaypointMarker_noRecordingTrack() throws Exception {
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+    
+    Location loc = new Location("gps");
+    Waypoint waypoint = new Waypoint();
+    waypoint.setId(1);
+    waypoint.setLocation(loc);
+    try {
+      service.insertWaypointMarker(waypoint);
+      fail("Expecting IllegalStateException");
+    } catch (IllegalStateException e) {
+      // Expected.
+    }
+  }
+  
+  @MediumTest
+  public void testInsertWaypointMarker_invalidWaypoint() throws Exception {
+    createDummyTrack(123, -1, true);
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+    
+    Waypoint waypoint = new Waypoint();
+    assertEquals(-1, service.insertWaypointMarker(waypoint));
+  }
+  
+  @MediumTest
+  public void testInsertWaypointMarker_validWaypoint() throws Exception {
+    createDummyTrack(123, -1, true);
+    
+    ITrackRecordingService service = bindAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+    
+    Location loc = new Location("gps");
+    Waypoint waypoint = new Waypoint();
+    waypoint.setId(1);
+    waypoint.setLocation(loc);
+    assertEquals(1, service.insertWaypointMarker(waypoint));
+  }
+  
+  private ITrackRecordingService bindAndGetService(Intent intent) {
     ITrackRecordingService service = ITrackRecordingService.Stub.asInterface(
         bindService(intent));
     assertNotNull(service);