Fix volume panel hang

SliceLiveData changed its behavior, if slice is null, it will not
notify LiveData observer but callback to onErrorListener. We only
reduce PanelSlicesLoaderCountdownLatch in LiveData observer.
Therefore the error slice caused PanelSlicesLoaderCountdownLatch
never count to 0, the UI was not displayed. It is solved by reducing
PanelSlicesLoaderCountdownLatch in onErrorListener and also not return
null in MediaOutputIndicatorSlice.

Test: manual
Fixes: 141084035
Change-Id: Iddb2dbdc0e0d2ac3e26071960bb667937f181121
This commit is contained in:
Raff Tsai
2019-09-17 23:47:07 +08:00
parent a9177ca0eb
commit d9cfbaf3b5
6 changed files with 67 additions and 41 deletions

View File

@@ -65,7 +65,9 @@ public class MediaOutputIndicatorSlice implements CustomSliceable {
@Override @Override
public Slice getSlice() { public Slice getSlice() {
if (!isVisible()) { if (!isVisible()) {
return null; return new ListBuilder(mContext, MEDIA_OUTPUT_INDICATOR_SLICE_URI, ListBuilder.INFINITY)
.setIsError(true)
.build();
} }
final IconCompat icon = IconCompat.createWithResource(mContext, final IconCompat icon = IconCompat.createWithResource(mContext,
com.android.internal.R.drawable.ic_settings_bluetooth); com.android.internal.R.drawable.ic_settings_bluetooth);

View File

@@ -53,9 +53,10 @@ import com.android.settingslib.core.instrumentation.MetricsFeatureProvider;
import com.google.android.setupdesign.DividerItemDecoration; import com.google.android.setupdesign.DividerItemDecoration;
import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map;
public class PanelFragment extends Fragment { public class PanelFragment extends Fragment {
@@ -86,7 +87,7 @@ public class PanelFragment extends Fragment {
private MetricsFeatureProvider mMetricsProvider; private MetricsFeatureProvider mMetricsProvider;
private String mPanelClosedKey; private String mPanelClosedKey;
private final List<LiveData<Slice>> mSliceLiveData = new ArrayList<>(); private final Map<Uri, LiveData<Slice>> mSliceLiveData = new LinkedHashMap<>();
@VisibleForTesting @VisibleForTesting
PanelSlicesLoaderCountdownLatch mPanelSlicesLoaderCountdownLatch; PanelSlicesLoaderCountdownLatch mPanelSlicesLoaderCountdownLatch;
@@ -97,14 +98,14 @@ public class PanelFragment extends Fragment {
private final ViewTreeObserver.OnGlobalLayoutListener mOnGlobalLayoutListener = private final ViewTreeObserver.OnGlobalLayoutListener mOnGlobalLayoutListener =
new ViewTreeObserver.OnGlobalLayoutListener() { new ViewTreeObserver.OnGlobalLayoutListener() {
@Override @Override
public void onGlobalLayout() { public void onGlobalLayout() {
animateIn(); animateIn();
if (mPanelSlices != null) { if (mPanelSlices != null) {
mPanelSlices.getViewTreeObserver().removeOnGlobalLayoutListener(this); mPanelSlices.getViewTreeObserver().removeOnGlobalLayoutListener(this);
} }
} }
}; };
private PanelSlicesAdapter mAdapter; private PanelSlicesAdapter mAdapter;
@@ -121,9 +122,9 @@ public class PanelFragment extends Fragment {
* Animate the old panel out from the screen, then update the panel with new content once the * Animate the old panel out from the screen, then update the panel with new content once the
* animation is done. * animation is done.
* <p> * <p>
* Takes the entire panel and animates out from behind the navigation bar. * Takes the entire panel and animates out from behind the navigation bar.
* <p> * <p>
* Call createPanelContent() once animation end. * Call createPanelContent() once animation end.
*/ */
void updatePanelWithAnimation() { void updatePanelWithAnimation() {
final View panelContent = mLayoutView.findViewById(R.id.panel_container); final View panelContent = mLayoutView.findViewById(R.id.panel_container);
@@ -210,10 +211,14 @@ public class PanelFragment extends Fragment {
mPanelSlicesLoaderCountdownLatch = new PanelSlicesLoaderCountdownLatch(sliceUris.size()); mPanelSlicesLoaderCountdownLatch = new PanelSlicesLoaderCountdownLatch(sliceUris.size());
for (Uri uri : sliceUris) { for (Uri uri : sliceUris) {
final LiveData<Slice> sliceLiveData = SliceLiveData.fromUri(getActivity(), uri); final LiveData<Slice> sliceLiveData = SliceLiveData.fromUri(getActivity(), uri,
(int type, Throwable source)-> {
removeSliceLiveData(uri);
mPanelSlicesLoaderCountdownLatch.markSliceLoaded(uri);
});
// Add slice first to make it in order. Will remove it later if there's an error. // Add slice first to make it in order. Will remove it later if there's an error.
mSliceLiveData.add(sliceLiveData); mSliceLiveData.put(uri, sliceLiveData);
sliceLiveData.observe(getViewLifecycleOwner(), slice -> { sliceLiveData.observe(getViewLifecycleOwner(), slice -> {
// If the Slice has already loaded, do nothing. // If the Slice has already loaded, do nothing.
@@ -238,12 +243,7 @@ public class PanelFragment extends Fragment {
*/ */
final SliceMetadata metadata = SliceMetadata.from(getActivity(), slice); final SliceMetadata metadata = SliceMetadata.from(getActivity(), slice);
if (slice == null || metadata.isErrorSlice()) { if (slice == null || metadata.isErrorSlice()) {
final List<String> whiteList = Arrays.asList( removeSliceLiveData(uri);
getResources().getStringArray(
R.array.config_panel_keep_observe_uri));
if (!whiteList.contains(uri.toString())) {
mSliceLiveData.remove(sliceLiveData);
}
mPanelSlicesLoaderCountdownLatch.markSliceLoaded(uri); mPanelSlicesLoaderCountdownLatch.markSliceLoaded(uri);
} else if (metadata.getLoadingState() == SliceMetadata.LOADED_ALL) { } else if (metadata.getLoadingState() == SliceMetadata.LOADED_ALL) {
mPanelSlicesLoaderCountdownLatch.markSliceLoaded(uri); mPanelSlicesLoaderCountdownLatch.markSliceLoaded(uri);
@@ -260,12 +260,21 @@ public class PanelFragment extends Fragment {
} }
} }
private void removeSliceLiveData(Uri uri) {
final List<String> whiteList = Arrays.asList(
getResources().getStringArray(
R.array.config_panel_keep_observe_uri));
if (!whiteList.contains(uri.toString())) {
mSliceLiveData.remove(uri);
}
}
/** /**
* When all of the Slices have loaded for the first time, then we can setup the * When all of the Slices have loaded for the first time, then we can setup the
* {@link RecyclerView}. * {@link RecyclerView}.
* <p> * <p>
* When the Recyclerview has been laid out, we can begin the animation with the * When the Recyclerview has been laid out, we can begin the animation with the
* {@link mOnGlobalLayoutListener}, which calls {@link #animateIn()}. * {@link mOnGlobalLayoutListener}, which calls {@link #animateIn()}.
*/ */
private void loadPanelWhenReady() { private void loadPanelWhenReady() {
if (mPanelSlicesLoaderCountdownLatch.isPanelReadyToLoad()) { if (mPanelSlicesLoaderCountdownLatch.isPanelReadyToLoad()) {
@@ -286,9 +295,9 @@ public class PanelFragment extends Fragment {
/** /**
* Animate a Panel onto the screen. * Animate a Panel onto the screen.
* <p> * <p>
* Takes the entire panel and animates in from behind the navigation bar. * Takes the entire panel and animates in from behind the navigation bar.
* <p> * <p>
* Relies on the Panel being having a fixed height to begin the animation. * Relies on the Panel being having a fixed height to begin the animation.
*/ */
private void animateIn() { private void animateIn() {
final View panelContent = mLayoutView.findViewById(R.id.panel_container); final View panelContent = mLayoutView.findViewById(R.id.panel_container);
@@ -319,7 +328,7 @@ public class PanelFragment extends Fragment {
animatorSet.setInterpolator(new DecelerateInterpolator()); animatorSet.setInterpolator(new DecelerateInterpolator());
animatorSet.playTogether( animatorSet.playTogether(
ObjectAnimator.ofFloat(sheet, View.TRANSLATION_Y, startY, endY), ObjectAnimator.ofFloat(sheet, View.TRANSLATION_Y, startY, endY),
ObjectAnimator.ofFloat(sheet, View.ALPHA, startAlpha,endAlpha)); ObjectAnimator.ofFloat(sheet, View.ALPHA, startAlpha, endAlpha));
return animatorSet; return animatorSet;
} }

View File

@@ -20,6 +20,7 @@ import static com.android.settings.slices.CustomSliceRegistry.MEDIA_OUTPUT_INDIC
import android.app.settings.SettingsEnums; import android.app.settings.SettingsEnums;
import android.content.Context; import android.content.Context;
import android.net.Uri;
import android.view.LayoutInflater; import android.view.LayoutInflater;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
@@ -38,6 +39,7 @@ import com.google.android.setupdesign.DividerItemDecoration;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Map;
/** /**
* RecyclerView adapter for Slices in Settings Panels. * RecyclerView adapter for Slices in Settings Panels.
@@ -56,9 +58,9 @@ public class PanelSlicesAdapter
private final PanelFragment mPanelFragment; private final PanelFragment mPanelFragment;
public PanelSlicesAdapter( public PanelSlicesAdapter(
PanelFragment fragment, List<LiveData<Slice>> sliceLiveData, int metricsCategory) { PanelFragment fragment, Map<Uri, LiveData<Slice>> sliceLiveData, int metricsCategory) {
mPanelFragment = fragment; mPanelFragment = fragment;
mSliceLiveData = new ArrayList<>(sliceLiveData); mSliceLiveData = new ArrayList<>(sliceLiveData.values());
mMetricsCategory = metricsCategory; mMetricsCategory = metricsCategory;
} }

View File

@@ -17,6 +17,8 @@
package com.android.settings.media; package com.android.settings.media;
import static android.app.slice.Slice.HINT_ERROR;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
@@ -110,11 +112,13 @@ public class MediaOutputIndicatorSliceTest {
} }
@Test @Test
public void getSlice_noConnectedDevice_returnNull() { public void getSlice_noConnectedDevice_returnErrorSlice() {
mDevicesList.clear(); mDevicesList.clear();
when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList); when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList);
assertThat(mMediaOutputIndicatorSlice.getSlice()).isNull(); final Slice mediaSlice = mMediaOutputIndicatorSlice.getSlice();
final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice);
assertThat(metadata.isErrorSlice()).isTrue();
} }
@Test @Test
@@ -129,6 +133,7 @@ public class MediaOutputIndicatorSliceTest {
assertThat(metadata.getTitle()).isEqualTo(mContext.getText(R.string.media_output_title)); assertThat(metadata.getTitle()).isEqualTo(mContext.getText(R.string.media_output_title));
assertThat(metadata.getSubtitle()).isEqualTo(mContext.getText( assertThat(metadata.getSubtitle()).isEqualTo(mContext.getText(
R.string.media_output_default_summary)); R.string.media_output_default_summary));
assertThat(metadata.isErrorSlice()).isFalse();
} }
@Test @Test
@@ -141,6 +146,7 @@ public class MediaOutputIndicatorSliceTest {
final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice); final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice);
assertThat(metadata.getTitle()).isEqualTo(mContext.getText(R.string.media_output_title)); assertThat(metadata.getTitle()).isEqualTo(mContext.getText(R.string.media_output_title));
assertThat(metadata.getSubtitle()).isEqualTo(TEST_A2DP_DEVICE_NAME); assertThat(metadata.getSubtitle()).isEqualTo(TEST_A2DP_DEVICE_NAME);
assertThat(metadata.isErrorSlice()).isFalse();
} }
@Test @Test
@@ -154,32 +160,39 @@ public class MediaOutputIndicatorSliceTest {
final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice); final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice);
assertThat(metadata.getTitle()).isEqualTo(mContext.getText(R.string.media_output_title)); assertThat(metadata.getTitle()).isEqualTo(mContext.getText(R.string.media_output_title));
assertThat(metadata.getSubtitle()).isEqualTo(TEST_HAP_DEVICE_NAME); assertThat(metadata.getSubtitle()).isEqualTo(TEST_HAP_DEVICE_NAME);
assertThat(metadata.isErrorSlice()).isFalse();
} }
@Test @Test
public void getSlice_audioModeIsInCommunication_returnNull() { public void getSlice_audioModeIsInCommunication_returnErrorSlice() {
mDevicesList.add(mA2dpDevice); mDevicesList.add(mA2dpDevice);
when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList); when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList);
mAudioManager.setMode(AudioManager.MODE_IN_COMMUNICATION); mAudioManager.setMode(AudioManager.MODE_IN_COMMUNICATION);
assertThat(mMediaOutputIndicatorSlice.getSlice()).isNull(); final Slice mediaSlice = mMediaOutputIndicatorSlice.getSlice();
final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice);
assertThat(metadata.isErrorSlice()).isTrue();
} }
@Test @Test
public void getSlice_audioModeIsRingtone_returnNull() { public void getSlice_audioModeIsRingtone_returnErrorSlice() {
mDevicesList.add(mA2dpDevice); mDevicesList.add(mA2dpDevice);
when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList); when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList);
mAudioManager.setMode(AudioManager.MODE_RINGTONE); mAudioManager.setMode(AudioManager.MODE_RINGTONE);
assertThat(mMediaOutputIndicatorSlice.getSlice()).isNull(); final Slice mediaSlice = mMediaOutputIndicatorSlice.getSlice();
final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice);
assertThat(metadata.isErrorSlice()).isTrue();
} }
@Test @Test
public void getSlice_audioModeIsInCall_returnNull() { public void getSlice_audioModeIsInCall_returnErrorSlice() {
mDevicesList.add(mA2dpDevice); mDevicesList.add(mA2dpDevice);
when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList); when(mA2dpProfile.getConnectedDevices()).thenReturn(mDevicesList);
mAudioManager.setMode(AudioManager.MODE_IN_CALL); mAudioManager.setMode(AudioManager.MODE_IN_CALL);
assertThat(mMediaOutputIndicatorSlice.getSlice()).isNull(); final Slice mediaSlice = mMediaOutputIndicatorSlice.getSlice();
final SliceMetadata metadata = SliceMetadata.from(mContext, mediaSlice);
assertThat(metadata.isErrorSlice()).isTrue();
} }
} }

View File

@@ -49,7 +49,9 @@ import org.robolectric.RuntimeEnvironment;
import org.robolectric.android.controller.ActivityController; import org.robolectric.android.controller.ActivityController;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map;
@RunWith(RobolectricTestRunner.class) @RunWith(RobolectricTestRunner.class)
public class PanelSlicesAdapterTest { public class PanelSlicesAdapterTest {
@@ -61,7 +63,7 @@ public class PanelSlicesAdapterTest {
private PanelFeatureProvider mPanelFeatureProvider; private PanelFeatureProvider mPanelFeatureProvider;
private FakeFeatureFactory mFakeFeatureFactory; private FakeFeatureFactory mFakeFeatureFactory;
private FakePanelContent mFakePanelContent; private FakePanelContent mFakePanelContent;
private List<LiveData<Slice>> mData = new ArrayList<>(); private Map<Uri, LiveData<Slice>> mData = new LinkedHashMap<>();
@Before @Before
public void setUp() { public void setUp() {
@@ -93,7 +95,7 @@ public class PanelSlicesAdapterTest {
doReturn(uri).when(slice).getUri(); doReturn(uri).when(slice).getUri();
final LiveData<Slice> liveData = mock(LiveData.class); final LiveData<Slice> liveData = mock(LiveData.class);
when(liveData.getValue()).thenReturn(slice); when(liveData.getValue()).thenReturn(slice);
mData.add(liveData); mData.put(uri, liveData);
} }
@Test @Test
@@ -111,7 +113,7 @@ public class PanelSlicesAdapterTest {
@Test @Test
public void sizeOfAdapter_shouldNotExceedMaxNum() { public void sizeOfAdapter_shouldNotExceedMaxNum() {
for (int i = 0; i < MAX_NUM_OF_SLICES + 2; i++) { for (int i = 0; i < MAX_NUM_OF_SLICES + 2; i++) {
addTestLiveData(DATA_URI); addTestLiveData(Uri.parse("uri" + i));
} }
assertThat(mData.size()).isEqualTo(MAX_NUM_OF_SLICES + 2); assertThat(mData.size()).isEqualTo(MAX_NUM_OF_SLICES + 2);

View File

@@ -187,8 +187,6 @@ public class OwnerInfoPreferenceControllerTest {
preference.performClick(); preference.performClick();
// Called once in setTargetFragment, and a second time to display the fragment.
verify(mFragment, times(2)).getFragmentManager();
verify(mFragment.getFragmentManager().beginTransaction()) verify(mFragment.getFragmentManager().beginTransaction())
.add(any(OwnerInfoSettings.class), anyString()); .add(any(OwnerInfoSettings.class), anyString());
} }