Merge "Store pending 'history disabled' cmds"

services/core/java/com/android/server/notification/NotificationHistoryManager.java

@@ -68,6 +68,8 @@ public class NotificationHistoryManager {
     private final SparseArray<List<String>> mUserPendingPackageRemovals = new SparseArray<>();
     @GuardedBy("mLock")
     private final SparseBooleanArray mHistoryEnabled = new SparseBooleanArray();
+    @GuardedBy("mLock")
+    private final SparseBooleanArray mUserPendingHistoryDisables = new SparseBooleanArray();
 
     public NotificationHistoryManager(Context context, Handler handler) {
         mContext = context;
@@ -75,6 +77,11 @@ public class NotificationHistoryManager {
         mSettingsObserver = new SettingsObserver(handler);
     }
 
+    @VisibleForTesting
+    void onDestroy() {
+        mSettingsObserver.stopObserving();
+    }
+
     void onBootPhaseAppsCanStart() {
         mSettingsObserver.observe();
     }
@@ -99,8 +106,8 @@ public class NotificationHistoryManager {
             }
 
             // delete history if it was disabled when the user was locked
-            if (!mHistoryEnabled.get(userId)) {
-                userHistory.disableHistory();
+            if (mUserPendingHistoryDisables.get(userId)) {
+                disableHistory(userHistory, userId);
             }
         }
     }
@@ -118,6 +125,7 @@ public class NotificationHistoryManager {
             // removed) - we just need clean up our internal state for GC
             mUserPendingPackageRemovals.put(userId, null);
             mHistoryEnabled.put(userId, false);
+            mUserPendingHistoryDisables.put(userId, false);
             onUserStopped(userId);
         }
     }
