From 5a2f5ecff5400c3364c98f0805be68e255051582 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Wed, 11 May 2022 11:42:11 +0800 Subject: [PATCH] [DO NOT MERGE] Fix flicker for Data Usage page Both "Mobile data usage" & "Non-carrier data usage". By, 1. Use summary placeholder for usage amount to avoid shift 2. Before fix CycleListener's onItemSelected() is called multiple times, cause the app list to flash, let DataUsageList to handle the dedup logic to better handling. 3. Before fix if return from App Usage page, no loading view is displayed (only first enter has it), move this to onResume() to fix. 4. Before fix the cycles passed to App Usage page is cached (even when the cycles are changed), clear the cache when onResume() to fix. 5. Listener in SpinnerPreference could be null, add safety guard to it. Fix: 187019210 Test: manual visual test Change-Id: I95e544c46333496f4f30ed77dafa4779b4d66019 --- res/xml/data_usage_list.xml | 3 +- .../settings/datausage/CycleAdapter.java | 12 +-- .../settings/datausage/DataUsageList.java | 77 ++++++++++++------- .../settings/datausage/SpinnerPreference.java | 35 +++++---- .../settings/datausage/DataUsageListTest.java | 3 +- 5 files changed, 74 insertions(+), 56 deletions(-) diff --git a/res/xml/data_usage_list.xml b/res/xml/data_usage_list.xml index 9ea6a914a77..644fca4faf7 100644 --- a/res/xml/data_usage_list.xml +++ b/res/xml/data_usage_list.xml @@ -17,7 +17,8 @@ + android:key="usage_amount" + android:title="@string/summary_placeholder"> diff --git a/src/com/android/settings/datausage/CycleAdapter.java b/src/com/android/settings/datausage/CycleAdapter.java index b41b6aad91b..2af40120864 100644 --- a/src/com/android/settings/datausage/CycleAdapter.java +++ b/src/com/android/settings/datausage/CycleAdapter.java @@ -21,7 +21,6 @@ import com.android.settingslib.net.NetworkCycleData; import com.android.settingslib.widget.SettingsSpinnerAdapter; import java.util.List; -import java.util.Objects; public class CycleAdapter extends SettingsSpinnerAdapter { @@ -67,7 +66,7 @@ public class CycleAdapter extends SettingsSpinnerAdapter * Rebuild list based on network data. Always selects the newest item, * updating the inspection range on chartData. */ - public boolean updateCycleList(List cycleData) { + public void updateCycleList(List cycleData) { mSpinner.setOnItemSelectedListener(mListener); // stash away currently selected cycle to try restoring below final CycleAdapter.CycleItem previousItem = (CycleAdapter.CycleItem) @@ -83,16 +82,7 @@ public class CycleAdapter extends SettingsSpinnerAdapter if (getCount() > 0) { final int position = findNearestPosition(previousItem); mSpinner.setSelection(position); - - // only force-update cycle when changed; skipping preserves any - // user-defined inspection region. - final CycleAdapter.CycleItem selectedItem = getItem(position); - if (!Objects.equals(selectedItem, previousItem)) { - mListener.onItemSelected(null, null, position, 0); - return false; - } } - return true; } /** diff --git a/src/com/android/settings/datausage/DataUsageList.java b/src/com/android/settings/datausage/DataUsageList.java index 4a44a1674e0..9d8fbb14162 100644 --- a/src/com/android/settings/datausage/DataUsageList.java +++ b/src/com/android/settings/datausage/DataUsageList.java @@ -69,6 +69,7 @@ import com.android.settingslib.net.UidDetailProvider; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; /** * Panel showing data usage history across various networks, including options @@ -111,7 +112,11 @@ public class DataUsageList extends DataUsageBaseFragment private ChartDataUsagePreference mChart; private List mCycleData; + // Caches the cycles for startAppDataUsage usage, which need be cleared when resumed. private ArrayList mCycles; + // Spinner will keep the selected cycle even after paused, this only keeps the displayed cycle, + // which need be cleared when resumed. + private CycleAdapter.CycleItem mLastDisplayedCycle; private UidDetailProvider mUidDetailProvider; private CycleAdapter mCycleAdapter; private Preference mUsageAmount; @@ -199,13 +204,15 @@ public class DataUsageList extends DataUsageBaseFragment mLoadingViewController = new LoadingViewController( getView().findViewById(R.id.loading_container), getListView()); - mLoadingViewController.showLoadingViewDelayed(); } @Override public void onResume() { super.onResume(); + mLoadingViewController.showLoadingViewDelayed(); mDataStateListener.start(mSubId); + mCycles = null; + mLastDisplayedCycle = null; // kick off loader for network history // TODO: consider chaining two loaders together instead of reloading @@ -319,9 +326,46 @@ public class DataUsageList extends DataUsageBaseFragment } // generate cycle list based on policy and available history - if (mCycleAdapter.updateCycleList(mCycleData)) { - updateDetailData(); + mCycleAdapter.updateCycleList(mCycleData); + updateSelectedCycle(); + } + + /** + * Updates the chart and detail data when initial loaded or selected cycle changed. + */ + private void updateSelectedCycle() { + // Avoid from updating UI after #onStop. + if (!getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.STARTED)) { + return; } + + // Avoid from updating UI when async query still on-going. + // This could happen when a request from #onMobileDataEnabledChange. + if (mCycleData == null) { + return; + } + + final int position = mCycleSpinner.getSelectedItemPosition(); + if (mCycleAdapter.getCount() == 0 || position < 0) { + return; + } + final CycleAdapter.CycleItem cycle = mCycleAdapter.getItem(position); + if (Objects.equals(cycle, mLastDisplayedCycle)) { + // Avoid duplicate update to avoid page flash. + return; + } + mLastDisplayedCycle = cycle; + + if (LOGD) { + Log.d(TAG, "showing cycle " + cycle + ", [start=" + cycle.start + ", end=" + + cycle.end + "]"); + } + + // update chart to show selected cycle, and update detail data + // to match updated sweep bounds. + mChart.setNetworkCycleData(mCycleData.get(position)); + + updateDetailData(); } /** @@ -495,33 +539,10 @@ public class DataUsageList extends DataUsageBaseFragment return Math.max(largest, item.total); } - private OnItemSelectedListener mCycleListener = new OnItemSelectedListener() { + private final OnItemSelectedListener mCycleListener = new OnItemSelectedListener() { @Override public void onItemSelected(AdapterView parent, View view, int position, long id) { - final CycleAdapter.CycleItem cycle = (CycleAdapter.CycleItem) - mCycleSpinner.getSelectedItem(); - - if (LOGD) { - Log.d(TAG, "showing cycle " + cycle + ", start=" + cycle.start + ", end=" - + cycle.end + "]"); - } - - // Avoid from updating UI after #onStop. - if (!getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.STARTED)) { - return; - } - - // Avoid from updating UI when async query still on-going. - // This could happen when a request from #onMobileDataEnabledChange. - if (mCycleData == null) { - return; - } - - // update chart to show selected cycle, and update detail data - // to match updated sweep bounds. - mChart.setNetworkCycleData(mCycleData.get(position)); - - updateDetailData(); + updateSelectedCycle(); } @Override diff --git a/src/com/android/settings/datausage/SpinnerPreference.java b/src/com/android/settings/datausage/SpinnerPreference.java index c4b7a4e18da..c6b5f9f8ecc 100644 --- a/src/com/android/settings/datausage/SpinnerPreference.java +++ b/src/com/android/settings/datausage/SpinnerPreference.java @@ -14,6 +14,7 @@ package com.android.settings.datausage; +import android.annotation.Nullable; import android.content.Context; import android.util.AttributeSet; import android.view.View; @@ -28,6 +29,7 @@ import com.android.settings.R; public class SpinnerPreference extends Preference implements CycleAdapter.SpinnerInterface { private CycleAdapter mAdapter; + @Nullable private AdapterView.OnItemSelectedListener mListener; private Object mCurrentObject; private int mPosition; @@ -88,19 +90,24 @@ public class SpinnerPreference extends Preference implements CycleAdapter.Spinne view.findViewById(R.id.cycles_spinner).performClick(); } - private final AdapterView.OnItemSelectedListener mOnSelectedListener - = new AdapterView.OnItemSelectedListener() { - @Override - public void onItemSelected(AdapterView parent, View view, int position, long id) { - if (mPosition == position) return; - mPosition = position; - mCurrentObject = mAdapter.getItem(position); - mListener.onItemSelected(parent, view, position, id); - } + private final AdapterView.OnItemSelectedListener mOnSelectedListener = + new AdapterView.OnItemSelectedListener() { + @Override + public void onItemSelected( + AdapterView parent, View view, int position, long id) { + if (mPosition == position) return; + mPosition = position; + mCurrentObject = mAdapter.getItem(position); + if (mListener != null) { + mListener.onItemSelected(parent, view, position, id); + } + } - @Override - public void onNothingSelected(AdapterView parent) { - mListener.onNothingSelected(parent); - } - }; + @Override + public void onNothingSelected(AdapterView parent) { + if (mListener != null) { + mListener.onNothingSelected(parent); + } + } + }; } diff --git a/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java b/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java index e4f5b1fc9e9..a13cf6c7c8f 100644 --- a/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java +++ b/tests/robotests/src/com/android/settings/datausage/DataUsageListTest.java @@ -94,6 +94,7 @@ public class DataUsageListTest { mMobileDataEnabledListener); ReflectionHelpers.setField(mDataUsageList, "services", mNetworkServices); doReturn(mLoaderManager).when(mDataUsageList).getLoaderManager(); + mDataUsageList.mLoadingViewController = mock(LoadingViewController.class); } @Test @@ -207,8 +208,6 @@ public class DataUsageListTest { @Test public void onLoadFinished_networkCycleDataCallback_shouldShowCycleSpinner() { - final LoadingViewController loadingViewController = mock(LoadingViewController.class); - mDataUsageList.mLoadingViewController = loadingViewController; final Spinner spinner = getSpinner(getHeader()); spinner.setVisibility(View.INVISIBLE); mDataUsageList.mCycleSpinner = spinner;