From e869d83ea8558e70682c4c42674097040c7eb966 Mon Sep 17 00:00:00 2001 From: Sundeep Ghuman Date: Wed, 25 Jan 2017 16:23:43 -0800 Subject: [PATCH] Create Settings Flags to Disable Scoring UI changes. When disabled, this will prevent badges from being shown in the status bar or wifi picker as well as prevent access points in the picker from being rearranged based on ranking scores. Fix missing permission dropped from previous CL to run NetworkControllerWifiTest and refactored tests to enable new setting. Bug: 34712533 Test: runtest --path frameworks/base/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java and runtest --path frameworks/base/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java Change-Id: I79c97f2205ebb70c0f7f5b1f66f7207055e5769b --- core/java/android/provider/Settings.java | 8 ++ .../android/settingslib/wifi/WifiTracker.java | 36 +++++++-- .../settingslib/wifi/WifiTrackerTest.java | 74 ++++++++++++++++--- .../policy/WifiSignalController.java | 25 ++++--- packages/SystemUI/tests/AndroidManifest.xml | 1 + .../policy/NetworkControllerWifiTest.java | 37 +++++----- 6 files changed, 137 insertions(+), 44 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index fd8b2cafc8744..7ebff5255e273 100755 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -8171,6 +8171,14 @@ public final class Settings { @SystemApi public static final String WIFI_WAKEUP_ENABLED = "wifi_wakeup_enabled"; + /** + * Value to specify whether network quality scores and badging should be shown in the UI. + * + * Type: int (0 for false, 1 for true) + * @hide + */ + public static final String NETWORK_SCORING_UI_ENABLED = "network_scoring_ui_enabled"; + /** * Value to specify if network recommendations from * {@link com.android.server.NetworkScoreService} are enabled. diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java index 799f388057c14..c6179943b4d31 100644 --- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java +++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java @@ -16,9 +16,11 @@ package com.android.settingslib.wifi; import android.content.BroadcastReceiver; +import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.database.ContentObserver; import android.net.ConnectivityManager; import android.net.Network; import android.net.NetworkCapabilities; @@ -38,6 +40,7 @@ import android.os.ConditionVariable; import android.os.Handler; import android.os.Looper; import android.os.Message; +import android.provider.Settings; import android.support.annotation.WorkerThread; import android.util.ArraySet; import android.util.Log; @@ -136,6 +139,8 @@ public class WifiTracker { private final NetworkScoreManager mNetworkScoreManager; private final WifiNetworkScoreCache mScoreCache; private final Set mRequestedScores = new ArraySet<>(); + private boolean mNetworkScoringUiEnabled; + private final ContentObserver mObserver; @VisibleForTesting Scanner mScanner; @@ -215,6 +220,16 @@ public class WifiTracker { Message.obtain(mWorkHandler, WorkHandler.MSG_UPDATE_NETWORK_SCORES).sendToTarget(); } }); + + mObserver = new ContentObserver(mWorkHandler) { + @Override + public void onChange(boolean selfChange) { + mNetworkScoringUiEnabled = + Settings.Global.getInt( + mContext.getContentResolver(), + Settings.Global.NETWORK_SCORING_UI_ENABLED, 0) == 1; + } + }; } /** @@ -274,6 +289,11 @@ public class WifiTracker { } }); + mContext.getContentResolver().registerContentObserver( + Settings.Global.getUriFor(Settings.Global.NETWORK_SCORING_UI_ENABLED), + false /* notifyForDescendants */, + mObserver); + mObserver.onChange(false /* selfChange */); // Set the initial value for mScoringUiEnabled resumeScanning(); if (!mRegistered) { @@ -327,6 +347,7 @@ public class WifiTracker { unregisterAndClearScoreCache(); } }); + mContext.getContentResolver().unregisterContentObserver(mObserver); } @WorkerThread @@ -537,10 +558,11 @@ public class WifiTracker { } } - - requestScoresForNetworkKeys(scoresToRequest); - for (AccessPoint ap : accessPoints) { - ap.updateScores(mScoreCache); + if (mNetworkScoringUiEnabled) { + requestScoresForNetworkKeys(scoresToRequest); + for (AccessPoint ap : accessPoints) { + ap.updateScores(mScoreCache); + } } // Pre-sort accessPoints to speed preference insertion @@ -641,7 +663,7 @@ public class WifiTracker { if (ap.update(connectionConfig, mLastInfo, mLastNetworkInfo)) { reorder = true; } - if (ap.updateScores(mScoreCache)) { + if (mNetworkScoringUiEnabled && ap.updateScores(mScoreCache)) { reorder = true; } } @@ -657,6 +679,10 @@ public class WifiTracker { *

