Merge "ONA: Track screen state in WifiNotificationController." into oc-dr1-dev

4 files changed, 59 insertions, 2 deletions

diff --git a/service/java/com/android/server/wifi/WifiConnectivityManager.java b/service/java/com/android/server/wifi/WifiConnectivityManager.java
index 13863c9bf..4a729e0c5 100644
--- a/service/java/com/android/server/wifi/WifiConnectivityManager.java
+++ b/service/java/com/android/server/wifi/WifiConnectivityManager.java
@@ -1034,6 +1034,8 @@ public class WifiConnectivityManager {
 
         mScreenOn = screenOn;
 
+        mWifiNotificationController.handleScreenStateChanged(screenOn);
+
         startConnectivityScan(SCAN_ON_SCHEDULE);
     }
 
diff --git a/service/java/com/android/server/wifi/WifiNotificationController.java b/service/java/com/android/server/wifi/WifiNotificationController.java
index 3558a8546..797fd438b 100644
--- a/service/java/com/android/server/wifi/WifiNotificationController.java
+++ b/service/java/com/android/server/wifi/WifiNotificationController.java
@@ -77,7 +77,8 @@ public class WifiNotificationController {
      * notification is not showing.
      */
     private boolean mNotificationShown;
-    /** Wi-Fi connection state from {@link WifiConnectivityManager} */
+    /** Whether the screen is on or not. */
+    private boolean mScreenOn;
 
     private final Context mContext;
     private FrameworkFacade mFrameworkFacade;
@@ -90,6 +91,8 @@ public class WifiNotificationController {
         mFrameworkFacade = framework;
         mNotificationBuilder = builder;
 
+        mScreenOn = false;
+
         // Setting is in seconds
         NOTIFICATION_REPEAT_DELAY_MS = mFrameworkFacade.getIntegerSetting(context,
                 Settings.Global.WIFI_NETWORKS_AVAILABLE_REPEAT_DELAY, 900) * 1000L;
@@ -130,13 +133,23 @@ public class WifiNotificationController {
             clearPendingNotification(false /* resetRepeatDelay */);
             return;
         }
-        if (mNotificationShown) {
+
+        // Do not show or update the notification if screen is off. We want to avoid a race that
+        // could occur between a user picking a network in settings and a network candidate picked
+        // through network selection, which will happen because screen on triggers a new
+        // connectivity scan.
+        if (mNotificationShown || !mScreenOn) {
             return;
         }
 
         setNotificationVisible(true, availableNetworks.size(), false, 0);
     }
 
+    /** Handles screen state changes. */
+    public void handleScreenStateChanged(boolean screenOn) {
+        mScreenOn = screenOn;
+    }
+
     /**
      * Display or don't display a notification that there are open Wi-Fi networks.
      * @param visible {@code true} if notification should be visible, {@code false} otherwise
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
index 11e220e7f..bdb6b14ed 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
@@ -679,6 +679,20 @@ public class WifiConnectivityManagerTest {
     }
 
     /**
+     * Verify that the ONA controller tracks screen state changes.
+     */
+    @Test
+    public void openNetworkNotificationControllerTracksScreenStateChanges() {
+        mWifiConnectivityManager.handleScreenStateChanged(false);
+
+        verify(mWifiNotificationController).handleScreenStateChanged(false);
+
+        mWifiConnectivityManager.handleScreenStateChanged(true);
+
+        verify(mWifiNotificationController).handleScreenStateChanged(true);
+    }
+
+    /**
      *  Verify that scan interval for screen on and wifi disconnected scenario
      *  is in the exponential backoff fashion.
      *
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNotificationControllerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNotificationControllerTest.java
index 9aa5ee938..27055a885 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNotificationControllerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNotificationControllerTest.java
@@ -70,6 +70,7 @@ public class WifiNotificationControllerTest {
         mNotificationController = new WifiNotificationController(
                 mContext, mock_looper.getLooper(), mFrameworkFacade,
                 mock(Notification.Builder.class));
+        mNotificationController.handleScreenStateChanged(true);
     }
 
     private List<ScanDetail> createOpenScanResults() {
@@ -114,6 +115,21 @@ public class WifiNotificationControllerTest {
 
         verify(mNotificationManager).cancelAsUser(any(), anyInt(), any());
     }
+    /**
+     * When a notification is showing, screen is off, and scan results with no open networks are
+     * handled, the notification is cleared.
+     */
+    @Test
+    public void handleScanResults_notificationShown_screenOff_emptyList_notificationCleared() {
+        mNotificationController.handleScanResults(createOpenScanResults());
+
+        verify(mNotificationManager).notifyAsUser(any(), anyInt(), any(), any());
+
+        mNotificationController.handleScreenStateChanged(false);
+        mNotificationController.handleScanResults(new ArrayList<>());
+
+        verify(mNotificationManager).cancelAsUser(any(), anyInt(), any());
+    }
 
     /**
      * If notification is showing, do not post another notification.
@@ -152,6 +168,18 @@ public class WifiNotificationControllerTest {
         verify(mNotificationManager, never()).cancelAsUser(any(), anyInt(), any());
     }
 
+    /**
+     * When screen is off and notification is not displayed, notification is not posted on handling
+     * new scan results with open networks.
+     */
+    @Test
+    public void screenOff_handleScanResults_notificationNotDisplayed() {
+        mNotificationController.handleScreenStateChanged(false);
+        mNotificationController.handleScanResults(createOpenScanResults());
+
+        verify(mNotificationManager, never()).notifyAsUser(any(), anyInt(), any(), any());
+    }
+
     /** Verifies that {@link UserManager#DISALLOW_CONFIG_WIFI} disables the feature. */
     @Test
     public void userHasDisallowConfigWifiRestriction_notificationNotDisplayed() {