From 7a693a90f7706554adfbc82dbe874be29cd1cae2 Mon Sep 17 00:00:00 2001 From: Bartlomiej Niechwiej Date: Wed, 17 Nov 2010 22:18:50 -0800 Subject: [PATCH] Make the app single process only, including the following fixes: 1) Simplify SharedPreferences management 2) Shut down signalManager/splitManager to avoid random crashes 3) Get rid of sharedPreferencesChanged from the service interface All tests pass. --- MyTracks/AndroidManifest.xml | 2 - .../android/apps/mytracks/MyTracks.java | 90 ++++--------------- .../services/DefaultTrackNameFactory.java | 1 - .../services/ITrackRecordingService.aidl | 9 -- .../mytracks/services/PreferenceManager.java | 36 ++++---- .../services/TaskExecuterManager.java | 26 ++++-- .../services/TrackRecordingService.java | 42 +-------- .../android/apps/mytracks/MyTracksTest.java | 71 --------------- 8 files changed, 58 insertions(+), 219 deletions(-) diff --git a/MyTracks/AndroidManifest.xml b/MyTracks/AndroidManifest.xml index 3e72ff098..935b3ebb9 100755 --- a/MyTracks/AndroidManifest.xml +++ b/MyTracks/AndroidManifest.xml @@ -13,7 +13,6 @@ android:value="AEdPqrEAAAAIi-_QiwoRSc9_bAC9cmuNXTQyU8ajJmGtKdhskQ" /> = 0; } @@ -701,58 +700,18 @@ public class MyTracks extends TabActivity implements OnTouchListener, } @Override - public void onSharedPreferenceChanged( - SharedPreferences sharedPreferences, String key) { - // The service itself cannot listen to changes (not supported by Android for - // services that run in a separate process). So we'll notify it manually: - if (key != null && trackRecordingService != null) { - try { - trackRecordingService.sharedPreferenceChanged(key); - } catch (RemoteException e) { - Log.w(MyTracksConstants.TAG, - "MyTracks: Cannot notify track recording service of changes " - + "to shared preferences: ", e); - } - } + public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, + String key) { if (key != null && key.equals(getString(R.string.selected_track_key))) { - selectedTrackId = - sharedPreferences.getLong(getString(R.string.selected_track_key), -1); + selectedTrackId = sharedPreferences.getLong( + getString(R.string.selected_track_key), -1); + } + if (key != null && key.equals(getString(R.string.recording_track_key))) { + recordingTrackId = sharedPreferences.getLong( + getString(R.string.recording_track_key), -1); } } - /** - * Simulates the recording of a random location. - * This is for debugging and testing only. Useful if there is no GPS signal - * available. - */ -// public void recordRandomLocation() { -// if (trackRecordingService != null) { -// Location loc = new Location("gps"); -// double latitude = 37.5 + random.nextDouble() / 1000; -// double longitude = -120.0 + random.nextDouble() / 1000; -// loc.setLatitude(latitude); -// loc.setLongitude(longitude); -// loc.setAltitude(random.nextDouble() * 100); -// loc.setTime(System.currentTimeMillis()); -// loc.setSpeed(random.nextFloat()); -// MyTracksMap map = -// (MyTracksMap) getLocalActivityManager().getActivity("tab1"); -// if (map != null) { -// map.onLocationChanged(loc); -// } -// StatsActivity stats = -// (StatsActivity) getLocalActivityManager().getActivity("tab2"); -// if (stats != null) { -// stats.onLocationChanged(loc); -// } -// try { -// trackRecordingService.recordLocation(loc); -// } catch (RemoteException e) { -// Log.e(MyTracksConstants.TAG, "MyTracks", e); -// } -// } -// } - /** * Resets status information for sending to MyMaps/Docs. */ @@ -1061,8 +1020,6 @@ public class MyTracks extends TabActivity implements OnTouchListener, ITrackRecordingService trackRecordingService) { try { recordingTrackId = trackRecordingService.startNewTrack(); - // TODO: This is a hack to propagate recordingTrackId in multiprocess env. - setRecordingTrackId(recordingTrackId); // Select the recording track. setSelectedTrackId(recordingTrackId); Toast.makeText(this, getString(R.string.status_now_recording), @@ -1104,8 +1061,6 @@ public class MyTracks extends TabActivity implements OnTouchListener, Intent intent = new Intent(MyTracks.this, MyTracksDetails.class); intent.putExtra("trackid", recordingTrackId); intent.putExtra("hasCancelButton", false); - // TODO: This is a hack to propagate recordingTrackId in multiprocess env. - setRecordingTrackId(recordingTrackId = -1); startActivity(intent); } tryUnbindTrackRecordingService(); @@ -1156,29 +1111,16 @@ public class MyTracks extends TabActivity implements OnTouchListener, * @param trackId the id of the track */ public void setSelectedTrackId(final long trackId) { - runOnUiThread(new Runnable() { - public void run() { - SharedPreferences.Editor editor = sharedPreferences.edit(); - editor.putLong(getString(R.string.selected_track_key), trackId); - editor.commit(); - } - }); + sharedPreferences + .edit() + .putLong(getString(R.string.selected_track_key), trackId) + .commit(); } long getSelectedTrackId() { return selectedTrackId; } - private void setRecordingTrackId(final long trackId) { - runOnUiThread(new Runnable() { - public void run() { - SharedPreferences.Editor editor = sharedPreferences.edit(); - editor.putLong(getString(R.string.recording_track_key), trackId); - editor.commit(); - } - }); - } - /** * Binds to track recording service if it is running. */ diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/DefaultTrackNameFactory.java b/MyTracks/src/com/google/android/apps/mytracks/services/DefaultTrackNameFactory.java index 802199460..79d2f5328 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/DefaultTrackNameFactory.java +++ b/MyTracks/src/com/google/android/apps/mytracks/services/DefaultTrackNameFactory.java @@ -20,7 +20,6 @@ import com.google.android.maps.mytracks.R; import android.content.Context; import android.content.SharedPreferences; -import android.text.format.Time; import java.text.SimpleDateFormat; import java.util.Date; diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/ITrackRecordingService.aidl b/MyTracks/src/com/google/android/apps/mytracks/services/ITrackRecordingService.aidl index c13e04d1e..d0e492563 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/ITrackRecordingService.aidl +++ b/MyTracks/src/com/google/android/apps/mytracks/services/ITrackRecordingService.aidl @@ -84,13 +84,4 @@ interface ITrackRecordingService { * Deletes all the stored tracks. */ void deleteAllTracks(); - - /** - * Notifies the service that its preferences may have been changed. - * This is necessary because the service running on a separate process cannot - * listen to the changes itself. - * - * @param key the preference key which may have changed - */ - void sharedPreferenceChanged(in String key); } diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/PreferenceManager.java b/MyTracks/src/com/google/android/apps/mytracks/services/PreferenceManager.java index 5081094d3..2fe7e5c33 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/PreferenceManager.java +++ b/MyTracks/src/com/google/android/apps/mytracks/services/PreferenceManager.java @@ -20,7 +20,7 @@ import com.google.android.apps.mytracks.MyTracksSettings; import com.google.android.maps.mytracks.R; import android.content.SharedPreferences; -import android.content.SharedPreferences.Editor; +import android.content.SharedPreferences.OnSharedPreferenceChangeListener; import android.util.Log; /** @@ -28,8 +28,9 @@ import android.util.Log; * * @author Sandor Dornbush */ -public class PreferenceManager { +public class PreferenceManager implements OnSharedPreferenceChangeListener { private TrackRecordingService service; + private SharedPreferences sharedPreferences; private final String announcementFrequencyKey; private final String autoResumeTrackCurrentRetryKey; private final String autoResumeTrackTimeoutKey; @@ -44,11 +45,14 @@ public class PreferenceManager { public PreferenceManager(TrackRecordingService service) { this.service = service; - if (getSharedPreferences() == null) { + this.sharedPreferences = service.getSharedPreferences( + MyTracksSettings.SETTINGS_NAME, 0); + if (sharedPreferences == null) { Log.w(MyTracksConstants.TAG, "TrackRecordingService: Couldn't get shared preferences."); throw new IllegalStateException("Couldn't get shared preferences"); } + sharedPreferences.registerOnSharedPreferenceChangeListener(this); announcementFrequencyKey = service.getString(R.string.announcement_frequency_key); @@ -72,6 +76,9 @@ public class PreferenceManager { service.getString(R.string.signal_sampling_frequency_key); splitFrequencyKey = service.getString(R.string.split_frequency_key); + + // Refresh all properties. + onSharedPreferenceChanged(sharedPreferences, null); } /** @@ -80,8 +87,9 @@ public class PreferenceManager { * * @param key the key that changed (may be null to update all preferences) */ - public void onSharedPreferenceChanged(String key) { - SharedPreferences sharedPreferences = getSharedPreferences(); + @Override + public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, + String key) { if (key == null || key.equals(minRecordingDistanceKey)) { service.setMinRecordingDistance( sharedPreferences.getInt( @@ -160,18 +168,16 @@ public class PreferenceManager { } public void setAutoResumeTrackCurrentRetry(int retryAttempts) { - SharedPreferences.Editor editor = getSharedPreferences().edit(); - editor.putInt(autoResumeTrackCurrentRetryKey, retryAttempts); - editor.commit(); + sharedPreferences + .edit() + .putInt(autoResumeTrackCurrentRetryKey, retryAttempts) + .commit(); } public void setRecordingTrack(long id) { - Editor editor = getSharedPreferences().edit(); - editor.putLong(recordingTrackKey, id); - editor.commit(); - } - - private SharedPreferences getSharedPreferences() { - return service.getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0); + sharedPreferences + .edit() + .putLong(recordingTrackKey, id) + .commit(); } } diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/TaskExecuterManager.java b/MyTracks/src/com/google/android/apps/mytracks/services/TaskExecuterManager.java index d378ab301..41aab2e1b 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/TaskExecuterManager.java +++ b/MyTracks/src/com/google/android/apps/mytracks/services/TaskExecuterManager.java @@ -15,10 +15,10 @@ */ package com.google.android.apps.mytracks.services; -import android.util.Log; - import com.google.android.apps.mytracks.MyTracksConstants; +import android.util.Log; + /** * This class will manage a period task executer. * @@ -26,13 +26,12 @@ import com.google.android.apps.mytracks.MyTracksConstants; */ public class TaskExecuterManager { - int frequency; - PeriodicTask task; - PeriodicTaskExecuter executer; + private int frequency; + private PeriodicTask task; + private PeriodicTaskExecuter executer; - public TaskExecuterManager(int frequency, - PeriodicTask task, - TrackRecordingService service) { + public TaskExecuterManager(int frequency, PeriodicTask task, + TrackRecordingService service) { this.task = task; setFrequency(frequency, service); } @@ -69,7 +68,7 @@ public class TaskExecuterManager { } /** - * Restore the task at the current frequency. + * Restores the task at the current frequency. */ public void restore() { if (frequency > 0) { @@ -77,4 +76,13 @@ public class TaskExecuterManager { executer.scheduleTask(frequency * 60000); } } + + /** + * Shuts down this executer. + */ + public void shutdown() { + if (executer != null) { + executer.shutdown(); + } + } } diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java b/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java index fb1a0ad97..4f081dea0 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java +++ b/MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java @@ -103,7 +103,7 @@ public class TrackRecordingService extends Service implements LocationListener { * recorded points (as compared to each location fix). It's used to overlay * waypoints precisely in the elevation profile chart. */ - private double length = 0; + private double length; /** * Status announcer executer. @@ -118,7 +118,7 @@ public class TrackRecordingService extends Service implements LocationListener { * The interval in milliseconds that we have requested to be notified of gps * readings. */ - private long currentRecordingInterval = 0; + private long currentRecordingInterval; /** * The policy used to decide how often we should request gps updates. @@ -605,33 +605,6 @@ public class TrackRecordingService extends Service implements LocationListener { // Do nothing } - /* - * SharedPreferencesChangeListener interface implementation. Note that - * services don't currently receive this event (Android platform limitation). - * This should be called from an activity whenever settings change. - */ - - /** - * Notifies that preferences have changed. - * Call this with key == null to update all preferences in one call. - * - * @param key the key that changed (may be null to update all preferences) - */ - public void onSharedPreferenceChanged(final String key) { - Log.d(MyTracksConstants.TAG, - "TrackRecordingService.onSharedPreferenceChanged"); - handler.post(new Runnable() { - @Override - public void run() { - prefManager.onSharedPreferenceChanged(key); - - if (isRecording) { - registerLocationListener(); - } - } - }); - } - /* * Application lifetime events: ============================ */ @@ -652,7 +625,6 @@ public class TrackRecordingService extends Service implements LocationListener { new TaskExecuterManager(-1, strengthTaskFactory.create(this), this); prefManager = new PreferenceManager(this); - prefManager.onSharedPreferenceChanged(null); registerLocationListener(); acquireWakeLock(); /** @@ -731,6 +703,7 @@ public class TrackRecordingService extends Service implements LocationListener { showNotification(); unregisterLocationListener(); shutdownAnnouncer(); + signalManager.shutdown(); splitManager.shutdown(); super.onDestroy(); } @@ -962,8 +935,8 @@ public class TrackRecordingService extends Service implements LocationListener { throw new IllegalStateException("No recording track in progress!"); } - isRecording = false; shutdownAnnouncer(); + isRecording = false; Track recordingTrack = providerUtils.getTrack(recordingTrackId); if (recordingTrack != null) { TripStatistics stats = recordingTrack.getStatistics(); @@ -998,13 +971,6 @@ public class TrackRecordingService extends Service implements LocationListener { public void recordLocation(Location loc) { onLocationChanged(loc); } - - @Override - public void sharedPreferenceChanged(String key) { - Log.d(MyTracksConstants.TAG, - "TrackRecordingService.sharedPreferenceChanged: " + key); - onSharedPreferenceChanged(key); - } }; public long startNewTrack() { diff --git a/MyTracksTest/src/com/google/android/apps/mytracks/MyTracksTest.java b/MyTracksTest/src/com/google/android/apps/mytracks/MyTracksTest.java index 8d5887e40..545c09450 100644 --- a/MyTracksTest/src/com/google/android/apps/mytracks/MyTracksTest.java +++ b/MyTracksTest/src/com/google/android/apps/mytracks/MyTracksTest.java @@ -15,7 +15,6 @@ */ package com.google.android.apps.mytracks; -import com.google.android.apps.mytracks.services.ITrackRecordingService; import com.google.android.maps.mytracks.R; import android.app.Activity; @@ -193,76 +192,6 @@ public class MyTracksTest extends ActivityInstrumentationTestCase2{ assertEquals(selectedTrackId, getActivity().getSelectedTrackId()); } - public void testRecording_changePreferences() throws Exception { - // Make sure we can start MyTracks and the activity doesn't start recording. - assertNotNull(getActivity()); - assertNotNull(MyTracks.getInstance()); - assertNotNull(getActivity().getSharedPreferences()); - - // Check if not recording. - clearSelectedAndRecordingTracks(); - waitForIdle(); - assertFalse(getActivity().isRecording()); - assertEquals(-1, getActivity().getRecordingTrackId()); - long selectedTrackId = getActivity().getSharedPreferences().getLong( - getActivity().getString(R.string.selected_track_key), -1); - assertEquals(selectedTrackId, getActivity().getSelectedTrackId()); - - // Start a new track. - getActivity().startRecording(); - long recordingTrackId = awaitRecordingStatus(5000, true); - assertTrue(recordingTrackId >= 0); - - // Wait until we are done and make sure that selectedTrack = recordingTrack. - waitForIdle(); - assertEquals(recordingTrackId, getActivity().getSharedPreferences().getLong( - getActivity().getString(R.string.recording_track_key), -1)); - selectedTrackId = getActivity().getSharedPreferences().getLong( - getActivity().getString(R.string.selected_track_key), -1); - assertEquals(recordingTrackId, selectedTrackId); - assertEquals(selectedTrackId, getActivity().getSelectedTrackId()); - - // Change shared preferences and observe if the service notices the change. - Editor editor = getActivity().getSharedPreferences().edit(); - editor.putInt(getActivity().getString(R.string.announcement_frequency_key), - 1); - editor.putInt(getActivity().getString(R.string.split_frequency_key), 1); - editor.putInt( - getActivity().getString(R.string.signal_sampling_frequency_key), 1); - editor.commit(); - - // Notify the service about changed preferences. - ITrackRecordingService service = getActivity().getTrackRecordingService(); - assertNotNull(service); - service.sharedPreferenceChanged(null); - - // TODO: Test if the service has updated its preferences. - - // Watch for MyTracksDetails activity. - ActivityMonitor monitor = getInstrumentation().addMonitor( - MyTracksDetails.class.getName(), null, false); - - // Now, stop the track and make sure that it is still selected, but - // no longer recording. - getActivity().stopRecording(); - - // Check if we got back MyTracksDetails activity. - Activity activity = getInstrumentation().waitForMonitor(monitor); - assertTrue(activity instanceof MyTracksDetails); - // Simulate a click on Save button. - Button save = (Button) activity.findViewById(R.id.trackdetails_save); - save.performClick(); - - // Check if after stopping the service all properties are up to date. - recordingTrackId = awaitRecordingStatus(5000, false); - assertEquals(-1, recordingTrackId); - assertEquals(recordingTrackId, getActivity().getRecordingTrackId()); - assertEquals(recordingTrackId, getActivity().getSharedPreferences().getLong( - getActivity().getString(R.string.recording_track_key), -1)); - // Make sure this is the same track as the last recording track ID. - assertEquals(selectedTrackId, getActivity().getSelectedTrackId()); - } - /** * Waits until the UI thread becomes idle. */