Merge "UserDataPreparer: only delete contents of user's system CE and DE dirs"

services/core/java/com/android/server/pm/UserDataPreparer.java

@@ -150,14 +150,18 @@ class UserDataPreparer {
             if (Objects.equals(volumeUuid, StorageManager.UUID_PRIVATE_INTERNAL)) {
                 if ((flags & StorageManager.FLAG_STORAGE_DE) != 0) {
                     FileUtils.deleteContentsAndDir(getUserSystemDirectory(userId));
-                    FileUtils.deleteContentsAndDir(getDataSystemDeDirectory(userId));
+                    // Delete the contents of /data/system_de/$userId, but not the directory itself
+                    // since vold is responsible for that and system_server isn't allowed to do it.
+                    FileUtils.deleteContents(getDataSystemDeDirectory(userId));
                 }
                 if ((flags & StorageManager.FLAG_STORAGE_CE) != 0) {
-                    FileUtils.deleteContentsAndDir(getDataSystemCeDirectory(userId));
+                    // Likewise, delete the contents of /data/system_ce/$userId but not the
+                    // directory itself.
+                    FileUtils.deleteContents(getDataSystemCeDirectory(userId));
                 }
             }
 
-            // Data with special labels is now gone, so finish the job
+            // All the user's data directories should be empty now, so finish the job.
             storage.destroyUserStorage(volumeUuid, userId, flags);
 
         } catch (Exception e) {

services/tests/servicestests/src/com/android/server/pm/UserDataPreparerTest.java

@@ -17,6 +17,8 @@
 package com.android.server.pm;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.isNull;
 import static org.mockito.Mockito.verify;
@@ -129,22 +131,16 @@ public class UserDataPreparerTest {
     }
 
     @Test
-    public void testDestroyUserData() throws Exception {
+    public void testDestroyUserData_De_DoesNotDestroyCe() throws Exception {
-        // Add file in CE
+        // Add file in CE storage
         File systemCeDir = mUserDataPreparer.getDataSystemCeDirectory(TEST_USER_ID);
         systemCeDir.mkdirs();
         File ceFile = new File(systemCeDir, "file");
         writeFile(ceFile, "-----" );
-        testDestroyUserData_De();
+        // Destroy DE storage, then verify that CE storage wasn't destroyed too.
-        // CE directory should be preserved
+        mUserDataPreparer.destroyUserData(TEST_USER_ID, StorageManager.FLAG_STORAGE_DE);
         assertEquals(Collections.singletonList(ceFile), Arrays.asList(FileUtils.listFilesOrEmpty(
                 systemCeDir)));
-
-        testDestroyUserData_Ce();
-
-        // Verify that testDir is empty
-        assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(
-                mUserDataPreparer.testDir)));
     }
 
     @Test
@@ -163,7 +159,13 @@ public class UserDataPreparerTest {
         verify(mStorageManagerMock).destroyUserStorage(isNull(String.class), eq(TEST_USER_ID),
                         eq(StorageManager.FLAG_STORAGE_DE));
 
-        assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(systemDir)));
+        // systemDir (normal path: /data/system/users/$userId) should have been deleted.
+        assertFalse(systemDir.exists());
+        // systemDeDir (normal path: /data/system_de/$userId) should still exist but be empty, since
+        // UserDataPreparer itself is responsible for deleting the contents of this directory, but
+        // it delegates to StorageManager.destroyUserStorage() for deleting the directory itself.
+        // We've mocked out StorageManager, so StorageManager.destroyUserStorage() will be a no-op.
+        assertTrue(systemDeDir.exists());
         assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(
                 systemDeDir)));
     }
@@ -181,6 +183,11 @@ public class UserDataPreparerTest {
         verify(mStorageManagerMock).destroyUserStorage(isNull(String.class), eq(TEST_USER_ID),
                 eq(StorageManager.FLAG_STORAGE_CE));
 
+        // systemCeDir (normal path: /data/system_ce/$userId) should still exist but be empty, since
+        // UserDataPreparer itself is responsible for deleting the contents of this directory, but
+        // it delegates to StorageManager.destroyUserStorage() for deleting the directory itself.
+        // We've mocked out StorageManager, so StorageManager.destroyUserStorage() will be a no-op.
+        assertTrue(systemCeDir.exists());
         assertEquals(Collections.emptyList(), Arrays.asList(FileUtils.listFilesOrEmpty(
                 systemCeDir)));
     }