Bug 1240710 - Pick (temporary) download directory depending on whether permission has been granted. r=nalexander,paolo,jchen
authorSebastian Kaspari <s.kaspari@gmail.com>
Thu, 18 Feb 2016 15:11:42 +0000
changeset 323231 129e9b9ce25be48c2544406be1d0e9e9ff33b51b
parent 323230 34c795c24d3178e5492e3f0989c13149e5c09e41
child 323232 398739346a3a872fd5aaf09098243b3dc8c9f4af
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander, paolo, jchen
bugs1240710
milestone47.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1240710 - Pick (temporary) download directory depending on whether permission has been granted. r=nalexander,paolo,jchen As soon as the user clicks on a link to download a file Gecko will start the download - even before prompting the user. This led to problems when the user hadn't granted the permission to write to the downloads directory yet. The download would fail even though the user (later) accepted the permission. With this patch we will start the download to the app's cache directory (only if we do not have the permission) and prompt the user. As soon as the user has accepted the permission the download will be moved to the public downloads directory (even while still downloading). If the permission is denied the download will be cancelled. After the permission has been granted all subsequent downloads will start writing to the downloads directory directly. MozReview-Commit-ID: CCqk9h7Sxor
mobile/android/base/java/org/mozilla/gecko/DownloadsIntegration.java
mobile/android/base/java/org/mozilla/gecko/permissions/Permissions.java
mobile/android/components/HelperAppDialog.js
uriloader/exthandler/nsExternalHelperAppService.cpp
widget/android/GeneratedJNIWrappers.cpp
widget/android/GeneratedJNIWrappers.h
--- a/mobile/android/base/java/org/mozilla/gecko/DownloadsIntegration.java
+++ b/mobile/android/base/java/org/mozilla/gecko/DownloadsIntegration.java
@@ -2,32 +2,34 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
 import org.mozilla.gecko.annotation.WrapForJNI;
 import org.mozilla.gecko.AppConstants.Versions;
+import org.mozilla.gecko.permissions.Permissions;
 import org.mozilla.gecko.util.NativeEventListener;
 import org.mozilla.gecko.util.NativeJSObject;
 import org.mozilla.gecko.util.EventCallback;
 
 import java.io.File;
 import java.lang.IllegalArgumentException;
 import java.util.ArrayList;
 import java.util.List;
 
 import android.app.DownloadManager;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.database.Cursor;
 import android.media.MediaScannerConnection;
 import android.media.MediaScannerConnection.MediaScannerConnectionClient;
 import android.net.Uri;
+import android.os.Environment;
 import android.text.TextUtils;
 import android.util.Log;
 
 public class DownloadsIntegration implements NativeEventListener
 {
     private static final String LOGTAG = "GeckoDownloadsIntegration";
 
     @SuppressWarnings("serial")
@@ -104,16 +106,35 @@ public class DownloadsIntegration implem
             return false;
         }
 
         return (PackageManager.COMPONENT_ENABLED_STATE_ENABLED == state ||
                 PackageManager.COMPONENT_ENABLED_STATE_DEFAULT == state);
     }
 
     @WrapForJNI
