Merge to head.

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

@@ -139,8 +139,8 @@ public class ChartActivity extends Activity implements TrackDataListener {
   }
 
   @Override
-  protected void onStart() {
+  protected void onResume() {
-    super.onStart();
+    super.onResume();
 
     dataHub.registerTrackDataListener(this, EnumSet.of(
         ListenerDataType.SELECTED_TRACK_CHANGED,
@@ -151,10 +151,10 @@ public class ChartActivity extends Activity implements TrackDataListener {
   }
 
   @Override
-  protected void onStop() {
+  protected void onPause() {
     dataHub.unregisterTrackDataListener(this);
 
-    super.onStop();
+    super.onPause();
   }
 
   private void zoomIn() {
@@ -418,6 +418,9 @@ public class ChartActivity extends Activity implements TrackDataListener {
 
   @Override
   public boolean onUnitsChanged(boolean metric) {
+    boolean changed = metric != this.metricUnits;
+    if (!changed) return false;
+
     this.metricUnits = metric;
 
     chartView.setMetricUnits(metric);
@@ -427,6 +430,9 @@ public class ChartActivity extends Activity implements TrackDataListener {
 
   @Override
   public boolean onReportSpeedChanged(boolean reportSpeed) {
+    boolean changed = reportSpeed != this.reportSpeed;
+    if (!changed) return false;
+
     this.reportSpeed = reportSpeed;
 
     chartView.setReportSpeed(reportSpeed, this);

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

@@ -178,9 +178,9 @@ public class MapActivity extends com.google.android.maps.MapActivity
   }
 
   @Override
-  protected void onStart() {
+  protected void onResume() {
     Log.d(TAG, "MapActivity.onStart");
-    super.onStart();
+    super.onResume();
 
     dataHub.registerTrackDataListener(this, EnumSet.of(
         ListenerDataType.SELECTED_TRACK_CHANGED,
@@ -201,12 +201,12 @@ public class MapActivity extends com.google.android.maps.MapActivity
   }
 
   @Override
-  protected void onStop() {
+  protected void onPause() {
     Log.d(TAG, "MapActivity.onStop");
 
     dataHub.unregisterTrackDataListener(this);
 
-    super.onStop();
+    super.onPause();
   }
 
   // Utility functions:
@@ -508,8 +508,7 @@ public class MapActivity extends com.google.android.maps.MapActivity
   @Override
   public void onCurrentLocationChanged(Location location) {
     if (!location.getProvider().equals(LocationManager.GPS_PROVIDER)) {
-      Log.d(TAG,
+      return;
-          "MapActivity: Network location update received (provider '" + location.getProvider() + "'.");
     }
 
     currentLocation = location;

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

@@ -28,6 +28,7 @@ import com.google.android.maps.mytracks.R;
 
 import android.app.Activity;
 import android.location.Location;
+import android.location.LocationManager;
 import android.os.Bundle;
 import android.util.DisplayMetrics;
 import android.util.Log;
@@ -131,18 +132,18 @@ public class StatsActivity extends Activity implements TrackDataListener {
   }
 
   @Override
-  protected void onStart() {
+  protected void onResume() {
     dataHub.registerTrackDataListener(this, EnumSet.of(
         ListenerDataType.SELECTED_TRACK_CHANGED,
         ListenerDataType.TRACK_UPDATES,
         ListenerDataType.LOCATION_UPDATES,
         ListenerDataType.DISPLAY_PREFERENCES));
 
-    super.onStart();
+    super.onResume();
   }
 
   @Override
-  protected void onStop() {
+  protected void onPause() {
     dataHub.unregisterTrackDataListener(this);
 
     if (thread != null) {
@@ -155,6 +156,9 @@ public class StatsActivity extends Activity implements TrackDataListener {
 
   @Override
   public boolean onUnitsChanged(boolean metric) {
+    // Ignore if unchanged.
+    if (metric == utils.isMetricUnits()) return false;
+
     utils.setMetricUnits(metric);
     updateLabels();
 
@@ -163,6 +167,9 @@ public class StatsActivity extends Activity implements TrackDataListener {
 
   @Override
   public boolean onReportSpeedChanged(boolean displaySpeed) {
+    // Ignore if unchanged.
+    if (displaySpeed == utils.isReportSpeed()) return false;
+
     utils.setReportSpeed(displaySpeed);
     updateLabels();
   
@@ -254,6 +261,10 @@ public class StatsActivity extends Activity implements TrackDataListener {
 
   @Override
   public void onCurrentLocationChanged(final Location loc) {
+    if (!loc.getProvider().equals(LocationManager.GPS_PROVIDER)) {
+      return;
+    }
+
     if (dataHub.isRecordingSelected()) {
       runOnUiThread(new Runnable() {
         @Override

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

@@ -83,6 +83,8 @@ public class StatsUtilities {
   public void setText(int id, double d, NumberFormat format) {
     if (!Double.isNaN(d) && !Double.isInfinite(d)) {
       setText(id, format.format(d));
+    } else {
+      setUnknown(id);
     }
   }
 
@@ -218,7 +220,7 @@ public class StatsUtilities {
     setGrade(R.id.max_grade_register, maxGrade);
   }
 
-	public void setAllStats(TripStatistics stats) {
+  public void setAllStats(TripStatistics stats) {
     setTime(R.id.moving_time_register, stats.getMovingTime());
     setDistance(R.id.total_distance_register, stats.getTotalDistance() / 1000);
     setSpeed(R.id.average_speed_register, stats.getAverageSpeed() * 3.6);

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

@@ -103,7 +103,7 @@ public class TrackDataHub {
 
     @Override
     public void notifyPointsUpdated() {
-      TrackDataHub.this.notifyPointsUpdated(true,
+      TrackDataHub.this.notifyPointsUpdated(true, 0,
           getListenersFor(ListenerDataType.POINT_UPDATES),
           getListenersFor(ListenerDataType.SAMPLED_OUT_POINT_UPDATES));
     }
@@ -170,16 +170,16 @@ public class TrackDataHub {
 
   // Cached GPS readings
   private Location lastSeenLocation;
-  private boolean hasProviderEnabled;
+  private boolean hasProviderEnabled = true;
   private boolean hasFix;
   private boolean hasGoodFix;
 
   // Transient state about the selected track
   private long selectedTrackId;
-  private long recordingTrackId;
   private long firstSeenLocationId;
   private long lastSeenLocationId;
   private int numLoadedPoints;
+  private int lastSamplingFrequency;
 
   /**
    * Default constructor.
@@ -238,7 +238,6 @@ public class TrackDataHub {
 
   private void loadSharedPreferences() {
     selectedTrackId = preferences.getLong(SELECTED_TRACK_KEY, -1);
-    recordingTrackId = preferences.getLong(RECORDING_TRACK_KEY, -1);
     useMetricUnits = preferences.getBoolean(METRIC_UNITS_KEY, true);
     reportSpeed = preferences.getBoolean(SPEED_REPORTING_KEY, true);
     minRequiredAccuracy = preferences.getInt(MIN_REQUIRED_ACCURACY_KEY,
@@ -339,7 +338,7 @@ public class TrackDataHub {
     if (!started) {
       loadSharedPreferences();
     }
-    return recordingTrackId > 0;
+    return preferences.getLong(RECORDING_TRACK_KEY, -1) > 0;
   }
 
   /** Returns whether the selected track is still being recorded. */
@@ -347,7 +346,8 @@ public class TrackDataHub {
     if (!started) {
       loadSharedPreferences();
     }
-    return recordingTrackId > 0 && recordingTrackId == selectedTrackId;
+    long recordingTrackId = preferences.getLong(RECORDING_TRACK_KEY, -1);
+    return recordingTrackId  > 0 && recordingTrackId == selectedTrackId;
   }
 
   /**
@@ -414,7 +414,11 @@ public class TrackDataHub {
    * Reloads all track data received so far into the specified listeners.
    */
   public void reloadDataForListener(TrackDataListener listener) {
-    reloadDataForListener(listeners.getRegistration(listener));
+    ListenerRegistration registration;
+    synchronized (listeners) {
+      registration = listeners.getRegistration(listener);
+    }
+    reloadDataForListener(registration);
   }
 
   /**
@@ -425,21 +429,29 @@ public class TrackDataHub {
       Log.w(TAG, "Not started, not reloading");
       return;
     }
+    if (registration == null) {
+      return;
+    }
 
     runInListenerThread(new Runnable() {
       @SuppressWarnings("unchecked")
       @Override
       public void run() {
+        // Reload everything if either it's a different track, or the track has been resampled
+        // (this also covers the case of a new registration).
+        boolean reloadAll = registration.lastTrackId != selectedTrackId ||
+                            registration.lastSamplingFrequency != lastSamplingFrequency;
+        Log.d(TAG, "Doing a " + (reloadAll ? "full" : "partial") + " reload for " + registration);
+
         TrackDataListener listener = registration.listener;
         Set<TrackDataListener> listenerSet = Collections.singleton(listener);
 
         if (registration.isInterestedIn(ListenerDataType.DISPLAY_PREFERENCES)) {
-          // Ignore the return values here, we're already sending the full data set anyway
+          reloadAll |= listener.onUnitsChanged(useMetricUnits);
-          listener.onUnitsChanged(useMetricUnits);
+          reloadAll |= listener.onReportSpeedChanged(reportSpeed);
-          listener.onReportSpeedChanged(reportSpeed);
         }
 
-        if (registration.isInterestedIn(ListenerDataType.SELECTED_TRACK_CHANGED)) {
+        if (reloadAll && registration.isInterestedIn(ListenerDataType.SELECTED_TRACK_CHANGED)) {
           notifySelectedTrackChanged(selectedTrackId, listenerSet);
         }
 
@@ -452,8 +464,10 @@ public class TrackDataHub {
         boolean interestedInSampledOutPoints =
             registration.isInterestedIn(ListenerDataType.SAMPLED_OUT_POINT_UPDATES);
         if (interestedInPoints || interestedInSampledOutPoints) {
-          notifyPointsCleared(listenerSet);
+          if (reloadAll) notifyPointsCleared(listenerSet);
+
           notifyPointsUpdated(false,
+              reloadAll ? 0 : registration.lastPointId + 1,
               listenerSet,
               interestedInSampledOutPoints ? listenerSet : Collections.EMPTY_SET);
         }
@@ -481,14 +495,16 @@ public class TrackDataHub {
    * Reloads all track data received so far into the specified listeners.
    */
   private void loadDataForAllListeners() {
-    if (!listeners.hasListeners()) {
-      Log.d(TAG, "No listeners, not reloading");
-      return;
-    }
     if (!started) {
       Log.w(TAG, "Not started, not reloading");
       return;
     }
+    synchronized (listeners) {
+      if (!listeners.hasListeners()) {
+        Log.d(TAG, "No listeners, not reloading");
+        return;
+      }
+    }
 
     runInListenerThread(new Runnable() {
       @Override
@@ -510,7 +526,7 @@ public class TrackDataHub {
         Set<TrackDataListener> sampledOutPointListeners =
             getListenersFor(ListenerDataType.SAMPLED_OUT_POINT_UPDATES);
         notifyPointsCleared(pointListeners);
-        notifyPointsUpdated(true, pointListeners, sampledOutPointListeners);
+        notifyPointsUpdated(true, 0, pointListeners, sampledOutPointListeners);
 
         notifyWaypointUpdated(getListenersFor(ListenerDataType.WAYPOINT_UPDATES));
         
@@ -532,9 +548,7 @@ public class TrackDataHub {
    * @param key the key to the preference that changed
    */
   private void notifyPreferenceChanged(String key) {
-    if (RECORDING_TRACK_KEY.equals(key)) {
+    if (MIN_REQUIRED_ACCURACY_KEY.equals(key)) {
-      recordingTrackId = preferences.getLong(RECORDING_TRACK_KEY, -1);
-    } else if (MIN_REQUIRED_ACCURACY_KEY.equals(key)) {
       minRequiredAccuracy = preferences.getInt(MIN_REQUIRED_ACCURACY_KEY,
           Constants.DEFAULT_MIN_REQUIRED_ACCURACY);
     } else if (METRIC_UNITS_KEY.equals(key)) {
@@ -559,7 +573,9 @@ public class TrackDataHub {
         for (TrackDataListener listener : displayListeners) {
           // TODO: Do the reloading just once for all interested listeners
           if (listener.onReportSpeedChanged(reportSpeed)) {
-            reloadDataForListener(listeners.getRegistration(listener));
+            synchronized (listeners) {
+              reloadDataForListener(listeners.getRegistration(listener));
+            }
           }
         }
       }
@@ -577,7 +593,9 @@ public class TrackDataHub {
 
         for (TrackDataListener listener : displayListeners) {
           if (listener.onUnitsChanged(useMetricUnits)) {
-            reloadDataForListener(listeners.getRegistration(listener));
+            synchronized (listeners) {
+              reloadDataForListener(listeners.getRegistration(listener));
+            }
           }
         }
       }
@@ -796,23 +814,20 @@ public class TrackDataHub {
   /**
    * Notifies the given listeners about track points in the given ID range.
    *
-   * @param minPointId the first point ID to notify, inclusive
-   * @param maxPointId the last poind ID to notify, inclusive
    * @param keepState whether to load and save state about the already-notified points.
    *        If true, only new points are reported.
    *        If false, then the whole track will be loaded, without affecting the store.
-   * @param listeners the listeners to notify
+   * @param minPointId the first point ID to notify, inclusive
-   * @param trackDataListeners 
    */
   private void notifyPointsUpdated(final boolean keepState,
-      final Set<TrackDataListener> sampledListeners,
+      final long minPointId, final Set<TrackDataListener> sampledListeners,
       final Set<TrackDataListener> sampledOutListeners) {
     if (sampledListeners.isEmpty() && sampledOutListeners.isEmpty()) return;
 
     runInListenerThread(new Runnable() {
       @Override
       public void run() {
-        notifyPointsUpdatedSync(keepState, sampledListeners, sampledOutListeners);
+        notifyPointsUpdatedSync(keepState, minPointId, sampledListeners, sampledOutListeners);
       }
     });
   }
@@ -821,12 +836,14 @@ public class TrackDataHub {
    * Asynchronous version of the above method.
    */
   private void notifyPointsUpdatedSync(boolean keepState,
-      Set<TrackDataListener> sampledListeners,
+      long minPointId, Set<TrackDataListener> sampledListeners,
       Set<TrackDataListener> sampledOutListeners) {
     // If we're loading state, start from after the last seen point up to the last recorded one
     // (all new points)
     // If we're not loading state, then notify about all the previously-seen points.
-    long minPointId = keepState ? lastSeenLocationId + 1 : 0;
+    if (minPointId <= 0) {
+      minPointId = keepState ? lastSeenLocationId + 1 : 0;
+    }
     long maxPointId = keepState ? -1 : lastSeenLocationId;
 
     // TODO: Move (re)sampling to a separate class.
@@ -914,11 +931,36 @@ public class TrackDataHub {
       lastSeenLocationId = localLastSeenLocationId;
     }
 
+    // Always keep the sampling frequency - if it changes we'll do a full reload above anyway.
+    lastSamplingFrequency = pointSamplingFrequency;
+
+    // Update the listener state
+    // TODO: Optimize this (sampledOutListeners should be a subset of sampledListeners, plus
+    //       getRegistration does a lookup for every listener, and this is in the critical path).
+    updateListenersState(sampledListeners,
+        currentSelectedTrackId, localLastSeenLocationId, pointSamplingFrequency);
+    updateListenersState(sampledOutListeners,
+        currentSelectedTrackId, localLastSeenLocationId, pointSamplingFrequency);
+
     for (TrackDataListener listener : sampledListeners) {
       listener.onNewTrackPointsDone();
     }
   }
 
+  private void updateListenersState(Set<TrackDataListener> sampledListeners,
+      long trackId, long lastPointId, int samplingFrequency) {
+    synchronized (listeners) {
+      for (TrackDataListener listener : sampledListeners) {
+        ListenerRegistration registration = listeners.getRegistration(listener);
+        if (registration != null) {
+          registration.lastTrackId = trackId;
+          registration.lastPointId = lastPointId;
+          registration.lastSamplingFrequency = samplingFrequency;
+        }
+      }
+    }
+  }
+
   private void notifyNewPoint(Location location,
       long locationId,
       long lastStoredLocationId,

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

@@ -27,6 +27,7 @@ import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.WeakHashMap;
 
 /**
  * Manager for the external data listeners and their listening types.
@@ -39,7 +40,11 @@ class TrackDataListeners {
   static class ListenerRegistration {
     final TrackDataListener listener;
     final EnumSet<ListenerDataType> types;
-    // TODO: Add the last-notified point ID here, to allow pausing/resuming.
+
+    // State that was last notified to the listener, for resuming after a pause.
+    long lastTrackId;
+    long lastPointId;
+    int lastSamplingFrequency;
 
     public ListenerRegistration(TrackDataListener listener,
         EnumSet<ListenerDataType> types) {
@@ -50,12 +55,26 @@ class TrackDataListeners {
     public boolean isInterestedIn(ListenerDataType type) {
       return types.contains(type);
     }
+
+    @Override
+    public String toString() {
+      return "ListenerRegistration [listener=" + listener + ", types=" + types
+          + ", lastTrackId=" + lastTrackId + ", lastPointId=" + lastPointId
+          + ", lastSamplingFrequency=" + lastSamplingFrequency + "]";
+    }
   }
 
   /** Map of external listener to its registration details. */
   private final Map<TrackDataListener, ListenerRegistration> registeredListeners =
       new HashMap<TrackDataListener, ListenerRegistration>();
 
+  /**
+   * Map of external paused listener to its registration details.
+   * This will automatically discard listeners which are GCed.
+   */
+  private final WeakHashMap<TrackDataListener, ListenerRegistration> oldListeners =
+      new WeakHashMap<TrackDataListener, ListenerRegistration>();
+
   /** Map of data type to external listeners interested in it. */
   private final Map<ListenerDataType, Set<TrackDataListener>> listenerSetsPerType =
       new EnumMap<ListenerDataType, Set<TrackDataListener>>(ListenerDataType.class);
@@ -77,11 +96,14 @@ class TrackDataListeners {
    */
   public ListenerRegistration registerTrackDataListener(final TrackDataListener listener, EnumSet<ListenerDataType> dataTypes) {
     Log.d(TAG, "Registered track data listener: " + listener);
-    ListenerRegistration registration = new ListenerRegistration(listener, dataTypes);
+    if (registeredListeners.containsKey(listener)) {
-
-    if (registeredListeners.get(listener) != null) {
       throw new IllegalStateException("Listener already registered");
     }
+
+    ListenerRegistration registration = oldListeners.remove(listener);
+    if (registration == null) {
+      registration = new ListenerRegistration(listener, dataTypes);
+    }
     registeredListeners.put(listener, registration);
 
     for (ListenerDataType type : dataTypes) {
@@ -111,10 +133,17 @@ class TrackDataListeners {
     for (ListenerDataType type : match.types) {
       listenerSetsPerType.get(type).remove(listener);
     }
+
+    // Keep it around in case it's re-registered soon
+    oldListeners.put(listener, match);
   }
 
   public ListenerRegistration getRegistration(TrackDataListener listener) {
-    return registeredListeners.get(listener);
+    ListenerRegistration registration = registeredListeners.get(listener);
+    if (registration == null) {
+      registration = oldListeners.get(listener);
+    }
+    return registration;
   }
 
   public Set<TrackDataListener> getListenersFor(ListenerDataType type) {

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

@@ -478,6 +478,14 @@ public class TrackDataHubTest extends AndroidTestCase {
     // TODO: test loading a track, getting updates, loading another, unloading
   }
 
+  public void testRelisten() {
+    // TODO: test re-registering an old points listener
+  }
+
+  public void testRelisten_changed() {
+    // TODO: test register, get points, unregister, change track, register
+  }
+
   private void expectStart() {
     dataSources.registerOnSharedPreferenceChangeListener(capture(preferenceListenerCapture));
   }