Will trigger a resort and notify listeners of changes if applicable. */ private void updateNetworkScores() { + if (!mNetworkScoringUiEnabled) { + return; + } + // Lock required to prevent accidental copying of AccessPoint states while the modification // is in progress. see #copyAndNotifyListeners long before = System.currentTimeMillis(); diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java index eaf0367822c89..08736c77c409a 100644 --- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java +++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/wifi/WifiTrackerTest.java @@ -110,7 +110,7 @@ public class WifiTrackerTest { private HandlerThread mWorkerThread; private Looper mLooper; private Looper mMainLooper; - private int mOriginalSettingValue; + private int mOriginalScoringUiSettingValue; @Before public void setUp() { @@ -175,19 +175,23 @@ public class WifiTrackerTest { } }).when(mockWifiListener).onAccessPointsChanged(); - mOriginalSettingValue = Settings.Global.getInt( - InstrumentationRegistry.getTargetContext().getContentResolver(), - Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, - 0 /* disabled */); - + // Turn on Scoring UI features + mOriginalScoringUiSettingValue = Settings.Global.getInt( + InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.NETWORK_SCORING_UI_ENABLED, + 0 /* disabled */); + Settings.Global.putInt( + InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.NETWORK_SCORING_UI_ENABLED, + 1 /* enabled */); } @After public void cleanUp() { Settings.Global.putInt( - InstrumentationRegistry.getTargetContext().getContentResolver(), - Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, - mOriginalSettingValue); + InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.NETWORK_SCORING_UI_ENABLED, + mOriginalScoringUiSettingValue); } private static ScanResult buildScanResult1() { @@ -333,9 +337,18 @@ public class WifiTrackerTest { WifiNetworkScoreCache scoreCache = mScoreCacheCaptor.getValue(); + CountDownLatch latch = new CountDownLatch(1); + doAnswer( + (invocation) -> { + latch.countDown(); + return null; + }).when(mockNetworkScoreManager) + .unregisterNetworkScoreCache(NetworkKey.TYPE_WIFI, scoreCache); + // Test unregister tracker.stopTracking(); + latch.await(LATCH_TIMEOUT, TimeUnit.MILLISECONDS); verify(mockNetworkScoreManager) .unregisterNetworkScoreCache(NetworkKey.TYPE_WIFI, scoreCache); } @@ -385,7 +398,28 @@ public class WifiTrackerTest { assertTrue(aps.size() == 2); assertEquals(aps.get(0).getSsidStr(), SSID_2); assertEquals(aps.get(1).getSsidStr(), SSID_1); + } + @Test + public void scoreCacheUpdateScoresShouldNotChangeSortOrderWhenSortingDisabled() + throws InterruptedException { + Settings.Global.putInt( + InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.NETWORK_SCORING_UI_ENABLED, + 0 /* disabled */); + + WifiTracker tracker = createTrackerAndInjectInitialScanResults(); + List aps = tracker.getAccessPoints(); + assertTrue(aps.size() == 2); + assertEquals(aps.get(0).getSsidStr(), SSID_1); + assertEquals(aps.get(1).getSsidStr(), SSID_2); + + updateScoresAndWaitForAccessPointsChangedCallback(); + + aps = tracker.getAccessPoints(); + assertTrue(aps.size() == 2); + assertEquals(aps.get(0).getSsidStr(), SSID_1); + assertEquals(aps.get(1).getSsidStr(), SSID_2); } @Test @@ -404,6 +438,28 @@ public class WifiTrackerTest { } } + @Test + public void noBadgesShouldBeInsertedIntoAccessPointWhenScoringUiDisabled() + throws InterruptedException { + Settings.Global.putInt( + InstrumentationRegistry.getTargetContext().getContentResolver(), + Settings.Global.NETWORK_SCORING_UI_ENABLED, + 0 /* disabled */); + + WifiTracker tracker = createTrackerAndInjectInitialScanResults(); + updateScoresAndWaitForAccessPointsChangedCallback(); + + List aps = tracker.getAccessPoints(); + + for (AccessPoint ap : aps) { + if (ap.getSsidStr().equals(SSID_1)) { + assertEquals(ScoredNetwork.BADGING_NONE, ap.getBadge()); + } else if (ap.getSsidStr().equals(SSID_2)) { + assertEquals(ScoredNetwork.BADGING_NONE, ap.getBadge()); + } + } + } + @Test public void scoresShouldBeRequestedForNewScanResultOnly() throws InterruptedException { mRequestScoresLatch = new CountDownLatch(2); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/WifiSignalController.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/WifiSignalController.java index 42c20ff6e2efd..886b8bebac818 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/WifiSignalController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/WifiSignalController.java @@ -56,7 +56,7 @@ public class WifiSignalController extends private final WifiNetworkScoreCache mScoreCache; private final WifiStatusTracker mWifiTracker; - private boolean mScoringEnabled = false; + private boolean mScoringUiEnabled = false; public WifiSignalController(Context context, boolean hasMobileData, CallbackHandler callbackHandler, NetworkControllerImpl networkController, @@ -95,25 +95,26 @@ public class WifiSignalController extends // Setup scoring mNetworkScoreManager = networkScoreManager; + configureScoringGating(); + registerScoreCache(); + } + + private void configureScoringGating() { ContentObserver observer = new ContentObserver(new Handler(Looper.getMainLooper())) { @Override public void onChange(boolean selfChange) { - mScoringEnabled = + mScoringUiEnabled = Settings.Global.getInt( mContext.getContentResolver(), - Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, 0) == 1; - if (!mScoringEnabled) { - mScoreCache.clearScores(); - } + Settings.Global.NETWORK_SCORING_UI_ENABLED, 0) == 1; } }; - ContentResolver cr = mContext.getContentResolver(); - cr.registerContentObserver( - Settings.Global.getUriFor(Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED), + mContext.getContentResolver().registerContentObserver( + Settings.Global.getUriFor(Settings.Global.NETWORK_SCORING_UI_ENABLED), false /* notifyForDescendants */, observer); - observer.onChange(false /* selfChange */); - registerScoreCache(); + + observer.onChange(false /* selfChange */); // Set the initial values } private void registerScoreCache() { @@ -199,7 +200,7 @@ public class WifiSignalController extends *

{@link #updateScoreCacheIfNecessary} should be called prior to this method. */ private int getWifiBadgeEnum() { - if (!mScoringEnabled || mWifiTracker.networkKey == null) { + if (!mScoringUiEnabled || mWifiTracker.networkKey == null) { return ScoredNetwork.BADGING_NONE; } ScoredNetwork score = mScoreCache.getScoredNetwork(mWifiTracker.networkKey); diff --git a/packages/SystemUI/tests/AndroidManifest.xml b/packages/SystemUI/tests/AndroidManifest.xml index 90e93217485d3..408e8f3298af9 100644 --- a/packages/SystemUI/tests/AndroidManifest.xml +++ b/packages/SystemUI/tests/AndroidManifest.xml @@ -33,6 +33,7 @@ + diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java index 06a512297282e..398940140c40d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerWifiTest.java @@ -1,7 +1,6 @@ package com.android.systemui.statusbar.policy; import android.content.Intent; -import android.graphics.drawable.Drawable; import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkKey; @@ -30,7 +29,6 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertNotNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; @@ -60,8 +58,6 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { private final List mRequestedKeys = new ArrayList<>(); private CountDownLatch mRequestScoresLatch; - private SettingOverrider mSettingsOverrider; - @Test public void testWifiIcon() { String testSsid = "Test SSID"; @@ -83,6 +79,7 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { @Test public void testBadgedWifiIcon() throws Exception { + // TODO(sghuman): Refactor this setup code when creating a test for the badged QsIcon. int testLevel = 1; RssiCurve mockBadgeCurve = mock(RssiCurve.class); Bundle attr = new Bundle(); @@ -94,12 +91,16 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { false /* meteredHint */, attr); - // Enable scoring - mSettingsOverrider = mContext.getSettingsProvider().acquireOverridesBuilder(this) - .addSetting("global", Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, "1") - .build(); - + // Must set the Settings value before instantiating the NetworkControllerImpl due to bugs in + // FakeSettingsProvider. + SettingOverrider settingsOverrider = + mContext.getSettingsProvider().acquireOverridesBuilder(this) + .addSetting("global", Settings.Global.NETWORK_SCORING_UI_ENABLED, "1") + .build(); + super.setUp(); // re-instantiate NetworkControllImpl now that setting has been updated setupNetworkScoreManager(); + + // Test Requesting Scores mRequestScoresLatch = new CountDownLatch(1); setWifiEnabled(true); setWifiState(true, TEST_SSID, TEST_BSSID); @@ -115,23 +116,23 @@ public class NetworkControllerWifiTest extends NetworkControllerBaseTest { Matchers.anyInt()); scoreCacheCaptor.getValue().updateScores(Arrays.asList(score)); + // Test badge is set setWifiLevel(testLevel); - NetworkController.SignalCallback mockCallback = - mock(NetworkController.SignalCallback.class); - mNetworkController.addCallback(mockCallback); - ArgumentCaptor iconState = ArgumentCaptor.forClass(IconState.class); - Mockito.verify(mockCallback).setWifiIndicators( - anyBoolean(), iconState.capture(), any(), anyBoolean(), anyBoolean(), any()); + ArgumentCaptor iconArg = ArgumentCaptor.forClass(IconState.class); + Mockito.verify(mCallbackHandler, Mockito.atLeastOnce()).setWifiIndicators( + anyBoolean(), iconArg.capture(), any(), anyBoolean(), anyBoolean(), + any()); + IconState iconState = iconArg.getValue(); assertEquals("Badged Wifi Resource is set", Utils.WIFI_PIE_FOR_BADGING[testLevel], - iconState.getValue().icon); + iconState.icon); assertEquals("SD Badge is set", Utils.getWifiBadgeResource(ScoredNetwork.BADGING_SD), - iconState.getValue().iconOverlay); + iconState.iconOverlay); - mSettingsOverrider.release(); + settingsOverrider.release(); } private void setupNetworkScoreManager() {