From 7db71ac87afffd3328e1ca7dd2fd3d0b1be13d8d Mon Sep 17 00:00:00 2001 From: Jason Chiu Date: Thu, 6 Aug 2020 16:05:33 +0800 Subject: [PATCH] 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 6a8d2c5e553a1c63e28cb877f5b565159e42ed97) --- .../android/settings/panel/PanelFragment.java | 9 +++ .../settings/panel/SettingsPanelActivity.java | 37 ++++++++--- .../panel/SettingsPanelActivityTest.java | 64 +++++++++++++++++++ 3 files changed, 100 insertions(+), 10 deletions(-) diff --git a/src/com/android/settings/panel/PanelFragment.java b/src/com/android/settings/panel/PanelFragment.java index 9cb626d03a9..d34a09f2ebd 100644 --- a/src/com/android/settings/panel/PanelFragment.java +++ b/src/com/android/settings/panel/PanelFragment.java @@ -97,6 +97,7 @@ public class PanelFragment extends Fragment { private TextView mHeaderSubtitle; private int mMaxHeight; private View mFooterDivider; + private boolean mPanelCreating; private final Map> mSliceLiveData = new LinkedHashMap<>(); @@ -127,6 +128,7 @@ public class PanelFragment extends Fragment { if (mPanelSlices != null) { mPanelSlices.getViewTreeObserver().removeOnGlobalLayoutListener(this); } + mPanelCreating = false; } }; @@ -140,6 +142,7 @@ public class PanelFragment extends Fragment { mLayoutView.getViewTreeObserver() .addOnGlobalLayoutListener(mPanelLayoutListener); mMaxHeight = getResources().getDimensionPixelSize(R.dimen.output_switcher_slice_max_height); + mPanelCreating = true; createPanelContent(); return mLayoutView; } @@ -153,6 +156,7 @@ public class PanelFragment extends Fragment { * Call createPanelContent() once animation end. */ void updatePanelWithAnimation() { + mPanelCreating = true; final View panelContent = mLayoutView.findViewById(R.id.panel_container); final AnimatorSet animatorSet = buildAnimatorSet(mLayoutView, 0.0f /* startY */, panelContent.getHeight() /* endY */, @@ -171,11 +175,16 @@ public class PanelFragment extends Fragment { animatorSet.start(); } + boolean isPanelCreating() { + return mPanelCreating; + } + private void createPanelContent() { final FragmentActivity activity = getActivity(); if (mLayoutView == null) { activity.finish(); } + final ViewGroup.LayoutParams params = mLayoutView.getLayoutParams(); params.height = ViewGroup.LayoutParams.WRAP_CONTENT; mLayoutView.setLayoutParams(params); diff --git a/src/com/android/settings/panel/SettingsPanelActivity.java b/src/com/android/settings/panel/SettingsPanelActivity.java index 68cb8d5163a..b7b15192354 100644 --- a/src/com/android/settings/panel/SettingsPanelActivity.java +++ b/src/com/android/settings/panel/SettingsPanelActivity.java @@ -21,6 +21,7 @@ import static com.android.settingslib.media.MediaOutputSliceConstants.EXTRA_PACK import android.content.Intent; import android.content.res.Configuration; import android.os.Bundle; +import android.text.TextUtils; import android.util.Log; import android.view.Gravity; import android.view.Window; @@ -41,12 +42,14 @@ import com.android.settings.core.HideNonSystemOverlayMixin; */ public class SettingsPanelActivity extends FragmentActivity { - private final String TAG = "panel_activity"; + private static final String TAG = "SettingsPanelActivity"; @VisibleForTesting final Bundle mBundle = new Bundle(); @VisibleForTesting boolean mForceCreation = false; + @VisibleForTesting + PanelFragment mPanelFragment; /** * Key specifying which Panel the app is requesting. @@ -87,7 +90,9 @@ public class SettingsPanelActivity extends FragmentActivity { @Override protected void onStop() { super.onStop(); - mForceCreation = true; + if (mPanelFragment != null && !mPanelFragment.isPanelCreating()) { + mForceCreation = true; + } } @Override @@ -104,10 +109,10 @@ public class SettingsPanelActivity extends FragmentActivity { return; } + final String action = callingIntent.getAction(); // We will use it once media output switch panel support remote device. final String mediaPackageName = callingIntent.getStringExtra(EXTRA_PACKAGE_NAME); - - mBundle.putString(KEY_PANEL_TYPE_ARGUMENT, callingIntent.getAction()); + mBundle.putString(KEY_PANEL_TYPE_ARGUMENT, action); mBundle.putString(KEY_CALLING_PACKAGE_NAME, getCallingPackage()); 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 (!shouldForceCreation && fragment != null && fragment instanceof PanelFragment) { - final PanelFragment panelFragment = (PanelFragment) fragment; - panelFragment.setArguments(mBundle); - panelFragment.updatePanelWithAnimation(); + mPanelFragment = (PanelFragment) fragment; + if (mPanelFragment.isPanelCreating()) { + 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 { setContentView(R.layout.settings_panel); @@ -127,9 +144,9 @@ public class SettingsPanelActivity extends FragmentActivity { window.setGravity(Gravity.BOTTOM); window.setLayout(WindowManager.LayoutParams.MATCH_PARENT, WindowManager.LayoutParams.WRAP_CONTENT); - final PanelFragment panelFragment = new PanelFragment(); - panelFragment.setArguments(mBundle); - fragmentManager.beginTransaction().add(R.id.main_content, panelFragment).commit(); + mPanelFragment = new PanelFragment(); + mPanelFragment.setArguments(new Bundle(mBundle)); + fragmentManager.beginTransaction().add(R.id.main_content, mPanelFragment).commit(); } } } diff --git a/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java b/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java index 833d510b7ac..4a14798b7d7 100644 --- a/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java +++ b/tests/robotests/src/com/android/settings/panel/SettingsPanelActivityTest.java @@ -26,6 +26,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -36,6 +37,9 @@ import android.os.Build; import android.view.Window; import android.view.WindowManager; +import androidx.fragment.app.FragmentManager; + +import com.android.settings.R; import com.android.settings.core.HideNonSystemOverlayMixin; import com.android.settings.testutils.FakeFeatureFactory; @@ -43,6 +47,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; @@ -56,6 +61,10 @@ public class SettingsPanelActivityTest { private FakeSettingsPanelActivity mSettingsPanelActivity; private PanelFeatureProvider mPanelFeatureProvider; private FakePanelContent mFakePanelContent; + @Mock + private PanelFragment mPanelFragment; + @Mock + private FragmentManager mFragmentManager; @Before public void setUp() { @@ -67,6 +76,10 @@ public class SettingsPanelActivityTest { mFakeFeatureFactory.panelFeatureProvider = mPanelFeatureProvider; mFakePanelContent = new FakePanelContent(); 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 @@ -141,11 +154,62 @@ public class SettingsPanelActivityTest { & 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 public void onConfigurationChanged_shouldForceUpdate() { mSettingsPanelActivity.mForceCreation = false; + mSettingsPanelActivity.onConfigurationChanged(new Configuration()); 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(); + } }