From acbe7b52635b28da3438ae056183ec6f5f9903a0 Mon Sep 17 00:00:00 2001 From: Weng Su Date: Tue, 7 Dec 2021 23:16:47 +0800 Subject: [PATCH 1/3] [Provider Model] Fix WiFi list count timing issue (QS Internet Dialog) - The WiFi list count will be updated when the WiFi scan result is called back, but it needs to be reduced immediately after turning off the airplane mode. - See b/206807144#comment12 for a video. Bug: 206807144 Test: manual test atest -c InternetAdapterTest \ InternetDialogTest Change-Id: I6082226a63808e8a532bf4e9f341800a1887b8f5 --- .../qs/tiles/dialog/InternetAdapter.java | 22 ++++++- .../qs/tiles/dialog/InternetDialog.java | 18 ++++-- .../dialog/InternetDialogController.java | 11 ++-- .../qs/tiles/dialog/InternetAdapterTest.java | 58 ++++++++++++++++++- .../qs/tiles/dialog/InternetDialogTest.java | 4 +- 5 files changed, 99 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetAdapter.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetAdapter.java index 99eb5b6519bc5..fbb465e202b0f 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetAdapter.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetAdapter.java @@ -53,7 +53,10 @@ public class InternetAdapter extends RecyclerView.Adapter mWifiEntries; - private int mWifiEntriesCount; + @VisibleForTesting + protected int mWifiEntriesCount; + @VisibleForTesting + protected int mMaxEntriesCount = InternetDialogController.MAX_WIFI_ENTRY_COUNT; protected View mHolderView; protected Context mContext; @@ -87,7 +90,8 @@ public class InternetAdapter extends RecyclerView.Adapter wifiEntries, int wifiEntriesCount) { mWifiEntries = wifiEntries; - mWifiEntriesCount = wifiEntriesCount; + mWifiEntriesCount = + (wifiEntriesCount < mMaxEntriesCount) ? wifiEntriesCount : mMaxEntriesCount; } /** @@ -100,6 +104,20 @@ public class InternetAdapter extends RecyclerView.Adapter count) { + mWifiEntriesCount = count; + notifyDataSetChanged(); + } + } + /** * ViewHolder for binding Wi-Fi view. */ diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java index 2a398496af77b..80f1ea600a3b0 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java @@ -138,7 +138,7 @@ public class InternetDialog extends SystemUIDialog implements @VisibleForTesting protected int mWifiEntriesCount; @VisibleForTesting - protected boolean mHasMoreEntry; + protected boolean mHasMoreWifiEntries; // Wi-Fi scanning progress bar protected boolean mIsProgressBarVisible; @@ -462,9 +462,17 @@ public class InternetDialog extends SystemUIDialog implements mSeeAllLayout.setVisibility(View.GONE); return; } - mWifiRecyclerView.setMinimumHeight(mWifiNetworkHeight * getWifiListMaxCount()); + final int wifiListMaxCount = getWifiListMaxCount(); + if (mAdapter.getItemCount() > wifiListMaxCount) { + mAdapter.setMaxEntriesCount(wifiListMaxCount); + mHasMoreWifiEntries = true; + } + final int wifiListMinHeight = mWifiNetworkHeight * wifiListMaxCount; + if (mWifiRecyclerView.getMinimumHeight() != wifiListMinHeight) { + mWifiRecyclerView.setMinimumHeight(wifiListMinHeight); + } mWifiRecyclerView.setVisibility(View.VISIBLE); - mSeeAllLayout.setVisibility(mHasMoreEntry ? View.VISIBLE : View.INVISIBLE); + mSeeAllLayout.setVisibility(mHasMoreWifiEntries ? View.VISIBLE : View.INVISIBLE); } @VisibleForTesting @@ -654,14 +662,14 @@ public class InternetDialog extends SystemUIDialog implements @Override @WorkerThread public void onAccessPointsChanged(@Nullable List wifiEntries, - @Nullable WifiEntry connectedEntry, boolean hasMoreEntry) { + @Nullable WifiEntry connectedEntry, boolean hasMoreWifiEntries) { // Should update the carrier network layout when it is connected under airplane mode ON. boolean shouldUpdateCarrierNetwork = mMobileNetworkLayout.getVisibility() == View.VISIBLE && mInternetDialogController.isAirplaneModeEnabled(); mHandler.post(() -> { mConnectedWifiEntry = connectedEntry; mWifiEntriesCount = wifiEntries == null ? 0 : wifiEntries.size(); - mHasMoreEntry = hasMoreEntry; + mHasMoreWifiEntries = hasMoreWifiEntries; updateDialog(shouldUpdateCarrierNetwork /* shouldUpdateMobileNetwork */); mAdapter.setWifiEntries(wifiEntries, mWifiEntriesCount); mAdapter.notifyDataSetChanged(); diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java index 4908d4fb214e2..a64927fe106e2 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java @@ -883,8 +883,11 @@ public class InternetDialogController implements AccessPointController.AccessPoi return; } - boolean hasMoreEntry = false; + boolean hasMoreWifiEntries = false; int count = MAX_WIFI_ENTRY_COUNT; + // TODO: (b/206807144) Plan to remove the design of calculating WiFi list here, and + // calculate the number of WiFi list by referring to the Visibility of each network in + // InternetDialog. if (mHasEthernet) { count -= 1; } @@ -895,7 +898,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi if (count > wifiTotalCount) { count = wifiTotalCount; } else if (count < wifiTotalCount) { - hasMoreEntry = true; + hasMoreWifiEntries = true; } WifiEntry connectedEntry = null; @@ -911,7 +914,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi mConnectedEntry = connectedEntry; mWifiEntriesCount = wifiEntries.size(); - mCallback.onAccessPointsChanged(wifiEntries, mConnectedEntry, hasMoreEntry); + mCallback.onAccessPointsChanged(wifiEntries, mConnectedEntry, hasMoreWifiEntries); } @Override @@ -1061,7 +1064,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi void dismissDialog(); void onAccessPointsChanged(@Nullable List wifiEntries, - @Nullable WifiEntry connectedEntry, boolean hasMoreEntry); + @Nullable WifiEntry connectedEntry, boolean hasMoreWifiEntries); } void makeOverlayToast(int stringId) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetAdapterTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetAdapterTest.java index 77946cf791aac..d3bb241baad42 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetAdapterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetAdapterTest.java @@ -1,5 +1,7 @@ package com.android.systemui.qs.tiles.dialog; +import static com.android.systemui.qs.tiles.dialog.InternetDialogController.MAX_WIFI_ENTRY_COUNT; + import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -83,7 +85,7 @@ public class InternetAdapterTest extends SysuiTestCase { @Test public void getItemCount_returnWifiEntriesCount() { - for (int i = 0; i < InternetDialogController.MAX_WIFI_ENTRY_COUNT; i++) { + for (int i = 0; i < MAX_WIFI_ENTRY_COUNT; i++) { mInternetAdapter.setWifiEntries(mWifiEntries, i /* wifiEntriesCount */); assertThat(mInternetAdapter.getItemCount()).isEqualTo(i); @@ -140,6 +142,60 @@ public class InternetAdapterTest extends SysuiTestCase { verify(mWifiIconInjector).getIcon(eq(true) /* noInternet */, anyInt()); } + @Test + public void setWifiEntries_wifiCountLessThenMaxCount_setWifiCount() { + final int wifiCount = MAX_WIFI_ENTRY_COUNT - 1; + mInternetAdapter.mMaxEntriesCount = MAX_WIFI_ENTRY_COUNT; + + mInternetAdapter.setWifiEntries(mWifiEntries, wifiCount); + + assertThat(mInternetAdapter.mWifiEntriesCount).isEqualTo(wifiCount); + } + + @Test + public void setWifiEntries_wifiCountGreaterThenMaxCount_setMaxCount() { + final int wifiCount = MAX_WIFI_ENTRY_COUNT; + mInternetAdapter.mMaxEntriesCount = MAX_WIFI_ENTRY_COUNT - 1; + + mInternetAdapter.setWifiEntries(mWifiEntries, wifiCount); + + assertThat(mInternetAdapter.mWifiEntriesCount).isEqualTo(mInternetAdapter.mMaxEntriesCount); + } + + @Test + public void setMaxEntriesCount_maxCountLessThenZero_doNothing() { + mInternetAdapter.mMaxEntriesCount = MAX_WIFI_ENTRY_COUNT; + final int maxCount = -1; + + mInternetAdapter.setMaxEntriesCount(maxCount); + + assertThat(mInternetAdapter.mMaxEntriesCount).isEqualTo(MAX_WIFI_ENTRY_COUNT); + } + + @Test + public void setMaxEntriesCount_maxCountGreaterThenWifiCount_updateMaxCount() { + mInternetAdapter.mWifiEntriesCount = MAX_WIFI_ENTRY_COUNT - 2; + mInternetAdapter.mMaxEntriesCount = MAX_WIFI_ENTRY_COUNT; + final int maxCount = MAX_WIFI_ENTRY_COUNT - 1; + + mInternetAdapter.setMaxEntriesCount(maxCount); + + assertThat(mInternetAdapter.mWifiEntriesCount).isEqualTo(MAX_WIFI_ENTRY_COUNT - 2); + assertThat(mInternetAdapter.mMaxEntriesCount).isEqualTo(maxCount); + } + + @Test + public void setMaxEntriesCount_maxCountLessThenWifiCount_updateBothCount() { + mInternetAdapter.mWifiEntriesCount = MAX_WIFI_ENTRY_COUNT; + mInternetAdapter.mMaxEntriesCount = MAX_WIFI_ENTRY_COUNT; + final int maxCount = MAX_WIFI_ENTRY_COUNT - 1; + + mInternetAdapter.setMaxEntriesCount(maxCount); + + assertThat(mInternetAdapter.mWifiEntriesCount).isEqualTo(maxCount); + assertThat(mInternetAdapter.mMaxEntriesCount).isEqualTo(maxCount); + } + @Test public void viewHolderUpdateEndIcon_wifiConnected_updateGearIcon() { mTestableResources.addOverride(GEAR_ICON_RES_ID, mGearIcon); diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java index 89537884903af..a00ebcc8fd6b7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java @@ -347,7 +347,7 @@ public class InternetDialogTest extends SysuiTestCase { // The preconditions WiFi ON and WiFi entries are already in setUp() mInternetDialog.mConnectedWifiEntry = null; mInternetDialog.mWifiEntriesCount = MAX_WIFI_ENTRY_COUNT; - mInternetDialog.mHasMoreEntry = true; + mInternetDialog.mHasMoreWifiEntries = true; mInternetDialog.updateDialog(false); @@ -360,7 +360,7 @@ public class InternetDialogTest extends SysuiTestCase { public void updateDialog_wifiOnAndHasBothWifiEntry_showBothWifiEntryAndSeeAll() { // The preconditions WiFi ON and WiFi entries are already in setUp() mInternetDialog.mWifiEntriesCount = MAX_WIFI_ENTRY_COUNT - 1; - mInternetDialog.mHasMoreEntry = true; + mInternetDialog.mHasMoreWifiEntries = true; mInternetDialog.updateDialog(false); From c5d0caf274926b7fc1e6ea82f12162517968ce1f Mon Sep 17 00:00:00 2001 From: Weng Su Date: Fri, 10 Dec 2021 03:56:23 +0800 Subject: [PATCH 2/3] [Provider Model] Reduce the max of WiFi networks (QS Internet Dialog) - The max of 3 wifi networks when Wi-Fi is on (regardless of if airplane mode is on or off). - The max of 6 items in Internet Panel. - See b/206807144#comment20 for a video. Bug: 206807144 Test: manual test atest -c InternetAdapterTest \ InternetDialogControllerTest \ InternetDialogTest Change-Id: Ia7889a78a6d803fbb5c5ac9107dcc4033626a27b --- .../qs/tiles/dialog/InternetDialog.java | 11 ++- .../dialog/InternetDialogController.java | 61 ++++-------- .../dialog/InternetDialogControllerTest.java | 99 ------------------- .../qs/tiles/dialog/InternetDialogTest.java | 41 ++++---- 4 files changed, 52 insertions(+), 160 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java index 80f1ea600a3b0..d894c96dfa289 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java @@ -16,6 +16,7 @@ package com.android.systemui.qs.tiles.dialog; import static com.android.systemui.Prefs.Key.QS_HAS_TURNED_OFF_MOBILE_DATA; +import static com.android.systemui.qs.tiles.dialog.InternetDialogController.MAX_WIFI_ENTRY_COUNT; import android.app.AlertDialog; import android.content.Context; @@ -79,6 +80,7 @@ public class InternetDialog extends SystemUIDialog implements private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); static final long PROGRESS_DELAY_MS = 1500L; + static final int MAX_NETWORK_COUNT = 4; private final Handler mHandler; private final Executor mBackgroundExecutor; @@ -478,13 +480,20 @@ public class InternetDialog extends SystemUIDialog implements @VisibleForTesting @MainThread int getWifiListMaxCount() { - int count = InternetDialogController.MAX_WIFI_ENTRY_COUNT; + // Use the maximum count of networks to calculate the remaining count for Wi-Fi networks. + int count = MAX_NETWORK_COUNT; if (mEthernetLayout.getVisibility() == View.VISIBLE) { count -= 1; } if (mMobileNetworkLayout.getVisibility() == View.VISIBLE) { count -= 1; } + + // If the remaining count is greater than the maximum count of the Wi-Fi network, the + // maximum count of the Wi-Fi network is used. + if (count > MAX_WIFI_ENTRY_COUNT) { + count = MAX_WIFI_ENTRY_COUNT; + } if (mConnectedWifListLayout.getVisibility() == View.VISIBLE) { count -= 1; } diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java index a64927fe106e2..e01918ec717aa 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java @@ -125,7 +125,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi R.string.all_network_unavailable; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); - static final int MAX_WIFI_ENTRY_COUNT = 4; + static final int MAX_WIFI_ENTRY_COUNT = 3; private WifiManager mWifiManager; private Context mContext; @@ -143,8 +143,6 @@ public class InternetDialogController implements AccessPointController.AccessPoi private AccessPointController mAccessPointController; private IntentFilter mConnectionStateFilter; private InternetDialogCallback mCallback; - private WifiEntry mConnectedEntry; - private int mWifiEntriesCount; private UiEventLogger mUiEventLogger; private BroadcastDispatcher mBroadcastDispatcher; private KeyguardUpdateMonitor mKeyguardUpdateMonitor; @@ -156,6 +154,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi private SignalDrawable mSignalDrawable; private LocationController mLocationController; private DialogLaunchAnimator mDialogLaunchAnimator; + private boolean mHasWifiEntries; @VisibleForTesting static final float TOAST_PARAMS_HORIZONTAL_WEIGHT = 1.0f; @@ -334,7 +333,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi return mContext.getText(SUBTITLE_TEXT_UNLOCK_TO_VIEW_NETWORKS); } - if (mConnectedEntry != null || mWifiEntriesCount > 0) { + if (mHasWifiEntries) { return mCanConfigWifi ? mContext.getText(SUBTITLE_TEXT_TAP_A_NETWORK_TO_CONNECT) : null; } @@ -875,46 +874,28 @@ public class InternetDialogController implements AccessPointController.AccessPoi return; } - if (accessPoints == null || accessPoints.size() == 0) { - mConnectedEntry = null; - mWifiEntriesCount = 0; - mCallback.onAccessPointsChanged(null /* wifiEntries */, null /* connectedEntry */, - false /* hasMoreEntry */); - return; - } - - boolean hasMoreWifiEntries = false; - int count = MAX_WIFI_ENTRY_COUNT; - // TODO: (b/206807144) Plan to remove the design of calculating WiFi list here, and - // calculate the number of WiFi list by referring to the Visibility of each network in - // InternetDialog. - if (mHasEthernet) { - count -= 1; - } - if (hasActiveSubId() || isCarrierNetworkActive()) { - count -= 1; - } - final int wifiTotalCount = accessPoints.size(); - if (count > wifiTotalCount) { - count = wifiTotalCount; - } else if (count < wifiTotalCount) { - hasMoreWifiEntries = true; - } - WifiEntry connectedEntry = null; - final List wifiEntries = new ArrayList<>(); - for (int i = 0; i < count; i++) { - WifiEntry entry = accessPoints.get(i); - if (connectedEntry == null && entry.isDefaultNetwork() && entry.hasInternetAccess()) { - connectedEntry = entry; - } else { - wifiEntries.add(entry); + List wifiEntries = null; + final int accessPointsSize = (accessPoints == null) ? 0 : accessPoints.size(); + final boolean hasMoreWifiEntries = (accessPointsSize > MAX_WIFI_ENTRY_COUNT); + if (accessPointsSize > 0) { + wifiEntries = new ArrayList<>(); + final int count = hasMoreWifiEntries ? MAX_WIFI_ENTRY_COUNT : accessPointsSize; + for (int i = 0; i < count; i++) { + WifiEntry entry = accessPoints.get(i); + if (connectedEntry == null && entry.isDefaultNetwork() + && entry.hasInternetAccess()) { + connectedEntry = entry; + } else { + wifiEntries.add(entry); + } } + mHasWifiEntries = true; + } else { + mHasWifiEntries = false; } - mConnectedEntry = connectedEntry; - mWifiEntriesCount = wifiEntries.size(); - mCallback.onAccessPointsChanged(wifiEntries, mConnectedEntry, hasMoreWifiEntries); + mCallback.onAccessPointsChanged(wifiEntries, connectedEntry, hasMoreWifiEntries); } @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java index 47f6e5c420f92..42a4b7f4b01dc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java @@ -434,7 +434,6 @@ public class InternetDialogControllerTest extends SysuiTestCase { @Test public void onAccessPointsChanged_oneConnectedEntry_callbackConnectedEntryOnly() { reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); mAccessPoints.clear(); mAccessPoints.add(mConnectedEntry); @@ -448,7 +447,6 @@ public class InternetDialogControllerTest extends SysuiTestCase { @Test public void onAccessPointsChanged_noConnectedEntryAndOneOther_callbackWifiEntriesOnly() { reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); mAccessPoints.clear(); mAccessPoints.add(mWifiEntry1); @@ -463,7 +461,6 @@ public class InternetDialogControllerTest extends SysuiTestCase { @Test public void onAccessPointsChanged_oneConnectedEntryAndOneOther_callbackCorrectly() { reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); mAccessPoints.clear(); mAccessPoints.add(mConnectedEntry); mAccessPoints.add(mWifiEntry1); @@ -479,7 +476,6 @@ public class InternetDialogControllerTest extends SysuiTestCase { @Test public void onAccessPointsChanged_oneConnectedEntryAndTwoOthers_callbackCorrectly() { reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); mAccessPoints.clear(); mAccessPoints.add(mConnectedEntry); mAccessPoints.add(mWifiEntry1); @@ -497,7 +493,6 @@ public class InternetDialogControllerTest extends SysuiTestCase { @Test public void onAccessPointsChanged_oneConnectedEntryAndThreeOthers_callbackCutMore() { reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); mAccessPoints.clear(); mAccessPoints.add(mConnectedEntry); mAccessPoints.add(mWifiEntry1); @@ -509,86 +504,13 @@ public class InternetDialogControllerTest extends SysuiTestCase { mWifiEntries.clear(); mWifiEntries.add(mWifiEntry1); mWifiEntries.add(mWifiEntry2); - mWifiEntries.add(mWifiEntry3); - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, mConnectedEntry, - false /* hasMoreEntry */); - - // Turn off airplane mode to has carrier network, then Wi-Fi entries will cut last one. - reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(false); - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - mWifiEntries.remove(mWifiEntry3); verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, mConnectedEntry, true /* hasMoreEntry */); } - @Test - public void onAccessPointsChanged_oneConnectedEntryAndFourOthers_callbackCutMore() { - reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); - mAccessPoints.clear(); - mAccessPoints.add(mConnectedEntry); - mAccessPoints.add(mWifiEntry1); - mAccessPoints.add(mWifiEntry2); - mAccessPoints.add(mWifiEntry3); - mAccessPoints.add(mWifiEntry4); - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - mWifiEntries.clear(); - mWifiEntries.add(mWifiEntry1); - mWifiEntries.add(mWifiEntry2); - mWifiEntries.add(mWifiEntry3); - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, mConnectedEntry, - true /* hasMoreEntry */); - - // Turn off airplane mode to has carrier network, then Wi-Fi entries will cut last one. - reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(false); - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - mWifiEntries.remove(mWifiEntry3); - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, mConnectedEntry, - true /* hasMoreEntry */); - } - - @Test - public void onAccessPointsChanged_oneCarrierWifiAndFourOthers_callbackCutMore() { - reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); - when(mMergedCarrierEntry.isDefaultNetwork()).thenReturn(true); - mAccessPoints.clear(); - mAccessPoints.add(mWifiEntry1); - mAccessPoints.add(mWifiEntry2); - mAccessPoints.add(mWifiEntry3); - mAccessPoints.add(mWifiEntry4); - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - mWifiEntries.clear(); - mWifiEntries.add(mWifiEntry1); - mWifiEntries.add(mWifiEntry2); - mWifiEntries.add(mWifiEntry3); - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, - null /* connectedEntry */, true /* hasMoreEntry */); - - // Turn off airplane mode to has carrier WiFi, then Wi-Fi entries will keep the same. - reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(false); - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, - null /* connectedEntry */, true /* hasMoreEntry */); - } - @Test public void onAccessPointsChanged_fourWifiEntries_callbackCutMore() { reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(true); mAccessPoints.clear(); mAccessPoints.add(mWifiEntry1); mAccessPoints.add(mWifiEntry2); @@ -601,27 +523,6 @@ public class InternetDialogControllerTest extends SysuiTestCase { mWifiEntries.add(mWifiEntry1); mWifiEntries.add(mWifiEntry2); mWifiEntries.add(mWifiEntry3); - mWifiEntries.add(mWifiEntry4); - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, - null /* connectedEntry */, false /* hasMoreEntry */); - - // If the Ethernet exists, then Wi-Fi entries will cut last one. - reset(mInternetDialogCallback); - mInternetDialogController.mHasEthernet = true; - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - mWifiEntries.remove(mWifiEntry4); - verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, - null /* connectedEntry */, true /* hasMoreEntry */); - - // Turn off airplane mode to has carrier network, then Wi-Fi entries will cut last one. - reset(mInternetDialogCallback); - fakeAirplaneModeEnabled(false); - - mInternetDialogController.onAccessPointsChanged(mAccessPoints); - - mWifiEntries.remove(mWifiEntry3); verify(mInternetDialogCallback).onAccessPointsChanged(mWifiEntries, null /* connectedEntry */, true /* hasMoreEntry */); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java index a00ebcc8fd6b7..562d319076356 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java @@ -518,45 +518,46 @@ public class InternetDialogTest extends SysuiTestCase { @Test public void getWifiListMaxCount_returnCountCorrectly() { - // Ethernet, MobileData, ConnectedWiFi are all hidden. + // Both of the Ethernet, MobileData is hidden. // Then the maximum count is equal to MAX_WIFI_ENTRY_COUNT. setNetworkVisible(false, false, false); assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT); - // Only one of Ethernet, MobileData, ConnectedWiFi is displayed. - // Then the maximum count is equal to MAX_WIFI_ENTRY_COUNT - 1. - setNetworkVisible(true, false, false); - - assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 1); - - setNetworkVisible(false, true, false); - - assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 1); - + // If the Connected Wi-Fi is displayed then reduce one of the Wi-Fi list max count. setNetworkVisible(false, false, true); assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 1); - // Only one of Ethernet, MobileData, ConnectedWiFi is hidden. - // Then the maximum count is equal to MAX_WIFI_ENTRY_COUNT - 2. - setNetworkVisible(true, true, false); + // Only one of Ethernet, MobileData is displayed. + // Then the maximum count is equal to MAX_WIFI_ENTRY_COUNT. + setNetworkVisible(true, false, false); - assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 2); + assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT); + setNetworkVisible(false, true, false); + + assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT); + + // If the Connected Wi-Fi is displayed then reduce one of the Wi-Fi list max count. setNetworkVisible(true, false, true); - assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 2); + assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 1); setNetworkVisible(false, true, true); - assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 2); + assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 1); - // Ethernet, MobileData, ConnectedWiFi are all displayed. - // Then the maximum count is equal to MAX_WIFI_ENTRY_COUNT - 3. + // Both of Ethernet, MobileData, ConnectedWiFi is displayed. + // Then the maximum count is equal to MAX_WIFI_ENTRY_COUNT - 1. + setNetworkVisible(true, true, false); + + assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 1); + + // If the Connected Wi-Fi is displayed then reduce one of the Wi-Fi list max count. setNetworkVisible(true, true, true); - assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 3); + assertThat(mInternetDialog.getWifiListMaxCount()).isEqualTo(MAX_WIFI_ENTRY_COUNT - 2); } private void setNetworkVisible(boolean ethernetVisible, boolean mobileDataVisible, From 995b8190038c06353be94e30882a21b1210db9db Mon Sep 17 00:00:00 2001 From: Weng Su Date: Tue, 14 Dec 2021 03:46:20 +0800 Subject: [PATCH 3/3] [Provider Model] Listen for callback to highlight WiFi network - The Highlighting WiFi network refers to the WifiEntry#hasInternetAccess() function. - The WifiEntry#hasInternetAccess() might not be updated immediately when ConnectivityManager calls back a connected WiFi entry with Internet capabilities. - Need to listen the WifiEntry.onUpdated() callback to aware that WifiEntry#hasInternetAccess() has been updated correctly. - Also fixed a missing update for the WiFi max count issue - See b/208701968#comment4 for the result video. Bug: 208701968 Test: manual test atest -c InternetAdapterTest \ InternetDialogControllerTest \ InternetDialogTest Change-Id: I59be23643678c892f37ab8d4c7d2e33cb8c8f5b2 --- .../qs/tiles/dialog/InternetDialog.java | 2 +- .../dialog/InternetDialogController.java | 56 +++++++++++++++++++ .../dialog/InternetDialogControllerTest.java | 31 ++++++++++ .../qs/tiles/dialog/InternetDialogTest.java | 5 ++ 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java index d894c96dfa289..41ff56a0e7332 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialog.java @@ -466,9 +466,9 @@ public class InternetDialog extends SystemUIDialog implements } final int wifiListMaxCount = getWifiListMaxCount(); if (mAdapter.getItemCount() > wifiListMaxCount) { - mAdapter.setMaxEntriesCount(wifiListMaxCount); mHasMoreWifiEntries = true; } + mAdapter.setMaxEntriesCount(wifiListMaxCount); final int wifiListMinHeight = mWifiNetworkHeight * wifiListMaxCount; if (mWifiRecyclerView.getMinimumHeight() != wifiListMinHeight) { mWifiRecyclerView.setMinimumHeight(wifiListMinHeight); diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java index e01918ec717aa..0d064af9c17dc 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogController.java @@ -18,6 +18,7 @@ package com.android.systemui.qs.tiles.dialog; import static com.android.settingslib.mobile.MobileMappings.getIconKey; import static com.android.settingslib.mobile.MobileMappings.mapIconSets; +import static com.android.wifitrackerlib.WifiEntry.CONNECTED_STATE_CONNECTED; import android.animation.Animator; import android.animation.AnimatorListenerAdapter; @@ -176,6 +177,8 @@ public class InternetDialogController implements AccessPointController.AccessPoi protected KeyguardStateController mKeyguardStateController; @VisibleForTesting protected boolean mHasEthernet = false; + @VisibleForTesting + protected ConnectedWifiInternetMonitor mConnectedWifiInternetMonitor; private final KeyguardUpdateMonitorCallback mKeyguardUpdateCallback = new KeyguardUpdateMonitorCallback() { @@ -236,6 +239,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi mSignalDrawable = new SignalDrawable(mContext); mLocationController = locationController; mDialogLaunchAnimator = dialogLaunchAnimator; + mConnectedWifiInternetMonitor = new ConnectedWifiInternetMonitor(); } void onStart(@NonNull InternetDialogCallback callback, boolean canConfigWifi) { @@ -276,6 +280,7 @@ public class InternetDialogController implements AccessPointController.AccessPoi mAccessPointController.removeAccessPointCallback(this); mKeyguardUpdateMonitor.removeCallback(mKeyguardUpdateCallback); mConnectivityManager.unregisterNetworkCallback(mConnectivityManagerNetworkCallback); + mConnectedWifiInternetMonitor.unregisterCallback(); } @VisibleForTesting @@ -881,8 +886,10 @@ public class InternetDialogController implements AccessPointController.AccessPoi if (accessPointsSize > 0) { wifiEntries = new ArrayList<>(); final int count = hasMoreWifiEntries ? MAX_WIFI_ENTRY_COUNT : accessPointsSize; + mConnectedWifiInternetMonitor.unregisterCallback(); for (int i = 0; i < count; i++) { WifiEntry entry = accessPoints.get(i); + mConnectedWifiInternetMonitor.registerCallbackIfNeed(entry); if (connectedEntry == null && entry.isDefaultNetwork() && entry.hasInternetAccess()) { connectedEntry = entry; @@ -970,6 +977,55 @@ public class InternetDialogController implements AccessPointController.AccessPoi } } + /** + * Helper class for monitoring the Internet access of the connected WifiEntry. + */ + @VisibleForTesting + protected class ConnectedWifiInternetMonitor implements WifiEntry.WifiEntryCallback { + + private WifiEntry mWifiEntry; + + public void registerCallbackIfNeed(WifiEntry entry) { + if (entry == null || mWifiEntry != null) { + return; + } + // If the Wi-Fi is not connected yet, or it's the connected Wi-Fi with Internet + // access. Then we don't need to listen to the callback to update the Wi-Fi entries. + if (entry.getConnectedState() != CONNECTED_STATE_CONNECTED + || (entry.isDefaultNetwork() && entry.hasInternetAccess())) { + return; + } + mWifiEntry = entry; + entry.setListener(this); + } + + public void unregisterCallback() { + if (mWifiEntry == null) { + return; + } + mWifiEntry.setListener(null); + mWifiEntry = null; + } + + @MainThread + @Override + public void onUpdated() { + if (mWifiEntry == null) { + return; + } + WifiEntry entry = mWifiEntry; + if (entry.getConnectedState() != CONNECTED_STATE_CONNECTED) { + unregisterCallback(); + return; + } + if (entry.isDefaultNetwork() && entry.hasInternetAccess()) { + unregisterCallback(); + // Trigger onAccessPointsChanged() to update the Wi-Fi entries. + scanWifiAccessPoints(); + } + } + } + /** * Return {@code true} If the Ethernet exists */ diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java index 42a4b7f4b01dc..0d6554103dac0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogControllerTest.java @@ -544,6 +544,37 @@ public class InternetDialogControllerTest extends SysuiTestCase { null /* connectedEntry */, false /* hasMoreEntry */); } + @Test + public void onAccessPointsChanged_connectedWifiNoInternetAccess_shouldSetListener() { + reset(mWifiEntry1); + mAccessPoints.clear(); + when(mWifiEntry1.getConnectedState()).thenReturn(WifiEntry.CONNECTED_STATE_CONNECTED); + when(mWifiEntry1.isDefaultNetwork()).thenReturn(true); + when(mWifiEntry1.hasInternetAccess()).thenReturn(false); + mAccessPoints.add(mWifiEntry1); + + mInternetDialogController.onAccessPointsChanged(mAccessPoints); + + verify(mWifiEntry1).setListener(mInternetDialogController.mConnectedWifiInternetMonitor); + } + + @Test + public void onUpdated_connectedWifiHasInternetAccess_shouldScanWifiAccessPoints() { + reset(mAccessPointController); + when(mWifiEntry1.getConnectedState()).thenReturn(WifiEntry.CONNECTED_STATE_CONNECTED); + when(mWifiEntry1.isDefaultNetwork()).thenReturn(true); + when(mWifiEntry1.hasInternetAccess()).thenReturn(false); + InternetDialogController.ConnectedWifiInternetMonitor mConnectedWifiInternetMonitor = + mInternetDialogController.mConnectedWifiInternetMonitor; + mConnectedWifiInternetMonitor.registerCallbackIfNeed(mWifiEntry1); + + // When the hasInternetAccess() changed to true, and call back the onUpdated() function. + when(mWifiEntry1.hasInternetAccess()).thenReturn(true); + mConnectedWifiInternetMonitor.onUpdated(); + + verify(mAccessPointController).scanForAccessPoints(); + } + @Test public void setMergedCarrierWifiEnabledIfNeed_carrierProvisionsEnabled_doNothing() { when(mCarrierConfigTracker.getCarrierProvisionsWifiMergedNetworksBool(SUB_ID)) diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java index 562d319076356..c20e88708f406 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/dialog/InternetDialogTest.java @@ -312,6 +312,7 @@ public class InternetDialogTest extends SysuiTestCase { assertThat(mConnectedWifi.getVisibility()).isEqualTo(View.GONE); // Show a blank block to fix the dialog height even if there is no WiFi list assertThat(mWifiList.getVisibility()).isEqualTo(View.VISIBLE); + verify(mInternetAdapter).setMaxEntriesCount(3); assertThat(mSeeAll.getVisibility()).isEqualTo(View.INVISIBLE); } @@ -326,6 +327,7 @@ public class InternetDialogTest extends SysuiTestCase { assertThat(mConnectedWifi.getVisibility()).isEqualTo(View.GONE); // Show a blank block to fix the dialog height even if there is no WiFi list assertThat(mWifiList.getVisibility()).isEqualTo(View.VISIBLE); + verify(mInternetAdapter).setMaxEntriesCount(3); assertThat(mSeeAll.getVisibility()).isEqualTo(View.INVISIBLE); } @@ -339,6 +341,7 @@ public class InternetDialogTest extends SysuiTestCase { assertThat(mConnectedWifi.getVisibility()).isEqualTo(View.VISIBLE); // Show a blank block to fix the dialog height even if there is no WiFi list assertThat(mWifiList.getVisibility()).isEqualTo(View.VISIBLE); + verify(mInternetAdapter).setMaxEntriesCount(2); assertThat(mSeeAll.getVisibility()).isEqualTo(View.INVISIBLE); } @@ -353,6 +356,7 @@ public class InternetDialogTest extends SysuiTestCase { assertThat(mConnectedWifi.getVisibility()).isEqualTo(View.GONE); assertThat(mWifiList.getVisibility()).isEqualTo(View.VISIBLE); + verify(mInternetAdapter).setMaxEntriesCount(3); assertThat(mSeeAll.getVisibility()).isEqualTo(View.VISIBLE); } @@ -366,6 +370,7 @@ public class InternetDialogTest extends SysuiTestCase { assertThat(mConnectedWifi.getVisibility()).isEqualTo(View.VISIBLE); assertThat(mWifiList.getVisibility()).isEqualTo(View.VISIBLE); + verify(mInternetAdapter).setMaxEntriesCount(2); assertThat(mSeeAll.getVisibility()).isEqualTo(View.VISIBLE); }