Prompt user for file access permission before download starts

content/browser/android/download_controller_android_impl.cc

Index: content/browser/android/download_controller_android_impl.cc
diff --git a/content/browser/android/download_controller_android_impl.cc b/content/browser/android/download_controller_android_impl.cc
index 68675b56d9ae2b83e5d64265b806d136f2be4740..53bb7d42874af5b5fb47ffc20fd9f82de88e4fc4 100644
--- a/content/browser/android/download_controller_android_impl.cc
+++ b/content/browser/android/download_controller_android_impl.cc
@@ -45,6 +45,19 @@ static void Init(JNIEnv* env, jobject obj) {
   DownloadControllerAndroidImpl::GetInstance()->Init(env, obj);
 }
 
+static void OnRequestFileAccessResult(
+    JNIEnv* env, jobject obj, jlong callback_id, jboolean granted) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(callback_id);
+
+  // Convert java long long int to c++ pointer, take ownership.
+  scoped_ptr<DownloadControllerAndroid::UserPermissionPromptCallback> callback(
+      reinterpret_cast<
+          DownloadControllerAndroid::UserPermissionPromptCallback*>(
+              callback_id));
+  callback.release()->Run(granted);
+}
+
 struct DownloadControllerAndroidImpl::JavaObject {
   ScopedJavaLocalRef<jobject> Controller(JNIEnv* env) {
     return GetRealObject(env, obj);
@@ -97,6 +110,56 @@ void DownloadControllerAndroidImpl::CancelDeferredDownload(
   }
 }
 
+bool DownloadControllerAndroidImpl::RequireUserPermissionPrompt(

[asanka, 2015/07/06 21:09:41]

Why not just have an AcquireFileAccessPermission() or some such that does the
equivalent of checking hasFileAccess() and calling requestFileAccess() if
necessary? It seems none of the call sites care about checking whether a
permission prompt is required without intending to prompt the user.

[qinmin, 2015/07/07 00:54:22]

Done.

+    WebContents* web_contents) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!web_contents)
+    return true;
+
+  ScopedJavaLocalRef<jobject> view =
+      GetContentViewCoreFromWebContents(web_contents);
+  if (view.is_null())
+    return true;
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  return !Java_DownloadController_hasFileAccess(
+      env, GetJavaObject()->Controller(env).obj(), view.obj());
+}
+
+void DownloadControllerAndroidImpl::PromptUserForPermission(
+    WebContents* web_contents,
+    const DownloadControllerAndroid::UserPermissionPromptCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!web_contents) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, base::Bind(callback, false));
+    return;
+  }
+  PromptUserForPermissionInternal(web_contents, callback);
+}
+
+void DownloadControllerAndroidImpl::PromptUserForPermissionInternal(
+    WebContents* web_contents,
+    const DownloadControllerAndroid::UserPermissionPromptCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(web_contents);
+
+  ScopedJavaLocalRef<jobject> view =
+      GetContentViewCoreFromWebContents(web_contents);
+  if (view.is_null()) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, base::Bind(callback, false));
+    return;
+  }
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  // Make copy on the heap so we can pass the pointer through JNI.
+  intptr_t callback_id = reinterpret_cast<intptr_t>(
+      new DownloadControllerAndroid::UserPermissionPromptCallback(callback));
+  Java_DownloadController_requestFileAccess(
+      env, GetJavaObject()->Controller(env).obj(), view.obj(), callback_id);
+}
+
 void DownloadControllerAndroidImpl::CreateGETDownload(
     int render_process_id, int render_view_id, int request_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -211,9 +274,7 @@ void DownloadControllerAndroidImpl::StartAndroidDownload(
     int render_process_id, int render_view_id,
     const DownloadInfoAndroid& info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  JNIEnv* env = base::android::AttachCurrentThread();
 
-  // Call newHttpGetDownload
   WebContents* web_contents = GetWebContents(render_process_id, render_view_id);
   if (!web_contents) {
     // The view went away. Can't proceed.
@@ -233,6 +294,40 @@ void DownloadControllerAndroidImpl::StartAndroidDownload(
                    info)));
     return;
   }
+
+  if (RequireUserPermissionPrompt(web_contents)) {
+    PromptUserForPermissionInternal(web_contents, base::Bind(
+        &DownloadControllerAndroidImpl::StartAndroidDownloadInternal,
+        base::Unretained(this), render_process_id, render_view_id,

[Ted C, 2015/07/06 20:03:39]

what is the lifetime of DownloadControllerAndroidImpl?  Just making sure there
isn't a situation where it could be destroyed while the file access is being
requested?

[qinmin, 2015/07/06 20:36:02]

DownloadControllerAndroidImpl's lifetime is at the process level, so we are safe
here.

+        info));
+    return;
+  }
+
+  StartAndroidDownloadInternal(render_process_id, render_view_id, info, true);
+}
+
+void DownloadControllerAndroidImpl::StartAndroidDownloadInternal(
+    int render_process_id, int render_view_id,
+    const DownloadInfoAndroid& info, bool allowed) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!allowed)
+    return;
+
+  // Call newHttpGetDownload
+  WebContents* web_contents = GetWebContents(render_process_id, render_view_id);
+  if (!web_contents) {
+    // The view went away. Can't proceed.
+    LOG(ERROR) << "Download failed on URL:" << info.url.spec();
+    return;
+  }
+  ScopedJavaLocalRef<jobject> view =
+      GetContentViewCoreFromWebContents(web_contents);
+  if (view.is_null()) {
+    LOG(ERROR) << "Download failed on URL:" << info.url.spec();
+    return;
+  }
+
+  JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> jurl =
       ConvertUTF8ToJavaString(env, info.url.spec());
   ScopedJavaLocalRef<jstring> juser_agent =