Voicemail ui indicates both time left and playback speed.
- Playback time indicator has changed from current position in
voicemail to time remaining.
- This same text view is also used to briefly display current playback
rate when playback rate buttons are pressed.
- Unit tests to show that this logic works.
- This cl introduces a new class whose job is to handle the transition
between the different text states.
- Fixes a crash when we might release the player before the last
update gets the duration from the released player.
- New test method to fetch a text view's content on the ui thread.
Bug: 5044075
Change-Id: Ie3cf6e58c1a0139edb78cf3564a8aec94512156f
diff --git a/res/values/strings.xml b/res/values/strings.xml
index 5ff5662..4069c49 100644
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -1645,6 +1645,17 @@
server directly to listen to the voicemails. [CHAR LIMIT=20] -->
<string name="voicemail_status_action_call_server">Call voicemail</string>
+ <!-- The slowest voicemail playback speed. [CHAR LIMIT=30] -->
+ <string name="voicemail_speed_slowest">slowest speed</string>
+ <!-- Slower than normal voicemail playback speed. [CHAR LIMIT=30] -->
+ <string name="voicemail_speed_slower">slow speed</string>
+ <!-- Normal voicemail playback speed. [CHAR LIMIT=30] -->
+ <string name="voicemail_speed_normal">normal speed</string>
+ <!-- Faster than normal pvoicemail playback speed. [CHAR LIMIT=30] -->
+ <string name="voicemail_speed_faster">fast speed</string>
+ <!-- Fastest voicemail playback speed. [CHAR LIMIT=30] -->
+ <string name="voicemail_speed_fastest">fastest speed</string>
+
<!-- The counter for calls in a group in the call log [CHAR LIMIT=5] -->
<string name="call_log_item_count">(%1$d)</string>
diff --git a/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java b/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java
index 436f13b..b1da1a1 100644
--- a/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java
+++ b/src/com/android/contacts/voicemail/VoicemailPlaybackFragment.java
@@ -37,7 +37,9 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.NotThreadSafe;
/**
@@ -64,6 +66,7 @@
private TextView mPlaybackPositionText;
private ImageButton mRateDecreaseButton;
private ImageButton mRateIncreaseButton;
+ private TextViewWithMessagesController mTextController;
@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container,
@@ -77,6 +80,7 @@
mPlaybackPositionText = (TextView) view.findViewById(R.id.playback_position_text);
mRateDecreaseButton = (ImageButton) view.findViewById(R.id.rate_decrease_button);
mRateIncreaseButton = (ImageButton) view.findViewById(R.id.rate_increase_button);
+ mTextController = new TextViewWithMessagesController(mPlaybackPositionText);
return view;
}
@@ -173,12 +177,9 @@
}
@Override
- public void setRateDisplay(float rate) {
- // TODO: This isn't being done yet. Old rate display code has been removed.
- // Instead we're going to temporarily fade out the track position when you change
- // rate, and display one of the words "slowest", "slower", "normal", "faster",
- // "fastest" briefly when you change speed, before fading back in the time.
- // At least, that's the current thinking.
+ public void setRateDisplay(float rate, int stringResourceId) {
+ mTextController.setTemporaryText(
+ getActivity().getString(stringResourceId), 1, TimeUnit.SECONDS);
}
@Override
@@ -202,16 +203,15 @@
}
@Override
- public void setClipLength(int clipLengthInMillis) {
- mPlaybackSeek.setMax(clipLengthInMillis);
- // TODO: The old code used to set the static lenght-of-clip text field, but now
- // the thinking is that we will only show this text whilst the recording is stopped.
- }
-
- @Override
- public void setClipPosition(int clipPositionInMillis) {
- mPlaybackSeek.setProgress(clipPositionInMillis);
- mPlaybackPositionText.setText(formatAsMinutesAndSeconds(clipPositionInMillis));
+ public void setClipPosition(int clipPositionInMillis, int clipLengthInMillis) {
+ int seekBarPosition = Math.max(0, clipPositionInMillis);
+ int seekBarMax = Math.max(seekBarPosition, clipLengthInMillis);
+ if (mPlaybackSeek.getMax() != seekBarMax) {
+ mPlaybackSeek.setMax(seekBarMax);
+ }
+ mPlaybackSeek.setProgress(seekBarPosition);
+ mTextController.setPermanentText(
+ formatAsMinutesAndSeconds(seekBarMax - seekBarPosition));
}
@Override
@@ -246,4 +246,61 @@
}
}
}
+
+ /**
+ * Controls a TextView with dynamically changing text.
+ * <p>
+ * There are two methods here of interest,
+ * {@link TextViewWithMessagesController#setPermanentText(String)} and
+ * {@link TextViewWithMessagesController#setTemporaryText(String, long, TimeUnit)}. The
+ * former is used to set the text on the text view immediately, and is used in our case for
+ * the countdown of duration remaining during voicemail playback. The second is used to
+ * temporarily replace this countdown with a message, in our case faster voicemail speed or
+ * slower voicemail speed, before returning to the countdown display.
+ * <p>
+ * All the methods on this class must be called from the ui thread.
+ */
+ private static final class TextViewWithMessagesController {
+ private final Object mLock = new Object();
+ private final TextView mTextView;
+ @GuardedBy("mLock") String mCurrentText = "";
+ @GuardedBy("mLock") Runnable mRunnable;
+
+ public TextViewWithMessagesController(TextView textView) {
+ mTextView = textView;
+ }
+
+ public void setPermanentText(String text) {
+ synchronized (mLock) {
+ mCurrentText = text;
+ // If there's currently a Runnable pending, then we don't alter the display
+ // text. The Runnable will use the most recent version of mCurrentText
+ // when it completes.
+ if (mRunnable == null) {
+ mTextView.setText(text);
+ }
+ }
+ }
+
+ public void setTemporaryText(String text, long duration, TimeUnit units) {
+ synchronized (mLock) {
+ mTextView.setText(text);
+ mRunnable = new Runnable() {
+ @Override
+ public void run() {
+ synchronized (mLock) {
+ // We check for (mRunnable == this) becuase if not true, then another
+ // setTemporaryText call has taken place in the meantime, and this
+ // one is now defunct and needs to take no action.
+ if (mRunnable == this) {
+ mRunnable = null;
+ mTextView.setText(mCurrentText);
+ }
+ }
+ }
+ };
+ mTextView.postDelayed(mRunnable, units.toMillis(duration));
+ }
+ }
+ }
}
diff --git a/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java b/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java
index 53e64e9..5e7b707 100644
--- a/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java
+++ b/src/com/android/contacts/voicemail/VoicemailPlaybackPresenter.java
@@ -16,6 +16,7 @@
package com.android.contacts.voicemail;
+import com.android.contacts.R;
import com.android.ex.variablespeed.MediaPlayerProxy;
import com.android.ex.variablespeed.SingleThreadedMediaPlayerProxy;
@@ -56,8 +57,7 @@
void setPositionSeekListener(SeekBar.OnSeekBarChangeListener listener);
void setSpeakerphoneListener(View.OnClickListener listener);
void setDeleteButtonListener(View.OnClickListener listener);
- void setClipLength(int clipLengthInMillis);
- void setClipPosition(int clipPositionInMillis);
+ void setClipPosition(int clipPositionInMillis, int clipLengthInMillis);
int getDesiredClipPosition();
void playbackStarted();
void playbackStopped();
@@ -65,7 +65,7 @@
boolean isSpeakerPhoneOn();
void setSpeakerPhoneOn(boolean on);
void finish();
- void setRateDisplay(float rate);
+ void setRateDisplay(float rate, int stringResourceId);
void setRateIncreaseButtonListener(View.OnClickListener listener);
void setRateDecreaseButtonListener(View.OnClickListener listener);
}
@@ -89,9 +89,31 @@
private static final float[] PRESET_RATES = new float[] {
0.64f, 0.8f, 1.0f, 1.25f, 1.5625f
};
+ /** The string resource ids corresponding to the names given to the above preset rates. */
+ private static final int[] PRESET_NAMES = new int[] {
+ R.string.voicemail_speed_slowest,
+ R.string.voicemail_speed_slower,
+ R.string.voicemail_speed_normal,
+ R.string.voicemail_speed_faster,
+ R.string.voicemail_speed_fastest,
+ };
+ /**
+ * Pointer into the {@link VoicemailPlaybackPresenter#PRESET_RATES} array.
+ * <p>
+ * This doesn't need to be synchronized, it's used only by the {@link RateChangeListener}
+ * which in turn is only executed on the ui thread. This can't be encapsulated inside the
+ * rate change listener since multiple rate change listeners must share the same value.
+ */
+ private int mRateIndex = 2;
- /** Index into {@link #PRESET_RATES} indicating the current playback speed. */
- private final AtomicInteger mCurrentPlaybackRate = new AtomicInteger(2);
+ /**
+ * The most recently calculated duration.
+ * <p>
+ * We cache this in a field since we don't want to keep requesting it from the player, as
+ * this can easily lead to throwing {@link IllegalStateException} (any time the player is
+ * released, it's illegal to ask for the duration).
+ */
+ private final AtomicInteger mDuration = new AtomicInteger(0);
private final PlaybackView mView;
private final MediaPlayerProxy mPlayer;
@@ -117,7 +139,7 @@
mView.setSpeakerPhoneOn(mView.isSpeakerPhoneOn());
mView.setRateDecreaseButtonListener(createRateDecreaseListener());
mView.setRateIncreaseButtonListener(createRateIncreaseListener());
- mView.setClipPosition(0);
+ mView.setClipPosition(0, 0);
mView.playbackStopped();
// TODO: Now I'm ignoring the bundle, when previously I was checking for contains against
// the PAUSED_STATE_KEY, and CLIP_POSITION_KEY.
@@ -131,6 +153,7 @@
}
public void onDestroy() {
+ mPositionUpdater.stopUpdating();
mPlayer.release();
}
@@ -174,6 +197,11 @@
return new RateChangeListener(true);
}
+ /**
+ * Listens to clicks on the rate increase and decrease buttons.
+ * <p>
+ * This class is not thread-safe, but all interactions with it will happen on the ui thread.
+ */
private class RateChangeListener implements View.OnClickListener {
private final boolean mIncrease;
@@ -183,33 +211,32 @@
@Override
public void onClick(View v) {
- int adjustment = (mIncrease ? 1 : -1);
- int andGet = mCurrentPlaybackRate.addAndGet(adjustment);
- if (andGet < 0) {
- // TODO: discussions with interaction design have suggested that we might make
- // an audible tone play here to indicate that you've hit the end of the range?
- // Let's firm up this decision.
- mCurrentPlaybackRate.set(0);
- } else if (andGet >= PRESET_RATES.length) {
- mCurrentPlaybackRate.set(PRESET_RATES.length - 1);
- } else {
- changeRate(PRESET_RATES[andGet]);
- }
+ // Adjust the current rate, then clamp it to the allowed values.
+ mRateIndex = clamp(mRateIndex + (mIncrease ? 1 : -1), 0, PRESET_RATES.length - 1);
+ // Whether or not we have actually changed the index, call changeRate().
+ // This will ensure that we show the "fastest" or "slowest" text on the ui to indicate
+ // to the user that it doesn't get any faster or slower.
+ changeRate(PRESET_RATES[mRateIndex], PRESET_NAMES[mRateIndex]);
}
}
+ /** Clamp the input value to between min and max inclusive. */
+ private static int clamp(int input, int min, int max) {
+ return Math.max(Math.min(input, max), min);
+ }
+
private void resetPrepareStartPlaying(int clipPositionInMillis) {
try {
mPlayer.reset();
mPlayer.setDataSource(mView.getDataSourceContext(), mVoicemailUri);
mPlayer.prepare();
- int clipLengthInMillis = mPlayer.getDuration();
- mView.setClipLength(clipLengthInMillis);
- int startPosition = Math.min(Math.max(clipPositionInMillis, 0), clipLengthInMillis);
+ mDuration.set(mPlayer.getDuration());
+ int startPosition = clamp(clipPositionInMillis, 0, mDuration.get());
+ mView.setClipPosition(startPosition, mDuration.get());
mPlayer.seekTo(startPosition);
mPlayer.start();
mView.playbackStarted();
- mPositionUpdater.startUpdating(startPosition, clipLengthInMillis);
+ mPositionUpdater.startUpdating(startPosition, mDuration.get());
} catch (IOException e) {
handleError(e);
}
@@ -217,18 +244,18 @@
private void handleError(Exception e) {
mView.playbackError(e);
- mPlayer.release();
mPositionUpdater.stopUpdating();
+ mPlayer.release();
}
public void handleCompletion(MediaPlayer mediaPlayer) {
- stopPlaybackAtPosition(0);
+ stopPlaybackAtPosition(0, mDuration.get());
}
- private void stopPlaybackAtPosition(int clipPosition) {
- mView.playbackStopped();
+ private void stopPlaybackAtPosition(int clipPosition, int duration) {
mPositionUpdater.stopUpdating();
- mView.setClipPosition(clipPosition);
+ mView.playbackStopped();
+ mView.setClipPosition(clipPosition, duration);
if (mPlayer.isPlaying()) {
mPlayer.pause();
}
@@ -241,7 +268,7 @@
public void onStartTrackingTouch(SeekBar arg0) {
if (mPlayer.isPlaying()) {
mShouldResumePlaybackAfterSeeking = true;
- stopPlaybackAtPosition(mPlayer.getCurrentPosition());
+ stopPlaybackAtPosition(mPlayer.getCurrentPosition(), mDuration.get());
} else {
mShouldResumePlaybackAfterSeeking = false;
}
@@ -250,7 +277,7 @@
@Override
public void onStopTrackingTouch(SeekBar arg0) {
if (mPlayer.isPlaying()) {
- stopPlaybackAtPosition(mPlayer.getCurrentPosition());
+ stopPlaybackAtPosition(mPlayer.getCurrentPosition(), mDuration.get());
}
if (mShouldResumePlaybackAfterSeeking) {
resetPrepareStartPlaying(mView.getDesiredClipPosition());
@@ -259,13 +286,13 @@
@Override
public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) {
- mView.setClipPosition(seekBar.getProgress());
+ mView.setClipPosition(seekBar.getProgress(), seekBar.getMax());
}
}
- private void changeRate(float rate) {
+ private void changeRate(float rate, int stringResourceId) {
((SingleThreadedMediaPlayerProxy) mPlayer).setVariableSpeed(rate);
- mView.setRateDisplay(rate);
+ mView.setRateDisplay(rate, stringResourceId);
}
private class SpeakerphoneListener implements View.OnClickListener {
@@ -288,7 +315,7 @@
@Override
public void onClick(View arg0) {
if (mPlayer.isPlaying()) {
- stopPlaybackAtPosition(mPlayer.getCurrentPosition());
+ stopPlaybackAtPosition(mPlayer.getCurrentPosition(), mDuration.get());
} else {
resetPrepareStartPlaying(mView.getDesiredClipPosition());
}
@@ -304,6 +331,12 @@
private final int mPeriodMillis;
private final Object mLock = new Object();
@GuardedBy("mLock") private ScheduledFuture<?> mScheduledFuture;
+ private final Runnable mSetClipPostitionRunnable = new Runnable() {
+ @Override
+ public void run() {
+ mView.setClipPosition(mPlayer.getCurrentPosition(), mDuration.get());
+ }
+ };
public PositionUpdater(ScheduledExecutorService executorService, int periodMillis) {
mExecutorService = executorService;
@@ -314,12 +347,7 @@
public void run() {
synchronized (mLock) {
if (mScheduledFuture != null) {
- mView.runOnUiThread(new Runnable() {
- @Override
- public void run() {
- mView.setClipPosition(mPlayer.getCurrentPosition());
- }
- });
+ mView.runOnUiThread(mSetClipPostitionRunnable);
}
}
}
diff --git a/tests/src/com/android/contacts/CallDetailActivityTest.java b/tests/src/com/android/contacts/CallDetailActivityTest.java
index c279860..c1efa3f 100644
--- a/tests/src/com/android/contacts/CallDetailActivityTest.java
+++ b/tests/src/com/android/contacts/CallDetailActivityTest.java
@@ -29,13 +29,17 @@
import android.net.Uri;
import android.provider.CallLog;
import android.test.ActivityInstrumentationTestCase2;
+import android.test.suitebuilder.annotation.LargeTest;
import android.view.Menu;
+import android.widget.TextView;
+import java.util.List;
import java.util.Locale;
/**
* Unit tests for the {@link CallDetailActivity}.
*/
+@LargeTest
public class CallDetailActivityTest extends ActivityInstrumentationTestCase2<CallDetailActivity> {
private static final String FAKE_VOICEMAIL_URI_STRING = "content://fake_uri";
private Uri mUri;
@@ -112,6 +116,29 @@
assertTrue(menu.findItem(R.id.remove_from_call_log).isVisible());
}
+ /**
+ * Test to show that we are correctly displaying playback rate on the ui.
+ * <p>
+ * See bug http://b/5044075.
+ */
+ public void testVoicemailPlaybackRateDisplayedOnUi() throws Throwable {
+ setActivityIntentForTestVoicemailEntry();
+ CallDetailActivity activity = getActivity();
+ // Find the TextView containing the duration. It should be initially displaying "00:00".
+ List<TextView> views = mTestUtils.getTextViewsWithString(activity, "00:00");
+ assertEquals(1, views.size());
+ TextView timeDisplay = views.get(0);
+ // Hit the plus button. At this point we should be displaying "fast speed".
+ mTestUtils.clickButton(activity, R.id.rate_increase_button);
+ assertEquals("fast speed", mTestUtils.getText(timeDisplay));
+ // Hit the minus button. We should be back to "normal" speed.
+ mTestUtils.clickButton(activity, R.id.rate_decrease_button);
+ assertEquals("normal speed", mTestUtils.getText(timeDisplay));
+ // Wait for one and a half seconds. The timer will be back.
+ Thread.sleep(1500);
+ assertEquals("00:00", mTestUtils.getText(timeDisplay));
+ }
+
private void setActivityIntentForTestCallEntry() {
createTestCallEntry(false);
setActivityIntent(new Intent(Intent.ACTION_VIEW, mUri));
diff --git a/tests/src/com/android/contacts/util/IntegrationTestUtils.java b/tests/src/com/android/contacts/util/IntegrationTestUtils.java
index a61ea57..afea349 100644
--- a/tests/src/com/android/contacts/util/IntegrationTestUtils.java
+++ b/tests/src/com/android/contacts/util/IntegrationTestUtils.java
@@ -72,6 +72,16 @@
});
}
+ /** Returns the result of running {@link TextView#getText()} on the ui thread. */
+ public CharSequence getText(final TextView view) throws Throwable {
+ return runOnUiThreadAndGetTheResult(new Callable<CharSequence>() {
+ @Override
+ public CharSequence call() {
+ return view.getText();
+ }
+ });
+ }
+
// TODO: Move this class and the appropriate documentation into a test library, having checked
// first to see if exactly this code already exists or not.
/**