From de08eaf43748255d1e778e6648dc442fb6202a12 Mon Sep 17 00:00:00 2001 From: Doris Ling Date: Wed, 4 Apr 2018 17:35:41 -0700 Subject: [PATCH] Fix issue in ring volume sample not always being played. - when we adjust the sound volme in Sound settings, we only re-post the stop sample message when we receive the onSampleStarting callback. However, if we change the volume while a sample is still playing, onSampleStarting will not be called as it's already started. This results in shortened sample duration, which in extreme case, the new sample will not be played at all if the new volume change is made almost towards the end of the previous sample period. So, everytime user change the volume, we should re-post the stop sample message, so that the sample playing duration would be extended properly. - also removed the original calls to the onStreamValueChanged() during init, as the original implementation is empty, and during init, we do not need any handling to start/stop the sample. Change-Id: I9f35ddfb6d809eeb83b1a732a09362286ff6ed77 Fixes: 77514234 Test: make RunSettingsRoboTests --- .../settings/notification/SoundSettings.java | 48 +++++++++---------- .../notification/VolumeSeekBarPreference.java | 10 ---- .../settings/widget/SeekBarPreference.java | 3 +- .../notification/SoundSettingsTest.java | 19 +++++++- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/src/com/android/settings/notification/SoundSettings.java b/src/com/android/settings/notification/SoundSettings.java index 9550758656b..a7ba7072c25 100644 --- a/src/com/android/settings/notification/SoundSettings.java +++ b/src/com/android/settings/notification/SoundSettings.java @@ -25,6 +25,7 @@ import android.os.Message; import android.os.UserHandle; import android.preference.SeekBarVolumizer; import android.provider.SearchIndexableResource; +import android.support.annotation.VisibleForTesting; import android.support.v7.preference.Preference; import android.text.TextUtils; @@ -50,8 +51,22 @@ public class SoundSettings extends DashboardFragment { private static final int SAMPLE_CUTOFF = 2000; // manually cap sample playback at 2 seconds - private final VolumePreferenceCallback mVolumeCallback = new VolumePreferenceCallback(); - private final H mHandler = new H(); + @VisibleForTesting + static final int STOP_SAMPLE = 1; + + @VisibleForTesting + final VolumePreferenceCallback mVolumeCallback = new VolumePreferenceCallback(); + @VisibleForTesting + final Handler mHandler = new Handler(Looper.getMainLooper()) { + @Override + public void handleMessage(Message msg) { + switch (msg.what) { + case STOP_SAMPLE: + mVolumeCallback.stopSample(); + break; + } + } + }; private RingtonePreference mRequestPreference; @@ -140,14 +155,17 @@ public class SoundSettings extends DashboardFragment { } mCurrent = sbv; if (mCurrent != null) { - mHandler.removeMessages(H.STOP_SAMPLE); - mHandler.sendEmptyMessageDelayed(H.STOP_SAMPLE, SAMPLE_CUTOFF); + mHandler.removeMessages(STOP_SAMPLE); + mHandler.sendEmptyMessageDelayed(STOP_SAMPLE, SAMPLE_CUTOFF); } } @Override public void onStreamValueChanged(int stream, int progress) { - // noop + if (mCurrent != null) { + mHandler.removeMessages(STOP_SAMPLE); + mHandler.sendEmptyMessageDelayed(STOP_SAMPLE, SAMPLE_CUTOFF); + } } public void stopSample() { @@ -157,26 +175,6 @@ public class SoundSettings extends DashboardFragment { } } - // === Callbacks === - - - private final class H extends Handler { - private static final int STOP_SAMPLE = 1; - - private H() { - super(Looper.getMainLooper()); - } - - @Override - public void handleMessage(Message msg) { - switch (msg.what) { - case STOP_SAMPLE: - mVolumeCallback.stopSample(); - break; - } - } - } - private static List buildPreferenceControllers(Context context, SoundSettings fragment, VolumeSeekBarPreference.Callback callback, Lifecycle lifecycle) { diff --git a/src/com/android/settings/notification/VolumeSeekBarPreference.java b/src/com/android/settings/notification/VolumeSeekBarPreference.java index 478a7d6b3a0..8a48e95c166 100644 --- a/src/com/android/settings/notification/VolumeSeekBarPreference.java +++ b/src/com/android/settings/notification/VolumeSeekBarPreference.java @@ -148,7 +148,6 @@ public class VolumeSeekBarPreference extends SeekBarPreference { mVolumizer.start(); mVolumizer.setSeekBar(mSeekBar); updateIconView(); - mCallback.onStreamValueChanged(mStream, mSeekBar.getProgress()); updateSuppressionText(); if (!isEnabled()) { mSeekBar.setEnabled(false); @@ -156,15 +155,6 @@ public class VolumeSeekBarPreference extends SeekBarPreference { } } - // during initialization, this preference is the SeekBar listener - @Override - public void onProgressChanged(SeekBar seekBar, int progress, boolean fromTouch) { - super.onProgressChanged(seekBar, progress, fromTouch); - if (mCallback != null) { - mCallback.onStreamValueChanged(mStream, progress); - } - } - private void updateIconView() { if (mIconView == null) return; if (mIconResId != 0) { diff --git a/src/com/android/settings/widget/SeekBarPreference.java b/src/com/android/settings/widget/SeekBarPreference.java index 5af21b3a004..1ad4a200512 100644 --- a/src/com/android/settings/widget/SeekBarPreference.java +++ b/src/com/android/settings/widget/SeekBarPreference.java @@ -233,8 +233,7 @@ public class SeekBarPreference extends RestrictedPreference } @Override - public void onProgressChanged( - SeekBar seekBar, int progress, boolean fromUser) { + public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) { if (fromUser && (mContinuousUpdates || !mTrackingTouch)) { syncProgress(seekBar); } diff --git a/tests/robotests/src/com/android/settings/notification/SoundSettingsTest.java b/tests/robotests/src/com/android/settings/notification/SoundSettingsTest.java index 7d192e7bfcc..4eef4b626fb 100644 --- a/tests/robotests/src/com/android/settings/notification/SoundSettingsTest.java +++ b/tests/robotests/src/com/android/settings/notification/SoundSettingsTest.java @@ -24,8 +24,10 @@ import static org.mockito.Mockito.when; import android.content.Context; import android.media.AudioManager; +import android.os.Handler; import android.os.UserManager; +import android.preference.SeekBarVolumizer; import com.android.settings.R; import com.android.settings.testutils.SettingsRobolectricTestRunner; import com.android.settings.testutils.XmlTestUtils; @@ -36,6 +38,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; +import org.robolectric.util.ReflectionHelpers; import java.util.List; @@ -44,7 +47,7 @@ public class SoundSettingsTest { @Test @Config(shadows = {ShadowUserManager.class, ShadowAudioHelper.class}) - public void testNonIndexableKeys_existInXmlLayout() { + public void getNonIndexableKeys_existInXmlLayout() { final Context context = spy(RuntimeEnvironment.application); AudioManager audioManager = mock(AudioManager.class); doReturn(audioManager).when(context).getSystemService(Context.AUDIO_SERVICE); @@ -66,4 +69,16 @@ public class SoundSettingsTest { assertThat(keys).containsAllIn(niks); } -} \ No newline at end of file + + @Test + public void onStreamValueChanged_shouldRepostStopSampleMessage() { + final SoundSettings settings = new SoundSettings(); + final Handler handler = settings.mHandler; + ReflectionHelpers.setField( + settings.mVolumeCallback, "mCurrent", mock(SeekBarVolumizer.class)); + + settings.mVolumeCallback.onStreamValueChanged(0, 5); + + assertThat(settings.mHandler.hasMessages(SoundSettings.STOP_SAMPLE)).isTrue(); + } +}