Download WebAPK into a WebAPK-specifc directory, and delete all cached ones.

chrome/browser/android/webapk/webapk_installer.cc

Index: chrome/browser/android/webapk/webapk_installer.cc
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc
index 37df154bd8a3115e4a5972973c4ac9b6b9766e7b..8f596b8a630393ed8c206357f11f09a1c160d397 100644
--- a/chrome/browser/android/webapk/webapk_installer.cc
+++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -52,6 +52,14 @@ const int kWebApkDownloadUrlTimeoutMs = 60000;
 // complete.
 const int kDownloadTimeoutMs = 60000;
 
+const int KBasicPosixPermissions = base::FILE_PERMISSION_READ_BY_USER |
+                                   base::FILE_PERMISSION_WRITE_BY_USER |
+                                   base::FILE_PERMISSION_EXECUTE_BY_USER |
+                                   base::FILE_PERMISSION_READ_BY_GROUP |
+                                   base::FILE_PERMISSION_WRITE_BY_GROUP |
+                                   base::FILE_PERMISSION_READ_BY_OTHERS |
+                                   base::FILE_PERMISSION_WRITE_BY_OTHERS;

[pkotwicz, 2016/09/24 01:28:35]

I did some local testing. According to my testing,
FILE_PERMISSION_WRITE_BY_GROUP and FILE_PERMISSION_WRITE_BY_OTHERS seem
unnecessary

[Xi Han, 2016/09/26 17:17:11]

Good catch, reverted it back.

+
 // Returns the scope from |info| if it is specified. Otherwise, returns the
 // default scope.
 GURL GetScope(const ShortcutInfo& info) {
@@ -128,6 +136,30 @@ GURL GetServerUrlForUpdate(const GURL& server_url,
               kDefaultWebApkServerUrlResponseType);
 }
 
+// Creates a directory depending on the type of the task, and set permissions.
+// It also creates any parent directory along the path if doesn't exist,
+// and sets permissions as well.
+base::FilePath CreateSubDirAndSetPermissionsInBackground(
+    const base::FilePath& output_root_dir,
+    WebApkInstaller::TaskType task_type,
+    const std::string& package_name) {
+  base::FilePath webapk_dir = output_root_dir.AppendASCII("webapks");
+  base::FilePath output_dir = webapk_dir.AppendASCII(
+      task_type == WebApkInstaller::INSTALL ? "install" : "update");

[pkotwicz, 2016/09/24 01:28:34]

Can you explain why you use a different directory name for installs and updates?

[Xi Han, 2016/09/26 17:17:10]

As discussed offline, add a comment here.

+  int posix_permissions = KBasicPosixPermissions |
+                          base::FILE_PERMISSION_EXECUTE_BY_OTHERS;
+  if (base::PathExists(output_dir))
+    base::DeleteFile(output_dir, true);
+
+  // Creates the directory to download and sets permissions.
+  if (!base::CreateDirectory(output_dir) ||
+      !base::SetPosixFilePermissions(webapk_dir, posix_permissions) ||
+      !base::SetPosixFilePermissions(output_dir, posix_permissions))
+    return base::FilePath();
+
+  return output_dir;
+}
+
 }  // anonymous namespace
 
 WebApkInstaller::WebApkInstaller(const ShortcutInfo& shortcut_info,
@@ -351,9 +383,21 @@ void WebApkInstaller::OnGotWebApkDownloadUrl(const GURL& download_url,
   base::FilePath output_dir;
   base::android::GetCacheDirectory(&output_dir);
   webapk_package_ = package_name;
-  // TODO(pkotwicz): Download WebAPKs into WebAPK-specific subdirectory
-  // directory.
-  // TODO(pkotwicz): Figure out when downloaded WebAPK should be deleted.
+
+  base::PostTaskAndReplyWithResult(
+      GetBackgroundTaskRunner().get(), FROM_HERE,
+      base::Bind(&CreateSubDirAndSetPermissionsInBackground,
+                 output_dir, task_type_, package_name),
+      base::Bind(&WebApkInstaller::OnCreateSubDirAndSetPermissions,
+                 weak_ptr_factory_.GetWeakPtr(), download_url));
+}
+
+void WebApkInstaller::OnCreateSubDirAndSetPermissions(
+    const GURL& download_url, const base::FilePath& output_dir) {
+  if (output_dir.empty()) {
+    OnFailure();
+    return;
+  }
 
   timer_.Start(
       FROM_HERE, base::TimeDelta::FromMilliseconds(download_timeout_ms_),
@@ -375,13 +419,10 @@ void WebApkInstaller::OnWebApkDownloaded(const base::FilePath& file_path,
     return;
   }
 
-  int posix_permissions = base::FILE_PERMISSION_READ_BY_USER |
-                          base::FILE_PERMISSION_WRITE_BY_USER |
-                          base::FILE_PERMISSION_READ_BY_GROUP |
-                          base::FILE_PERMISSION_READ_BY_OTHERS;
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
-      base::Bind(&base::SetPosixFilePermissions, file_path, posix_permissions),
+      base::Bind(&base::SetPosixFilePermissions, file_path,
+                 KBasicPosixPermissions),
       base::Bind(&WebApkInstaller::OnWebApkMadeWorldReadable,
                  weak_ptr_factory_.GetWeakPtr(), file_path));
 }