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..07b122872197773830087325fb2ee8b1b5015f2e 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::AcquireFileAccessPermissionCallback> cb(
+      reinterpret_cast<
+          DownloadControllerAndroid::AcquireFileAccessPermissionCallback*>(
+              callback_id));
+  cb->Run(granted);
+}
+
 struct DownloadControllerAndroidImpl::JavaObject {
   ScopedJavaLocalRef<jobject> Controller(JNIEnv* env) {
     return GetRealObject(env, obj);
@@ -59,7 +72,11 @@ bool DownloadControllerAndroidImpl::RegisterDownloadController(JNIEnv* env) {
 
 // static
 DownloadControllerAndroid* DownloadControllerAndroid::Get() {
-  return DownloadControllerAndroidImpl::GetInstance();
+  if (!DownloadControllerAndroid::download_controller_) {

[asanka, 2015/07/07 17:39:24]

Nit: Given that this code no longer has the atomicity guarantee that it used to
have, it's worth binding it to a thread with a DCHECK_CURRENTLY_ON(...);

[qinmin, 2015/07/07 20:52:50]

CreateGETDownload was called on IO thread. Moving it to UI thread involves quite
some refactoring. Added a lock here to protect the download_controller_ instead.

+    DownloadControllerAndroid::SetDownloadControllerAndroid(
+        DownloadControllerAndroidImpl::GetInstance());
+  }
+  return DownloadControllerAndroid::download_controller_;
 }
 
 // static
@@ -97,6 +114,60 @@ void DownloadControllerAndroidImpl::CancelDeferredDownload(
   }
 }
 
+void DownloadControllerAndroidImpl::AcquireFileAccessPermission(
+    WebContents* web_contents,
+    const DownloadControllerAndroid::AcquireFileAccessPermissionCallback& cb) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!web_contents) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, base::Bind(cb, false));
+    return;
+  }
+
+  ScopedJavaLocalRef<jobject> view =
+      GetContentViewCoreFromWebContents(web_contents);
+  if (HasFileAccessPermission(view)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, base::Bind(cb, true));
+    return;
+  }
+
+  PromptUserForPermission(web_contents, cb);
+}
+
+bool DownloadControllerAndroidImpl::HasFileAccessPermission(
+    ScopedJavaLocalRef<jobject> j_content_view_core) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (j_content_view_core.is_null())
+    return false;
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  return Java_DownloadController_hasFileAccess(
+      env, GetJavaObject()->Controller(env).obj(), j_content_view_core.obj());
+}
+
+void DownloadControllerAndroidImpl::PromptUserForPermission(
+    WebContents* web_contents,
+    const DownloadControllerAndroid::AcquireFileAccessPermissionCallback& cb) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(web_contents);
+
+  ScopedJavaLocalRef<jobject> view =
+      GetContentViewCoreFromWebContents(web_contents);
+  if (view.is_null()) {

[asanka, 2015/07/07 17:39:24]

Nit: This check should be performed in AcquireFileAccessPermission() above.

Once that's moved, the rest of the function just consists of the
requestFileAccess() call, which might as well also be folded into AFAP().

[qinmin, 2015/07/07 20:52:50]

Done.

+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE, base::Bind(cb, 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::AcquireFileAccessPermissionCallback(cb));
+  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 +282,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 +302,32 @@ void DownloadControllerAndroidImpl::StartAndroidDownload(
                    info)));
     return;
   }
+
+  AcquireFileAccessPermission(
+      web_contents,
+      base::Bind(&DownloadControllerAndroidImpl::StartAndroidDownloadInternal,
+                 base::Unretained(this), render_process_id, render_view_id,
+                 info));
+}
+
+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);
+  // The view went away. Can't proceed.
+  if (!web_contents)
+    return;
+  ScopedJavaLocalRef<jobject> view =
+      GetContentViewCoreFromWebContents(web_contents);
+  if (view.is_null())
+    return;
+
+  JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> jurl =
       ConvertUTF8ToJavaString(env, info.url.spec());
   ScopedJavaLocalRef<jstring> juser_agent =