@@ -213,20 +221,29 @@ public class NotificationHistoryManager {
 
     void onHistoryEnabledChanged(@UserIdInt int userId, boolean historyEnabled) {
         synchronized (mLock) {
-            mHistoryEnabled.put(userId, historyEnabled);
-
-            // These requests might fail if the user is locked; onUserUnlocked will pick up those
-            // cases
+            if (historyEnabled) {
+                mHistoryEnabled.put(userId, historyEnabled);
+            }
             final NotificationHistoryDatabase userHistory =
                     getUserHistoryAndInitializeIfNeededLocked(userId);
             if (userHistory != null) {
                 if (!historyEnabled) {
-                    userHistory.disableHistory();
+                    disableHistory(userHistory, userId);
                 }
+            } else {
+                mUserPendingHistoryDisables.put(userId, !historyEnabled);
             }
         }
     }
 
+    private void disableHistory(NotificationHistoryDatabase userHistory, @UserIdInt int userId) {
+        userHistory.disableHistory();
+
+        mUserPendingHistoryDisables.put(userId, false);
+        mHistoryEnabled.put(userId, false);
+        mUserState.put(userId, null);
+    }
+
     @GuardedBy("mLock")
     private @Nullable NotificationHistoryDatabase getUserHistoryAndInitializeIfNeededLocked(
             int userId) {
@@ -316,6 +333,11 @@ public class NotificationHistoryManager {
             }
         }
 
+        void stopObserving() {
+            ContentResolver resolver = mContext.getContentResolver();
+            resolver.unregisterContentObserver(this);
+        }
+
         @Override
         public void onChange(boolean selfChange, Uri uri, int userId) {
             update(uri, userId);

services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryManagerTest.java

@@ -40,8 +40,8 @@ import androidx.test.runner.AndroidJUnit4;
 
 import com.android.server.UiServiceTestCase;
 
+import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
@@ -115,6 +115,11 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase {
         mHistoryManager.onBootPhaseAppsCanStart();
     }
 
+    @After
+    public void tearDown() {
+        mHistoryManager.onDestroy();
+    }
+
     @Test
     public void testOnUserUnlocked() {
         assertThat(mHistoryManager.doesHistoryExistForUser(USER_SYSTEM)).isFalse();
@@ -126,21 +131,51 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase {
     }
 
     @Test
-    @Ignore("b/147012298")
     public void testOnUserUnlocked_historyDisabled() {
+        // create a history
+        mHistoryManager.onUserUnlocked(USER_SYSTEM);
+        assertThat(mHistoryManager.doesHistoryExistForUser(USER_SYSTEM)).isTrue();
+        // lock user
+        mHistoryManager.onUserStopped(USER_SYSTEM);
+
+        // turn off history
         Settings.Secure.putIntForUser(getContext().getContentResolver(),
                 Settings.Secure.NOTIFICATION_HISTORY_ENABLED, 0, USER_SYSTEM);
         mHistoryManager.mSettingsObserver.update(null, USER_SYSTEM);
-        assertThat(mHistoryManager.doesHistoryExistForUser(USER_SYSTEM)).isFalse();
-        assertThat(mHistoryManager.isUserUnlocked(USER_SYSTEM)).isFalse();
 
+        // unlock user, verify that history is disabled
         mHistoryManager.onUserUnlocked(USER_SYSTEM);
 
         assertThat(mHistoryManager.doesHistoryExistForUser(USER_SYSTEM)).isFalse();
-        assertThat(mHistoryManager.isUserUnlocked(USER_SYSTEM)).isFalse();
         verify(mDb, times(1)).disableHistory();
     }
 
+    @Test
+    public void testOnUserUnlocked_historyDisabledThenEnabled() {
+        // create a history
+        mHistoryManager.onUserUnlocked(USER_SYSTEM);
+        assertThat(mHistoryManager.doesHistoryExistForUser(USER_SYSTEM)).isTrue();
+
+        // lock user
+        mHistoryManager.onUserStopped(USER_SYSTEM);
+
+        // turn off history
+        Settings.Secure.putIntForUser(getContext().getContentResolver(),
+                Settings.Secure.NOTIFICATION_HISTORY_ENABLED, 0, USER_SYSTEM);
+        mHistoryManager.mSettingsObserver.update(null, USER_SYSTEM);
+
+        // turn on history
+        Settings.Secure.putIntForUser(getContext().getContentResolver(),
+                Settings.Secure.NOTIFICATION_HISTORY_ENABLED, 1, USER_SYSTEM);
+        mHistoryManager.mSettingsObserver.update(null, USER_SYSTEM);
+
+        // unlock user, verify that history is NOT disabled
+        mHistoryManager.onUserUnlocked(USER_SYSTEM);
+
+        assertThat(mHistoryManager.doesHistoryExistForUser(USER_SYSTEM)).isTrue();
+        verify(mDb, never()).disableHistory();
+    }
+
     @Test
     public void testOnUserUnlocked_cleansUpRemovedPackages() {
         String pkg = "pkg";
@@ -223,6 +258,8 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase {
 
     @Test
     public void testOnPackageRemoved_historyDisabled() {
+        mHistoryManager.onUserUnlocked(USER_SYSTEM);
+
         Settings.Secure.putIntForUser(getContext().getContentResolver(),
                 Settings.Secure.NOTIFICATION_HISTORY_ENABLED, 0, USER_SYSTEM);
         mHistoryManager.mSettingsObserver.update(null, USER_SYSTEM);
@@ -427,6 +464,8 @@ public class NotificationHistoryManagerTest extends UiServiceTestCase {
     public void testIsHistoryEnabled() {
         assertThat(mHistoryManager.isHistoryEnabled(USER_SYSTEM)).isTrue();
 
+        mHistoryManager.onUserUnlocked(USER_SYSTEM);
+
         Settings.Secure.putIntForUser(getContext().getContentResolver(),
                 Settings.Secure.NOTIFICATION_HISTORY_ENABLED, 0,  USER_SYSTEM);
         mHistoryManager.mSettingsObserver.update(null, USER_SYSTEM);