From dcfbccbfbc14df312f7fd60e33e50626112a80e3 Mon Sep 17 00:00:00 2001 From: Weng Su Date: Wed, 26 Oct 2022 16:31:00 +0800 Subject: [PATCH 1/3] Remove Modify popup menu for non-admin - Don't add the Modify ContextMenu if the user is not an admin. Bug: 237672190 Test: manual test make RunSettingsRoboTests ROBOTEST_FILTER=NetworkProviderSettingsTest Change-Id: I62e0f5559fa2315450221fcb71e348af59af9549 --- .../network/NetworkProviderSettings.java | 20 ++++++++++++---- .../network/NetworkProviderSettingsTest.java | 24 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/com/android/settings/network/NetworkProviderSettings.java b/src/com/android/settings/network/NetworkProviderSettings.java index 1c480e05735..c5295e67d88 100644 --- a/src/com/android/settings/network/NetworkProviderSettings.java +++ b/src/com/android/settings/network/NetworkProviderSettings.java @@ -34,6 +34,7 @@ import android.net.wifi.WifiManager; import android.os.Bundle; import android.os.Handler; import android.os.PowerManager; +import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; import android.telephony.TelephonyManager; @@ -598,10 +599,7 @@ public class NetworkProviderSettings extends RestrictedSettingsFragment return; } - if (mSelectedWifiEntry.isSaved() && mSelectedWifiEntry.getConnectedState() - != WifiEntry.CONNECTED_STATE_CONNECTED) { - menu.add(Menu.NONE, MENU_ID_MODIFY, 0 /* order */, R.string.wifi_modify); - } + addModifyMenuIfSuitable(menu, mSelectedWifiEntry); } @VisibleForTesting @@ -621,6 +619,14 @@ public class NetworkProviderSettings extends RestrictedSettingsFragment } } + @VisibleForTesting + void addModifyMenuIfSuitable(ContextMenu menu, WifiEntry wifiEntry) { + if (mIsAdmin && wifiEntry.isSaved() + && wifiEntry.getConnectedState() != WifiEntry.CONNECTED_STATE_CONNECTED) { + menu.add(Menu.NONE, MENU_ID_MODIFY, 0 /* order */, R.string.wifi_modify); + } + } + private boolean canForgetNetwork() { return mSelectedWifiEntry.canForget() && !WifiUtils.isNetworkLockedDown(getActivity(), mSelectedWifiEntry.getWifiConfiguration()); @@ -643,6 +649,12 @@ public class NetworkProviderSettings extends RestrictedSettingsFragment () -> launchWifiDppConfiguratorActivity(mSelectedWifiEntry)); return true; case MENU_ID_MODIFY: + if (!mIsAdmin) { + Log.e(TAG, "Can't modify Wi-Fi because the user isn't admin."); + EventLog.writeEvent(0x534e4554, "237672190", UserHandle.myUserId(), + "User isn't admin"); + return true; + } showDialog(mSelectedWifiEntry, WifiConfigUiBase2.MODE_MODIFY); return true; default: diff --git a/tests/robotests/src/com/android/settings/network/NetworkProviderSettingsTest.java b/tests/robotests/src/com/android/settings/network/NetworkProviderSettingsTest.java index 5122def9843..35891843c1b 100644 --- a/tests/robotests/src/com/android/settings/network/NetworkProviderSettingsTest.java +++ b/tests/robotests/src/com/android/settings/network/NetworkProviderSettingsTest.java @@ -17,9 +17,11 @@ package com.android.settings.network; import static com.android.settings.network.NetworkProviderSettings.MENU_ID_DISCONNECT; import static com.android.settings.network.NetworkProviderSettings.MENU_ID_FORGET; +import static com.android.settings.network.NetworkProviderSettings.MENU_ID_MODIFY; import static com.android.settings.network.NetworkProviderSettings.MENU_ID_SHARE; import static com.android.settings.wifi.WifiConfigUiBase2.MODE_CONNECT; import static com.android.settings.wifi.WifiConfigUiBase2.MODE_MODIFY; +import static com.android.wifitrackerlib.WifiEntry.CONNECTED_STATE_DISCONNECTED; import static com.google.common.truth.Truth.assertThat; @@ -729,6 +731,28 @@ public class NetworkProviderSettingsTest { verify(mContextMenu, never()).add(anyInt(), eq(MENU_ID_FORGET), anyInt(), anyInt()); } + @Test + public void addModifyMenuIfSuitable_isAdmin_addMenu() { + mNetworkProviderSettings.mIsAdmin = true; + when(mWifiEntry.isSaved()).thenReturn(true); + when(mWifiEntry.getConnectedState()).thenReturn(CONNECTED_STATE_DISCONNECTED); + + mNetworkProviderSettings.addModifyMenuIfSuitable(mContextMenu, mWifiEntry); + + verify(mContextMenu).add(anyInt(), eq(MENU_ID_MODIFY), anyInt(), anyInt()); + } + + @Test + public void addModifyMenuIfSuitable_isNotAdmin_notAddMenu() { + mNetworkProviderSettings.mIsAdmin = false; + when(mWifiEntry.isSaved()).thenReturn(true); + when(mWifiEntry.getConnectedState()).thenReturn(CONNECTED_STATE_DISCONNECTED); + + mNetworkProviderSettings.addModifyMenuIfSuitable(mContextMenu, mWifiEntry); + + verify(mContextMenu, never()).add(anyInt(), eq(MENU_ID_MODIFY), anyInt(), anyInt()); + } + @Test public void getNonIndexableKeys_allowedChangeWifiState_keyNotReturned() { when(mWifiRestriction.isChangeWifiStateAllowed(mContext)).thenReturn(true); From 88d876318c47931289dcc67764705c4069555346 Mon Sep 17 00:00:00 2001 From: Zaiyue Xue Date: Fri, 14 Oct 2022 13:51:36 +0800 Subject: [PATCH 2/3] Fix broken battery usage Robolectric tests Bug: 248686898 Test: presubmit Change-Id: I4f09f9e5af57a62249970cc2170b60dab267488e Merged-In: I4f09f9e5af57a62249970cc2170b60dab267488e --- .../BatteryChartPreferenceControllerTest.java | 29 +++++++++++++- .../batteryusage/DataProcessorTest.java | 40 ++++++++++++++----- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java index 26e0f5074ed..4e552b941c0 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java @@ -18,6 +18,8 @@ package com.android.settings.fuelgauge.batteryusage; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyFloat; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; @@ -36,6 +38,7 @@ import android.os.Bundle; import android.os.LocaleList; import android.text.format.DateUtils; import android.view.View; +import android.view.ViewPropertyAnimator; import android.widget.LinearLayout; import androidx.preference.Preference; @@ -85,6 +88,8 @@ public final class BatteryChartPreferenceControllerTest { @Mock private BatteryChartView mHourlyChartView; @Mock + private ViewPropertyAnimator mViewPropertyAnimator; + @Mock private PowerGaugePreference mPowerGaugePreference; @Mock private BatteryUtils mBatteryUtils; @@ -116,6 +121,7 @@ public final class BatteryChartPreferenceControllerTest { .when(mFeatureFactory.powerUsageFeatureProvider) .getHideApplicationEntries(mContext); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); + setupHourlyChartViewAnimationMock(); mBatteryChartPreferenceController = createController(); mBatteryChartPreferenceController.mPrefContext = mContext; mBatteryChartPreferenceController.mAppListPrefGroup = mAppListGroup; @@ -174,11 +180,15 @@ public final class BatteryChartPreferenceControllerTest { @Test public void setBatteryChartViewModel_6Hours() { + reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); + mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(6)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.GONE); verify(mHourlyChartView, atLeastOnce()).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); // Ignore fast refresh ui from the data processor callback. verify(mHourlyChartView, atLeast(0)).setViewModel(null); verify(mHourlyChartView, atLeastOnce()).setViewModel(new BatteryChartViewModel( @@ -192,6 +202,10 @@ public final class BatteryChartPreferenceControllerTest { @Test public void setBatteryChartViewModel_60Hours() { + reset(mDailyChartView); + reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); + BatteryChartViewModel expectedDailyViewModel = new BatteryChartViewModel( List.of(100, 83, 59, 41), // "Sat", "Sun", "Mon", "Mon" @@ -205,16 +219,18 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(60)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.VISIBLE); - verify(mHourlyChartView, atLeastOnce()).setVisibility(View.GONE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(0f); verify(mDailyChartView).setViewModel(expectedDailyViewModel); reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); mBatteryChartPreferenceController.mDailyChartIndex = 0; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(0); verify(mDailyChartView).setViewModel(expectedDailyViewModel); @@ -234,12 +250,14 @@ public final class BatteryChartPreferenceControllerTest { reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); mBatteryChartPreferenceController.mDailyChartIndex = 1; mBatteryChartPreferenceController.mHourlyChartIndex = 6; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(1); verify(mDailyChartView).setViewModel(expectedDailyViewModel); BatteryChartViewModel expectedHourlyViewModel = new BatteryChartViewModel( @@ -264,6 +282,7 @@ public final class BatteryChartPreferenceControllerTest { reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); mBatteryChartPreferenceController.mDailyChartIndex = 2; mBatteryChartPreferenceController.mHourlyChartIndex = @@ -271,6 +290,7 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(2); verify(mDailyChartView).setViewModel(expectedDailyViewModel); verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( @@ -734,4 +754,11 @@ public final class BatteryChartPreferenceControllerTest { controller.mPrefContext = mContext; return controller; } + + private void setupHourlyChartViewAnimationMock() { + doReturn(mViewPropertyAnimator).when(mHourlyChartView).animate(); + doReturn(mViewPropertyAnimator).when(mViewPropertyAnimator).alpha(anyFloat()); + doReturn(mViewPropertyAnimator).when(mViewPropertyAnimator).setDuration(anyLong()); + doReturn(mViewPropertyAnimator).when(mViewPropertyAnimator).setListener(any()); + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java index c4832041314..df9d8655109 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java @@ -579,9 +579,13 @@ public class DataProcessorTest { ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY, /*consumePercentage=*/ 25.0, /*foregroundUsageTimeInMs=*/ 50, /*backgroundUsageTimeInMs=*/ 60); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 3); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 3); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test @@ -674,9 +678,13 @@ public class DataProcessorTest { assertThat(resultMap.get(0).get(0)).isNotNull(); assertThat(resultMap.get(0).get(DataProcessor.SELECTED_INDEX_ALL)).isNotNull(); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 2); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 2); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test @@ -739,9 +747,13 @@ public class DataProcessorTest { assertThat(resultMap.get(0).get(0)).isNotNull(); assertThat(resultMap.get(0).get(DataProcessor.SELECTED_INDEX_ALL)).isNotNull(); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 1); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 1); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test @@ -814,9 +826,13 @@ public class DataProcessorTest { ConvertUtils.CONSUMER_TYPE_UID_BATTERY, /*consumePercentage=*/ 50.0, /*foregroundUsageTimeInMs=*/ 10, /*backgroundUsageTimeInMs=*/ 20); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 1); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 1); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 1); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 1); } @Test @@ -889,9 +905,13 @@ public class DataProcessorTest { resultEntry = resultDiffData.getAppDiffEntryList().get(1); assertThat(resultEntry.mBackgroundUsageTimeInMs).isEqualTo(0); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 2); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 2); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test From 4fbd2064d6e835cf8e145c08d5692b67376ddbcb Mon Sep 17 00:00:00 2001 From: Zaiyue Xue Date: Tue, 25 Oct 2022 15:23:37 +0800 Subject: [PATCH 3/3] Fix b/248686898: Battery Usage list renders items on top of each other after swiping back from an app Root cause: When adding an animation to the hourly chartview which is scolled up out of the page, onBindViewHolder() will be trigered, then UI is refreshed again, and then the animation is added to the hourly charview again. This is a dead loop. This fix adds animation to the hourly chartview only if the visibility is changed. http://cs/android/packages/apps/Settings/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java;l=676;rcl=9f24ea815d637a05e1b7ddf2d4372d716d749229 http://cs/android/packages/apps/Settings/src/com/android/settings/fuelgauge/batteryusage/BatteryHistoryPreference.java;l=102;rcl=9f24ea815d637a05e1b7ddf2d4372d716d749229 Bug: 248686898 Fix: 248686898 Test: manually Change-Id: I93d4d089f537515d452c1330f5d75a6726b229f8 (cherry picked from commit aabea1689896adf3799a7eeffe11795dee3bd25c) Merged-In: I93d4d089f537515d452c1330f5d75a6726b229f8 --- .../batteryusage/BatteryChartPreferenceController.java | 4 +++- .../batteryusage/BatteryChartPreferenceControllerTest.java | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java index 2c924885c3b..56da0f47932 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java @@ -108,6 +108,7 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll private boolean mIs24HourFormat; private boolean mIsFooterPrefAdded = false; + private boolean mHourlyChartVisible = true; private View mBatteryChartViewGroup; private View mCategoryTitleView; private PreferenceScreen mPreferenceScreen; @@ -690,9 +691,10 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll } private void animateBatteryHourlyChartView(final boolean visible) { - if (mHourlyChartView == null) { + if (mHourlyChartView == null || mHourlyChartVisible == visible) { return; } + mHourlyChartVisible = visible; if (visible) { mHourlyChartView.setVisibility(View.VISIBLE); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java index 4e552b941c0..9fbcb1688db 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java @@ -187,8 +187,6 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(6)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.GONE); - verify(mHourlyChartView, atLeastOnce()).setVisibility(View.VISIBLE); - verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); // Ignore fast refresh ui from the data processor callback. verify(mHourlyChartView, atLeast(0)).setViewModel(null); verify(mHourlyChartView, atLeastOnce()).setViewModel(new BatteryChartViewModel( @@ -256,7 +254,6 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.mHourlyChartIndex = 6; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); - verify(mHourlyChartView).setVisibility(View.VISIBLE); verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(1); verify(mDailyChartView).setViewModel(expectedDailyViewModel); @@ -289,7 +286,6 @@ public final class BatteryChartPreferenceControllerTest { BatteryChartViewModel.SELECTED_INDEX_ALL; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); - verify(mHourlyChartView).setVisibility(View.VISIBLE); verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(2); verify(mDailyChartView).setViewModel(expectedDailyViewModel);