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..6dfd437406b53735ab3fa24bfe7c5a58ad7a392f 100644
--- a/content/browser/android/download_controller_android_impl.cc
+++ b/content/browser/android/download_controller_android_impl.cc
@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "content/browser/android/content_view_core_impl.h"
 #include "content/browser/android/deferred_download_observer.h"
+#include "content/browser/android/mock_download_controller_android.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/browser/download/download_manager_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
@@ -45,6 +46,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.release()->Run(granted);

[asanka, 2015/07/07 05:03:33]

cb leaks. Don't release().

[qinmin, 2015/07/07 06:14:05]

Done. Thanks for catching this.

+}
+
 struct DownloadControllerAndroidImpl::JavaObject {
   ScopedJavaLocalRef<jobject> Controller(JNIEnv* env) {
     return GetRealObject(env, obj);
@@ -59,6 +73,8 @@ bool DownloadControllerAndroidImpl::RegisterDownloadController(JNIEnv* env) {
 
 // static
 DownloadControllerAndroid* DownloadControllerAndroid::Get() {
+  if (DownloadControllerAndroid::use_mock_controller())

[asanka, 2015/07/07 05:03:33]

Alternatively, allow setting the DownloadControllerAndroid*. That way test code
can inject a mock that the test code controls.

Let's not compile mocks into production code.

[qinmin, 2015/07/07 06:14:05]

Done.

+    return MockDownloadControllerAndroid::GetInstance();
   return DownloadControllerAndroidImpl::GetInstance();
 }
 
@@ -97,6 +113,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()) {
+    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 +281,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 +301,36 @@ 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);
+  if (!web_contents) {
+    // The view went away. Can't proceed.
+    LOG(ERROR) << "Download failed on URL:" << info.url.spec();

[asanka, 2015/07/07 05:03:33]

Did you intend to remove these log entries?

[qinmin, 2015/07/07 06:14:05]

This could happen if tab was closed immediately after download initiates. But
that doesn't feel like a bug. Removed the error log.

+    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 =