Fix crash when altering rate for voicemails without content.
- Repro for crash was to click on voicemail from call log without
content, try to play it (which would silently fail), then try to
use the rate increase/decrease buttons.
- The error being reported to the ui was disabling all the ui elements
but not the rate buttons.
- This fix makes the ui disable the rate buttons too, as well as showing
a Toast, and logging the exception to logcat.
- This cl also adds the unit tests that prove that the bug is fixed.
- In the process, I discovered another bug where missing extras from
the intent used to start the CallDetailsActivity could cause a crash,
so this adds a test and a fix for that case too.
- Also introduces IntegrationTestUtils class, with some useful methods
for doing things like clicking on different parts of the ui.
Bug: 5047879
Change-Id: I46d18723fe783a7a820446e1e13e19b5af82fa5c
diff --git a/src/com/android/contacts/CallDetailActivity.java b/src/com/android/contacts/CallDetailActivity.java
index 2bea2f3..14fec85 100644
--- a/src/com/android/contacts/CallDetailActivity.java
+++ b/src/com/android/contacts/CallDetailActivity.java
@@ -174,13 +174,7 @@
public void onResume() {
super.onResume();
updateData(getCallLogEntryUris());
- Uri voicemailUri = getIntent().getExtras().getParcelable(EXTRA_VOICEMAIL_URI);
- optionallyHandleVoicemail(voicemailUri);
- if (voicemailUri != null) {
- mAsyncQueryHandler.startVoicemailStatusQuery(voicemailUri);
- } else {
- mStatusMessageView.setVisibility(View.GONE);
- }
+ optionallyHandleVoicemail();
}
/**
@@ -189,18 +183,22 @@
* If the Intent used to start this Activity contains the suitable extras, then start voicemail
* playback. If it doesn't, then hide the voicemail ui.
*/
- private void optionallyHandleVoicemail(Uri voicemailUri) {
+ private void optionallyHandleVoicemail() {
+ Uri voicemailUri = getIntent().getParcelableExtra(EXTRA_VOICEMAIL_URI);
FragmentManager manager = getFragmentManager();
VoicemailPlaybackFragment fragment = (VoicemailPlaybackFragment) manager.findFragmentById(
R.id.voicemail_playback_fragment);
- if (voicemailUri == null) {
- // No voicemail uri: hide the voicemail fragment.
- manager.beginTransaction().hide(fragment).commit();
- } else {
- // A voicemail: extra tells us if we should start playback or not.
- boolean startPlayback = getIntent().getExtras().getBoolean(
+ if (voicemailUri != null) {
+ // Has voicemail uri: leave the fragment visible. Optionally start the playback.
+ // Do a query to fetch the voicemail status messages.
+ boolean startPlayback = getIntent().getBooleanExtra(
EXTRA_VOICEMAIL_START_PLAYBACK, false);
fragment.setVoicemailUri(voicemailUri, startPlayback);
+ mAsyncQueryHandler.startVoicemailStatusQuery(voicemailUri);
+ } else {
+ // No voicemail uri: hide the voicemail fragment and the status view.
+ manager.beginTransaction().hide(fragment).commit();
+ mStatusMessageView.setVisibility(View.GONE);
}
}
diff --git a/src/com/android/contacts/CallDetailActivityQueryHandler.java b/src/com/android/contacts/CallDetailActivityQueryHandler.java
index c1d87b2..df5b4f7 100644
--- a/src/com/android/contacts/CallDetailActivityQueryHandler.java
+++ b/src/com/android/contacts/CallDetailActivityQueryHandler.java
@@ -57,8 +57,8 @@
* If the voicemail record does not have an audio yet then it fires the second query to get the
* voicemail status of the associated source.
*/
- public void startVoicemailStatusQuery(Uri voicemaiUri) {
- startQuery(QUERY_VOICEMAIL_CONTENT_TOKEN, null, voicemaiUri, VOICEMAIL_CONTENT_PROJECTION,
+ public void startVoicemailStatusQuery(Uri voicemailUri) {
+ startQuery(QUERY_VOICEMAIL_CONTENT_TOKEN, null, voicemailUri, VOICEMAIL_CONTENT_PROJECTION,
null, null, null);
}
@@ -67,11 +67,12 @@
try {
if (token == QUERY_VOICEMAIL_CONTENT_TOKEN) {
// Query voicemail status only if this voicemail record does not have audio.
- if (cursor.moveToFirst() && hasNoAudio(cursor)) {
+ if (moveToFirst(cursor) && hasNoAudio(cursor)) {
startQuery(QUERY_VOICEMAIL_STATUS_TOKEN, null,
Status.buildSourceUri(getSourcePackage(cursor)),
VoicemailStatusHelperImpl.PROJECTION, null, null, null);
} else {
+ // nothing to show in status
mCallDetailActivity.updateVoicemailStatusMessage(null);
}
} else if (token == QUERY_VOICEMAIL_STATUS_TOKEN) {
@@ -84,6 +85,15 @@
}
}
+ /** Check that the cursor is non-null and can be moved to first. */
+ private boolean moveToFirst(Cursor cursor) {
+ if (cursor == null || !cursor.moveToFirst()) {
+ Log.e(TAG, "Cursor not valid, could not move to first");
+ return false;
+ }
+ return true;
+ }
+
private boolean hasNoAudio(Cursor voicemailCursor) {
return voicemailCursor.getInt(HAS_CONTENT_COLUMN_INDEX) == 0;
}
diff --git a/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java b/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java
index 328360b..436f13b 100644
--- a/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java
+++ b/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java
@@ -16,21 +16,23 @@
package com.android.contacts.voicemail;
+import com.android.contacts.R;
+import com.android.ex.variablespeed.MediaPlayerProxy;
+import com.android.ex.variablespeed.VariableSpeed;
+
import android.app.Fragment;
import android.content.Context;
import android.media.AudioManager;
import android.net.Uri;
import android.os.Bundle;
+import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageButton;
import android.widget.SeekBar;
import android.widget.TextView;
-
-import com.android.contacts.R;
-import com.android.ex.variablespeed.MediaPlayerProxy;
-import com.android.ex.variablespeed.VariableSpeed;
+import android.widget.Toast;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -50,6 +52,7 @@
*/
@NotThreadSafe
public class VoicemailPlaybackFragment extends Fragment {
+ private static final String TAG = "VoicemailPlayback";
private static final int NUMBER_OF_THREADS_IN_POOL = 2;
private VoicemailPlaybackPresenter mPresenter;
@@ -217,10 +220,14 @@
}
@Override
- public void playbackError() {
+ public void playbackError(Exception e) {
+ mRateIncreaseButton.setEnabled(false);
+ mRateDecreaseButton.setEnabled(false);
mStartStopButton.setEnabled(false);
mPlaybackSeek.setProgress(0);
mPlaybackSeek.setEnabled(false);
+ Toast.makeText(getActivity(), R.string.voicemail_playback_error, Toast.LENGTH_SHORT);
+ Log.e(TAG, "Could not play voicemail", e);
}
@Override
diff --git a/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java b/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java
index 11764c9..53e64e9 100644
--- a/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java
+++ b/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java
@@ -16,6 +16,9 @@
package com.android.contacts.voicemail;
+import com.android.ex.variablespeed.MediaPlayerProxy;
+import com.android.ex.variablespeed.SingleThreadedMediaPlayerProxy;
+
import android.content.Context;
import android.media.MediaPlayer;
import android.net.Uri;
@@ -23,9 +26,6 @@
import android.view.View;
import android.widget.SeekBar;
-import com.android.ex.variablespeed.MediaPlayerProxy;
-import com.android.ex.variablespeed.SingleThreadedMediaPlayerProxy;
-
import java.io.IOException;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
@@ -61,7 +61,7 @@
int getDesiredClipPosition();
void playbackStarted();
void playbackStopped();
- void playbackError();
+ void playbackError(Exception e);
boolean isSpeakerPhoneOn();
void setSpeakerPhoneOn(boolean on);
void finish();
@@ -216,7 +216,7 @@
}
private void handleError(Exception e) {
- mView.playbackError();
+ mView.playbackError(e);
mPlayer.release();
mPositionUpdater.stopUpdating();
}