Fix some voicemail playback bugs
* Don't fetch voicemail until playback is started
* Prevent multiple voicemail fetches for the same voicemail
* Fixed an issue where the seekbar would be stuck in the same
position even after seeking
* Fix for playback position getting reset after the voicemail
view is scrolled out of view and back in
* Respect the invariant that if mMediaPlayer is null, mPrepared
is always false. This fixes potential NPEs when resuming playback.
Bug: 22127956
Bug: 22333494
Change-Id: I5c419424950c0e21317cbd133ca6f7e1edd9fd31
diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java b/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java
index 2d65504..2017bc5 100644
--- a/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java
+++ b/src/com/android/dialer/voicemail/VoicemailPlaybackLayout.java
@@ -75,6 +75,21 @@
private final Object mLock = new Object();
@GuardedBy("mLock") private ScheduledFuture<?> mScheduledFuture;
+ private Runnable mUpdateClipPositionRunnable = new Runnable() {
+ @Override
+ public void run() {
+ int currentPositionMs = 0;
+ synchronized (mLock) {
+ if (mScheduledFuture == null || mPresenter == null) {
+ // This task has been canceled. Just stop now.
+ return;
+ }
+ currentPositionMs = mPresenter.getMediaPlayerPosition();
+ }
+ setClipPosition(currentPositionMs, mDurationMs);
+ }
+ };
+
public PositionUpdater(int durationMs, ScheduledExecutorService executorService) {
mDurationMs = durationMs;
mExecutorService = executorService;
@@ -82,28 +97,12 @@
@Override
public void run() {
- post(new Runnable() {
- @Override
- public void run() {
- int currentPositionMs = 0;
- synchronized (mLock) {
- if (mScheduledFuture == null || mPresenter == null) {
- // This task has been canceled. Just stop now.
- return;
- }
- currentPositionMs = mPresenter.getMediaPlayerPosition();
- }
- setClipPosition(currentPositionMs, mDurationMs);
- }
- });
+ post(mUpdateClipPositionRunnable);
}
public void startUpdating() {
synchronized (mLock) {
- if (mScheduledFuture != null) {
- mScheduledFuture.cancel(false);
- mScheduledFuture = null;
- }
+ cancelPendingRunnables();
mScheduledFuture = mExecutorService.scheduleAtFixedRate(
this, 0, SLIDER_UPDATE_PERIOD_MILLIS, TimeUnit.MILLISECONDS);
}
@@ -111,12 +110,17 @@
public void stopUpdating() {
synchronized (mLock) {
- if (mScheduledFuture != null) {
- mScheduledFuture.cancel(false);
- mScheduledFuture = null;
- }
+ cancelPendingRunnables();
}
}
+
+ private void cancelPendingRunnables() {
+ if (mScheduledFuture != null) {
+ mScheduledFuture.cancel(true);
+ mScheduledFuture = null;
+ }
+ removeCallbacks(mUpdateClipPositionRunnable);
+ }
}
/**
@@ -139,7 +143,7 @@
@Override
public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) {
- setClipPosition(seekBar.getProgress(), seekBar.getMax());
+ setClipPosition(progress, seekBar.getMax());
}
};
@@ -246,6 +250,10 @@
onSpeakerphoneOn(mPresenter.isSpeakerphoneOn());
}
+ if (mPositionUpdater != null) {
+ mPositionUpdater.stopUpdating();
+ mPositionUpdater = null;
+ }
mPositionUpdater = new PositionUpdater(duration, executorService);
mPositionUpdater.startUpdating();
}
diff --git a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
index f262105..f76af59 100644
--- a/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
+++ b/src/com/android/dialer/voicemail/VoicemailPlaybackPresenter.java
@@ -207,6 +207,11 @@
mPosition = savedInstanceState.getInt(CLIP_POSITION_KEY, 0);
mIsPlaying = savedInstanceState.getBoolean(IS_PLAYING_STATE_KEY, false);
}
+
+ if (mMediaPlayer == null) {
+ mIsPrepared = false;
+ mIsPlaying = false;
+ }
}
/**
@@ -240,9 +245,14 @@
mVoicemailUri = voicemailUri;
mDuration.set(0);
- mIsPlaying = startPlayingImmediately;
- checkForContent();
+ if (startPlayingImmediately) {
+ // Since setPlaybackView can get called during the view binding process, we don't
+ // want to reset mIsPlaying to false if the user is currently playing the
+ // voicemail and the view is rebound.
+ mIsPlaying = startPlayingImmediately;
+ checkForContent();
+ }
// Default to earpiece.
mView.onSpeakerphoneOn(false);
@@ -284,6 +294,7 @@
if (mMediaPlayer != null) {
mMediaPlayer.release();
mMediaPlayer = null;
+ mIsPrepared = false;
}
if (mActivity != null) {
@@ -489,6 +500,7 @@
mIsPrepared = true;
mDuration.set(mMediaPlayer.getDuration());
+ mPosition = mMediaPlayer.getCurrentPosition();
mView.enableUiElements();
Log.d(TAG, "onPrepared: mPosition=" + mPosition);
@@ -561,6 +573,10 @@
*/
public void resumePlayback() {
if (!mIsPrepared) {
+ // If we haven't downloaded the voicemail yet, attempt to download it.
+ checkForContent();
+ mIsPlaying = true;
+
return;
}
@@ -607,7 +623,7 @@
mIsPlaying = false;
- if (mMediaPlayer.isPlaying()) {
+ if (mMediaPlayer != null && mMediaPlayer.isPlaying()) {
mMediaPlayer.pause();
}