Fix up _some_web app directory StrictMode violations

chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java
index db8d63297c52aa8182d19c8f5890b80ab91a8144..f2907ba381926637f4f8765840afd76e0ec53340 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java
@@ -15,114 +15,72 @@ import android.os.AsyncTask;
 import android.os.Build;
 import android.os.StrictMode;
 import android.text.TextUtils;
-import android.util.Log;
 
 import org.chromium.base.ApplicationStatus;
+import org.chromium.base.FileUtilities;
+import org.chromium.base.Log;
 import org.chromium.chrome.browser.document.DocumentUtils;
 
 import java.io.File;
 import java.util.HashSet;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
- * Manages directories created to store data for webapps.
+ * Manages directories created to store data for web apps.
+ *
+ * Running this task should be called by WebappActivities after they have restored all the data
+ * they need from their directory.
  *
  * Directories managed by this class are all subdirectories of the app_WebappActivity/ directory,
  * which each WebappActivity using a directory named either for its Webapp's ID in Document mode,
  * or the index of the WebappActivity if it is a subclass of the WebappManagedActivity class (which
  * are used in pre-L devices to allow multiple WebappActivities launching).
  */
-public class WebappDirectoryManager extends AsyncTask<Void, Void, Void> {
-    private static final String TAG = "WebappDirectoryCleaner";
-    private static final String WEBAPP_DIRECTORY_NAME = "WebappActivity";
+public class WebappDirectoryManager extends AsyncTask<WebappActivity, Void, Void> {
+    protected static final String WEBAPP_DIRECTORY_NAME = "WebappActivity";
+    private static final String TAG = "WebappDirectoryManag";
 
     /** Whether or not the class has already started trying to clean up obsolete directories. */
     private static final AtomicBoolean sMustCleanUpOldDirectories = new AtomicBoolean(true);
 
-    /** Scheme used for Intents fired for WebappActivity instances. */
-    private final String mWebappScheme;
-
-    /** Directories that will be deleted. */
-    private final HashSet<File> mDirectoriesToDelete;
-
-    /**
-     * Constructs a WebappDirectoryManager, which will manage the deletion of directories
-     * corresponding to webapps that no longer need their data.
-     *
-     * Should be called by WebappActivities after they have restored all the data they need from
-     * their directory.
-     *
-     * @param directory Directory that must be deleted.  Corresponds to the current webapp.
-     * @param webappScheme Scheme used for WebappActivities when building their Intent data URIs.
-     * @param deleteOldDirectories Whether directories for old WebappActivities should be purged.
-     */
-    public WebappDirectoryManager(
-            final File directory, String webappScheme, boolean deleteOldDirectories) {
-        mWebappScheme = webappScheme;
-
-        mDirectoriesToDelete = new HashSet<File>();
-        mDirectoriesToDelete.add(directory);
+    @Override
+    protected final Void doInBackground(WebappActivity... params) {
+        Set<File> directoriesToDelete = new HashSet<File>();
+        if (params.length == 1 && params[0] != null) {
+            directoriesToDelete.add(getWebappDirectory(params[0]));
+        }
 
-        if (deleteOldDirectories && sMustCleanUpOldDirectories.getAndSet(false)) {
+        boolean shouldDeleteOldDirectories = Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;
+        if (shouldDeleteOldDirectories && sMustCleanUpOldDirectories.getAndSet(false)) {
             assert Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;

[Peter Wen, 2015/09/22 17:16:31]

Is this assert useful since the boolean right above it is the exact same value?

[gone, 2015/09/22 18:49:56]

Yeah, not as useful as it used to be.  Removing.

-            Context context = ApplicationStatus.getApplicationContext();
-            cleanUpOldWebappDirectories(
-                    mDirectoriesToDelete, context.getApplicationInfo().dataDir);
+            cleanUpOldWebappDirectories(directoriesToDelete, getBaseDataDirectory());
         }
-    }
 
-    @Override
-    protected Void doInBackground(Void... params) {
-        for (File directory : mDirectoriesToDelete) {
+        for (File directory : directoriesToDelete) {
             if (isCancelled()) return null;
-
-            File[] files = directory.listFiles();
-            if (files != null) {
-                // Delete all the files in the directory.
-                for (File file : files) {
-                    if (!file.delete()) Log.e(TAG, "Failed to delete file: " + file.getPath());
-                }
-            }
-
-            // Delete the directory itself.
-            if (!directory.delete()) {
-                Log.e(TAG, "Failed to delete directory: " + directory.getPath());
-            }
+            FileUtilities.recursivelyDeleteFile(directory);
         }
+
         return null;
     }
 
     /**
-     * Removes all directories using the old pre-K directory structure, which used directories named
+     * Removes all directories using the old pre-L directory structure, which used directories named
      * app_WebappActivity*.  Also deletes directories corresponding to WebappActivities that are no
      * longer listed in Android's recents, since these will be unable to restore their data, anyway.
      * @param directoriesToDelete Set to append directory names to.
      * @param baseDirectory Base directory of all of Chrome's persisted files.
      */
-    @TargetApi(Build.VERSION_CODES.LOLLIPOP)
-    private void cleanUpOldWebappDirectories(
-            HashSet<File> directoriesToDelete, String baseDirectory) {
-        Context context = ApplicationStatus.getApplicationContext();
-        File webappBaseDirectory = null;
-
-        // Temporarily allowing disk access while fixing. TODO: http://crbug.com/525781
-        StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
-        try {
-            webappBaseDirectory = context.getDir(WEBAPP_DIRECTORY_NAME, Context.MODE_PRIVATE);
-        } finally {
-            StrictMode.setThreadPolicy(oldPolicy);
-        }
+    private void cleanUpOldWebappDirectories(Set<File> directoriesToDelete, String baseDirectory) {
+        File webappBaseDirectory = getBaseWebappDirectory();
 
         // Figure out what WebappActivities are still listed in Android's recents menu.
-        HashSet<String> liveWebapps = new HashSet<String>();
-        ActivityManager manager =
-                (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE);
-        for (AppTask task : manager.getAppTasks()) {
-            Intent intent = DocumentUtils.getBaseIntentFromTask(task);
-            if (intent == null) continue;
-
+        Set<String> liveWebapps = new HashSet<String>();
+        Set<Intent> baseIntents = getBaseIntentsForAllTasks();
+        for (Intent intent : baseIntents) {
             Uri data = intent.getData();
-            if (data != null && TextUtils.equals(mWebappScheme, data.getScheme())) {
+            if (data != null && TextUtils.equals(WebappActivity.WEBAPP_SCHEME, data.getScheme())) {
                 liveWebapps.add(data.getHost());
             }
 
@@ -143,7 +101,7 @@ public class WebappDirectoryManager extends AsyncTask<Void, Void, Void> {
         }
 
         if (webappBaseDirectory != null) {
-            // Delete all webapp directories in the main directory.
+            // Delete all web app directories in the main directory.
             String webappDirectoryAppBaseName = webappBaseDirectory.getName();
             File dataDirectory = new File(baseDirectory);
             File[] files = dataDirectory.listFiles();
@@ -156,7 +114,7 @@ public class WebappDirectoryManager extends AsyncTask<Void, Void, Void> {
                 }
             }
 
-            // Clean out webapp directories no longer corresponding to tasks in Recents.
+            // Clean out web app directories no longer corresponding to tasks in Recents.
             if (webappBaseDirectory.exists()) {
                 files = webappBaseDirectory.listFiles();
                 if (files != null) {
@@ -169,9 +127,9 @@ public class WebappDirectoryManager extends AsyncTask<Void, Void, Void> {
     }
 
     /**
-     * Returns the name of the directory for this webapp.
-     * @param identifier ID for the webapp.  Used as a subdirectory name.
-     * @return File for storing information about the webapp.
+     * Returns the directory for a web app, creating it if necessary.
+     * @param identifier ID for the web app.  Used as a subdirectory name.
+     * @return File for storing information about the web app.
      */
     public static File getWebappDirectory(String identifier) {
         Context context = ApplicationStatus.getApplicationContext();
@@ -181,11 +139,44 @@ public class WebappDirectoryManager extends AsyncTask<Void, Void, Void> {
             File baseDirectory = context.getDir(WEBAPP_DIRECTORY_NAME, Context.MODE_PRIVATE);
             File webappDirectory = new File(baseDirectory, identifier);
             if (!webappDirectory.exists() && !webappDirectory.mkdir()) {
-                Log.e(TAG, "Failed to create webapp directory.");
+                Log.e(TAG, "Failed to create web app directory.");
             }
             return webappDirectory;
         } finally {
             StrictMode.setThreadPolicy(oldPolicy);
         }
     }
+
+    /** Returns the path to the directory containing all of Chrome's data. */
+    protected String getBaseDataDirectory() {
+        return ApplicationStatus.getApplicationContext().getApplicationInfo().dataDir;
+    }
+
+    /** Returns the directory containing all of Chrome's web app data, creating it if needed. */
+    protected File getBaseWebappDirectory() {
+        return ApplicationStatus.getApplicationContext().getDir(
+                WEBAPP_DIRECTORY_NAME, Context.MODE_PRIVATE);
+    }
+
+    /** Returns the directory for this the web app's data, creating it if needed. */
+    protected File getWebappDirectory(WebappActivity activity) {
+        assert activity != null;
+        return activity.getActivityDirectory();
+    }
+
+    /** Returns a Set of Intents for all Chrome tasks currently known by the ActivityManager. */
+    @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+    protected Set<Intent> getBaseIntentsForAllTasks() {
+        Set<Intent> baseIntents = new HashSet<Intent>();
+
+        Context context = ApplicationStatus.getApplicationContext();
+        ActivityManager manager =
+                (ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE);
+        for (AppTask task : manager.getAppTasks()) {
+            Intent intent = DocumentUtils.getBaseIntentFromTask(task);
+            if (intent != null) baseIntents.add(intent);
+        }
+
+        return baseIntents;
+    }
 }