Fix the endless panel loading

Re-launching volume panel continuously will trigger an endless panel
loading, show a transparent unfinished UI, and then block the user's
screen.

Root cause:
When the activity receives a new intent from user's clicking, it will
call PanelFragment#createPanelContent to update the current fragment.
The method triggers an animation and then loads the panel content. If
multiple invocations run concurrently before the animation or the
loading finish, the loader's countdown latch will be increased
abnormally and lead to the endless loading.

Solution:
1. Since the invocations are in UI thread, simply add a flag to avoid
reentrance when the panel is animating or loading.
2. Filter out the same panel's creation request when the panel is still
visible.
3. Do not force a panel's recreation when it's under construction.

Fixes: 143889510
Fixes: 160491854
Test: robotest, manual
Change-Id: I821faedeb62354929f3af9804cbbe44ee5bb8a53
Merged-In: I821faedeb62354929f3af9804cbbe44ee5bb8a53
(cherry picked from commit 6a8d2c5e55)
This commit is contained in:
Jason Chiu
2020-08-06 16:05:33 +08:00
parent ca04ccd911
commit 7db71ac87a
3 changed files with 100 additions and 10 deletions

View File

@@ -97,6 +97,7 @@ public class PanelFragment extends Fragment {
private TextView mHeaderSubtitle; private TextView mHeaderSubtitle;
private int mMaxHeight; private int mMaxHeight;
private View mFooterDivider; private View mFooterDivider;
private boolean mPanelCreating;
private final Map<Uri, LiveData<Slice>> mSliceLiveData = new LinkedHashMap<>(); private final Map<Uri, LiveData<Slice>> mSliceLiveData = new LinkedHashMap<>();
@@ -127,6 +128,7 @@ public class PanelFragment extends Fragment {
if (mPanelSlices != null) { if (mPanelSlices != null) {
mPanelSlices.getViewTreeObserver().removeOnGlobalLayoutListener(this); mPanelSlices.getViewTreeObserver().removeOnGlobalLayoutListener(this);
} }
mPanelCreating = false;
} }
}; };
@@ -140,6 +142,7 @@ public class PanelFragment extends Fragment {
mLayoutView.getViewTreeObserver() mLayoutView.getViewTreeObserver()
.addOnGlobalLayoutListener(mPanelLayoutListener); .addOnGlobalLayoutListener(mPanelLayoutListener);
mMaxHeight = getResources().getDimensionPixelSize(R.dimen.output_switcher_slice_max_height); mMaxHeight = getResources().getDimensionPixelSize(R.dimen.output_switcher_slice_max_height);
mPanelCreating = true;
createPanelContent(); createPanelContent();
return mLayoutView; return mLayoutView;
} }
@@ -153,6 +156,7 @@ public class PanelFragment extends Fragment {
* Call createPanelContent() once animation end. * Call createPanelContent() once animation end.
*/ */
void updatePanelWithAnimation() { void updatePanelWithAnimation() {
mPanelCreating = true;
final View panelContent = mLayoutView.findViewById(R.id.panel_container); final View panelContent = mLayoutView.findViewById(R.id.panel_container);
final AnimatorSet animatorSet = buildAnimatorSet(mLayoutView, final AnimatorSet animatorSet = buildAnimatorSet(mLayoutView,
0.0f /* startY */, panelContent.getHeight() /* endY */, 0.0f /* startY */, panelContent.getHeight() /* endY */,
@@ -171,11 +175,16 @@ public class PanelFragment extends Fragment {
animatorSet.start(); animatorSet.start();
} }
boolean isPanelCreating() {
return mPanelCreating;
}
private void createPanelContent() { private void createPanelContent() {
final FragmentActivity activity = getActivity(); final FragmentActivity activity = getActivity();
if (mLayoutView == null) { if (mLayoutView == null) {
activity.finish(); activity.finish();
} }
final ViewGroup.LayoutParams params = mLayoutView.getLayoutParams(); final ViewGroup.LayoutParams params = mLayoutView.getLayoutParams();
params.height = ViewGroup.LayoutParams.WRAP_CONTENT; params.height = ViewGroup.LayoutParams.WRAP_CONTENT;
mLayoutView.setLayoutParams(params); mLayoutView.setLayoutParams(params);

View File

@@ -21,6 +21,7 @@ import static com.android.settingslib.media.MediaOutputSliceConstants.EXTRA_PACK
import android.content.Intent; import android.content.Intent;
import android.content.res.Configuration; import android.content.res.Configuration;
import android.os.Bundle; import android.os.Bundle;
import android.text.TextUtils;
import android.util.Log; import android.util.Log;
import android.view.Gravity; import android.view.Gravity;
import android.view.Window; import android.view.Window;
@@ -41,12 +42,14 @@ import com.android.settings.core.HideNonSystemOverlayMixin;
*/ */
public class SettingsPanelActivity extends FragmentActivity { public class SettingsPanelActivity extends FragmentActivity {
private final String TAG = "panel_activity"; private static final String TAG = "SettingsPanelActivity";
@VisibleForTesting @VisibleForTesting
final Bundle mBundle = new Bundle(); final Bundle mBundle = new Bundle();
@VisibleForTesting @VisibleForTesting
boolean mForceCreation = false; boolean mForceCreation = false;
@VisibleForTesting
PanelFragment mPanelFragment;
/** /**
* Key specifying which Panel the app is requesting. * Key specifying which Panel the app is requesting.
@@ -87,7 +90,9 @@ public class SettingsPanelActivity extends FragmentActivity {
@Override @Override
protected void onStop() { protected void onStop() {
super.onStop(); super.onStop();
mForceCreation = true; if (mPanelFragment != null && !mPanelFragment.isPanelCreating()) {
mForceCreation = true;
}
} }
@Override @Override
@@ -104,10 +109,10 @@ public class SettingsPanelActivity extends FragmentActivity {
return; return;
} }
final String action = callingIntent.getAction();
// We will use it once media output switch panel support remote device. // We will use it once media output switch panel support remote device.
final String mediaPackageName = callingIntent.getStringExtra(EXTRA_PACKAGE_NAME); final String mediaPackageName = callingIntent.getStringExtra(EXTRA_PACKAGE_NAME);
mBundle.putString(KEY_PANEL_TYPE_ARGUMENT, action);
mBundle.putString(KEY_PANEL_TYPE_ARGUMENT, callingIntent.getAction());
mBundle.putString(KEY_CALLING_PACKAGE_NAME, getCallingPackage()); mBundle.putString(KEY_CALLING_PACKAGE_NAME, getCallingPackage());
mBundle.putString(KEY_MEDIA_PACKAGE_NAME, mediaPackageName); mBundle.putString(KEY_MEDIA_PACKAGE_NAME, mediaPackageName);
@@ -116,9 +121,21 @@ public class SettingsPanelActivity extends FragmentActivity {
// If fragment already exists and visible, we will need to update panel with animation. // If fragment already exists and visible, we will need to update panel with animation.
if (!shouldForceCreation && fragment != null && fragment instanceof PanelFragment) { if (!shouldForceCreation && fragment != null && fragment instanceof PanelFragment) {
final PanelFragment panelFragment = (PanelFragment) fragment; mPanelFragment = (PanelFragment) fragment;
panelFragment.setArguments(mBundle); if (mPanelFragment.isPanelCreating()) {
panelFragment.updatePanelWithAnimation(); Log.w(TAG, "A panel is creating, skip " + action);
return;
}
final Bundle bundle = fragment.getArguments();
if (bundle != null
&& TextUtils.equals(action, bundle.getString(KEY_PANEL_TYPE_ARGUMENT))) {
Log.w(TAG, "Panel is showing the same action, skip " + action);
return;
}
mPanelFragment.setArguments(new Bundle(mBundle));
mPanelFragment.updatePanelWithAnimation();
} else { } else {
setContentView(R.layout.settings_panel); setContentView(R.layout.settings_panel);
@@ -127,9 +144,9 @@ public class SettingsPanelActivity extends FragmentActivity {
window.setGravity(Gravity.BOTTOM); window.setGravity(Gravity.BOTTOM);
window.setLayout(WindowManager.LayoutParams.MATCH_PARENT, window.setLayout(WindowManager.LayoutParams.MATCH_PARENT,
WindowManager.LayoutParams.WRAP_CONTENT); WindowManager.LayoutParams.WRAP_CONTENT);
final PanelFragment panelFragment = new PanelFragment(); mPanelFragment = new PanelFragment();
panelFragment.setArguments(mBundle); mPanelFragment.setArguments(new Bundle(mBundle));
fragmentManager.beginTransaction().add(R.id.main_content, panelFragment).commit(); fragmentManager.beginTransaction().add(R.id.main_content, mPanelFragment).commit();
} }
} }
} }

View File

@@ -26,6 +26,7 @@ import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
@@ -36,6 +37,9 @@ import android.os.Build;
import android.view.Window; import android.view.Window;
import android.view.WindowManager; import android.view.WindowManager;
import androidx.fragment.app.FragmentManager;
import com.android.settings.R;
import com.android.settings.core.HideNonSystemOverlayMixin; import com.android.settings.core.HideNonSystemOverlayMixin;
import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.FakeFeatureFactory;
@@ -43,6 +47,7 @@ import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations; import org.mockito.MockitoAnnotations;
import org.robolectric.Robolectric; import org.robolectric.Robolectric;
import org.robolectric.RobolectricTestRunner; import org.robolectric.RobolectricTestRunner;
@@ -56,6 +61,10 @@ public class SettingsPanelActivityTest {
private FakeSettingsPanelActivity mSettingsPanelActivity; private FakeSettingsPanelActivity mSettingsPanelActivity;
private PanelFeatureProvider mPanelFeatureProvider; private PanelFeatureProvider mPanelFeatureProvider;
private FakePanelContent mFakePanelContent; private FakePanelContent mFakePanelContent;
@Mock
private PanelFragment mPanelFragment;
@Mock
private FragmentManager mFragmentManager;
@Before @Before
public void setUp() { public void setUp() {
@@ -67,6 +76,10 @@ public class SettingsPanelActivityTest {
mFakeFeatureFactory.panelFeatureProvider = mPanelFeatureProvider; mFakeFeatureFactory.panelFeatureProvider = mPanelFeatureProvider;
mFakePanelContent = new FakePanelContent(); mFakePanelContent = new FakePanelContent();
doReturn(mFakePanelContent).when(mPanelFeatureProvider).getPanel(any(), any()); doReturn(mFakePanelContent).when(mPanelFeatureProvider).getPanel(any(), any());
mSettingsPanelActivity.mPanelFragment = mPanelFragment;
when(mFragmentManager.findFragmentById(R.id.main_content)).thenReturn(mPanelFragment);
when(mSettingsPanelActivity.getSupportFragmentManager()).thenReturn(mFragmentManager);
} }
@Test @Test
@@ -141,11 +154,62 @@ public class SettingsPanelActivityTest {
& SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS).isEqualTo(0); & SYSTEM_FLAG_HIDE_NON_SYSTEM_OVERLAY_WINDOWS).isEqualTo(0);
} }
@Test
public void onStop_panelIsNotCreating_shouldForceUpdate() {
mSettingsPanelActivity.mForceCreation = false;
when(mPanelFragment.isPanelCreating()).thenReturn(false);
mSettingsPanelActivity.mPanelFragment = mPanelFragment;
mSettingsPanelActivity.onStop();
assertThat(mSettingsPanelActivity.mForceCreation).isTrue();
}
@Test
public void onStop_panelIsCreating_shouldNotForceUpdate() {
mSettingsPanelActivity.mForceCreation = false;
when(mPanelFragment.isPanelCreating()).thenReturn(true);
mSettingsPanelActivity.mPanelFragment = mPanelFragment;
mSettingsPanelActivity.onStop();
assertThat(mSettingsPanelActivity.mForceCreation).isFalse();
}
@Test @Test
public void onConfigurationChanged_shouldForceUpdate() { public void onConfigurationChanged_shouldForceUpdate() {
mSettingsPanelActivity.mForceCreation = false; mSettingsPanelActivity.mForceCreation = false;
mSettingsPanelActivity.onConfigurationChanged(new Configuration()); mSettingsPanelActivity.onConfigurationChanged(new Configuration());
assertThat(mSettingsPanelActivity.mForceCreation).isTrue(); assertThat(mSettingsPanelActivity.mForceCreation).isTrue();
} }
@Test
public void onNewIntent_panelIsNotCreating_shouldUpdatePanel() {
when(mPanelFragment.isPanelCreating()).thenReturn(false);
mSettingsPanelActivity.onNewIntent(mSettingsPanelActivity.getIntent());
verify(mPanelFragment).updatePanelWithAnimation();
}
@Test
public void onNewIntent_panelIsCreating_shouldNotUpdatePanel() {
when(mPanelFragment.isPanelCreating()).thenReturn(true);
mSettingsPanelActivity.onNewIntent(mSettingsPanelActivity.getIntent());
verify(mPanelFragment, never()).updatePanelWithAnimation();
}
@Test
public void onNewIntent_panelIsShowingTheSameAction_shouldNotUpdatePanel() {
when(mPanelFragment.isPanelCreating()).thenReturn(false);
when(mPanelFragment.getArguments()).thenReturn(mSettingsPanelActivity.mBundle);
mSettingsPanelActivity.onNewIntent(mSettingsPanelActivity.getIntent());
verify(mPanelFragment, never()).updatePanelWithAnimation();
}
} }