+    public static String getTemporaryDownloadDirectory() {
+        Context context = GeckoAppShell.getApplicationContext();
+
+        if (Permissions.has(context, android.Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+            // We do have the STORAGE permission, so we can save the file directly to the public
+            // downloads directory.
+            return Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS)
+                    .getAbsolutePath();
+        } else {
+            // Without the permission we are going to start to download the file to the cache
+            // directory. Later in the process we will ask for the permission and the download
+            // process will move the file to the actual downloads directory. If we do not get the
+            // permission then the download will be cancelled.
+            return context.getCacheDir().getAbsolutePath();
+        }
+    }
+
+
+    @WrapForJNI
     public static void scanMedia(final String aFile, String aMimeType) {
         String mimeType = aMimeType;
         if (UNKNOWN_MIME_TYPES.contains(mimeType)) {
             // If this is a generic undefined mimetype, erase it so that we can try to determine
             // one from the file extension below.
             mimeType = "";
         }
 
--- a/mobile/android/base/java/org/mozilla/gecko/permissions/Permissions.java
+++ b/mobile/android/base/java/org/mozilla/gecko/permissions/Permissions.java
@@ -87,16 +87,23 @@ public class Permissions {
 
         try {
             return blockingTask.get();
         } catch (InterruptedException | ExecutionException | CancellationException e) {
             return false;
         }
     }
 
+    /**
+     * Determine whether you have been granted particular permissions.
+     */
+    public static boolean has(Context context, String... permissions) {
+        return permissionHelper.hasPermissions(context, permissions);
+    }
+
     /* package-private */ static void setPermissionHelper(PermissionsHelper permissionHelper) {
         Permissions.permissionHelper = permissionHelper;
     }
 
     /**
      * Callback for Activity.onRequestPermissionsResult(). All activities that prompt for permissions using this class
      * should implement onRequestPermissionsResult() and call this method.
      */
--- a/mobile/android/components/HelperAppDialog.js
+++ b/mobile/android/components/HelperAppDialog.js
@@ -14,16 +14,18 @@ const URI_GENERIC_ICON_DOWNLOAD = "drawa
 
 Cu.import("resource://gre/modules/Downloads.jsm");
 Cu.import("resource://gre/modules/FileUtils.jsm");
 Cu.import("resource://gre/modules/HelperApps.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetter(this, "RuntimePermissions", "resource://gre/modules/RuntimePermissions.jsm");
+
 // -----------------------------------------------------------------------
 // HelperApp Launcher Dialog
 // -----------------------------------------------------------------------
 
 XPCOMUtils.defineLazyModuleGetter(this, "Snackbars", "resource://gre/modules/Snackbars.jsm");
 
 XPCOMUtils.defineLazyGetter(this, "ContentAreaUtils", function() {
   let ContentAreaUtils = {};
@@ -236,19 +238,25 @@ HelperAppLauncherDialog.prototype = {
       Services.prefs.clearUserPref(this._getPrefName(mime));
   },
 
   promptForSaveToFileAsync: function (aLauncher, aContext, aDefaultFile,
                                       aSuggestedFileExt, aForcePrompt) {
     Task.spawn(function* () {
       let file = null;
       try {
-        let preferredDir = yield Downloads.getPreferredDownloadsDirectory();
-        file = this.validateLeafName(new FileUtils.File(preferredDir),
-                                     aDefaultFile, aSuggestedFileExt);
+        let hasPermission = yield RuntimePermissions.waitForPermissions(RuntimePermissions.WRITE_EXTERNAL_STORAGE);
+        if (hasPermission) {
+          // If we do have the STORAGE permission then pick the public downloads directory as destination
+          // for this file. Without the permission saveDestinationAvailable(null) will be called which
+          // will effectively cancel the download.
+          let preferredDir = yield Downloads.getPreferredDownloadsDirectory();
+          file = this.validateLeafName(new FileUtils.File(preferredDir),
+                                       aDefaultFile, aSuggestedFileExt);
+        }
       } finally {
         // The file argument will be null in case any exception occurred.
         aLauncher.saveDestinationAvailable(file);
       }
     }.bind(this)).catch(Cu.reportError);
   },
 
   validateLeafName: function hald_validateLeafName(aLocalFile, aLeafName, aFileExt) {
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -371,26 +371,30 @@ static nsresult GetDownloadDirectory(nsI
   nsresult rv = dsf->mFile->Exists(&alreadyThere);
   NS_ENSURE_SUCCESS(rv, rv);
   if (!alreadyThere) {
     rv = dsf->mFile->Create(nsIFile::DIRECTORY_TYPE, 0770);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   dir = dsf->mFile;
 #elif defined(ANDROID)
-  // On mobile devices, we are avoiding exposing users to the file
-  // system, and don't save downloads to temp directories
-
-  // On Android we only return something if we have and SD-card
-  char* downloadDir = getenv("DOWNLOADS_DIRECTORY");
+  // We ask Java for the temporary download directory. The directory will be
+  // different depending on whether we have the permission to write to the
+  // public download directory or not.
+  // In the case where we do not have the permission we will start the
+  // download to the app cache directory and later move it to the final
+  // destination after prompting for the permission.
+  auto downloadDir = widget::DownloadsIntegration::GetTemporaryDownloadDirectory();
+
   nsresult rv;
   if (downloadDir) {
     nsCOMPtr<nsIFile> ldir;
-    rv = NS_NewNativeLocalFile(nsDependentCString(downloadDir),
+    rv = NS_NewNativeLocalFile(downloadDir->ToCString(),
                                true, getter_AddRefs(ldir));
+
     NS_ENSURE_SUCCESS(rv, rv);
     dir = do_QueryInterface(ldir);
 
     // If we're not checking for availability we're done.
     if (aSkipChecks) {
       dir.forget(_directory);
       return NS_OK;
     }
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -26,16 +26,24 @@ template<> const char mozilla::jni::Cont
         "org/mozilla/gecko/AlarmReceiver";
 
 constexpr char AlarmReceiver::NotifyAlarmFired_t::name[];
 constexpr char AlarmReceiver::NotifyAlarmFired_t::signature[];
 
 template<> const char mozilla::jni::Context<DownloadsIntegration, jobject>::name[] =
         "org/mozilla/gecko/DownloadsIntegration";
 
+constexpr char DownloadsIntegration::GetTemporaryDownloadDirectory_t::name[];
+constexpr char DownloadsIntegration::GetTemporaryDownloadDirectory_t::signature[];
+
+auto DownloadsIntegration::GetTemporaryDownloadDirectory() -> mozilla::jni::String::LocalRef
+{
+    return mozilla::jni::Method<GetTemporaryDownloadDirectory_t>::Call(DownloadsIntegration::Context(), nullptr);
+}
+
 constexpr char DownloadsIntegration::ScanMedia_t::name[];
 constexpr char DownloadsIntegration::ScanMedia_t::signature[];
 
 auto DownloadsIntegration::ScanMedia(mozilla::jni::String::Param a0, mozilla::jni::String::Param a1) -> void
 {
     return mozilla::jni::Method<ScanMedia_t>::Call(DownloadsIntegration::Context(), nullptr, a0, a1);
 }
 
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -85,16 +85,31 @@ public:
     template<class Impl> class Natives;
 };
 
 class DownloadsIntegration : public mozilla::jni::ObjectBase<DownloadsIntegration, jobject>
 {
 public:
     explicit DownloadsIntegration(const Context& ctx) : ObjectBase<DownloadsIntegration, jobject>(ctx) {}
 
+    struct GetTemporaryDownloadDirectory_t {
+        typedef DownloadsIntegration Owner;
+        typedef mozilla::jni::String::LocalRef ReturnType;
+        typedef mozilla::jni::String::Param SetterType;
+        typedef mozilla::jni::Args<> Args;
+        static constexpr char name[] = "getTemporaryDownloadDirectory";
+        static constexpr char signature[] =
+                "()Ljava/lang/String;";
+        static const bool isStatic = true;
+        static const mozilla::jni::ExceptionMode exceptionMode =
+                mozilla::jni::ExceptionMode::ABORT;
+    };
+
+    static auto GetTemporaryDownloadDirectory() -> mozilla::jni::String::LocalRef;
+
     struct ScanMedia_t {
         typedef DownloadsIntegration Owner;
         typedef void ReturnType;
         typedef void SetterType;
         typedef mozilla::jni::Args<
                 mozilla::jni::String::Param,
                 mozilla::jni::String::Param> Args;
         static constexpr char name[] = "scanMedia";