move file access permission logic to DownloadResourceThrottle

chrome/browser/download/download_resource_throttle.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/download_resource_throttle.h"
 
 #include "base/bind.h"
 #include "chrome/browser/download/download_stats.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_controller.h"
 
-DownloadResourceThrottle::DownloadResourceThrottle(
-    DownloadRequestLimiter* limiter,
+#if defined(OS_ANDROID)
+#include "content/public/browser/android/download_controller_android.h"
+#include "content/public/browser/render_view_host.h"
+
+using content::DownloadControllerAndroid;
+#endif
+
+using content::BrowserThread;
+
+namespace {
+
+void OnCanDownloadDecided(
+    const DownloadRequestLimiter::Callback& callback, bool allow) {
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE, base::Bind(callback, allow));
+}
+
+void CanDownload(
+    scoped_refptr<DownloadRequestLimiter> limiter,
     int render_process_id,
     int render_view_id,
     const GURL& url,
+    const std::string& request_method,
+    const DownloadRequestLimiter::Callback& continue_callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  limiter->CanDownload(render_process_id, render_view_id, url, request_method,
+                       base::Bind(&OnCanDownloadDecided, continue_callback));
+}
+
+#if defined(OS_ANDROID)
+void OnAcquireFileAccessPermissionDone(
+    scoped_refptr<DownloadRequestLimiter> limiter,
+    int render_process_id,
+    int render_view_id,
+    const GURL& url,
+    const std::string& request_method,
+    const DownloadRequestLimiter::Callback& continue_callback,
+    bool granted) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (granted) {
+    CanDownload(limiter, render_process_id, render_view_id, url,
+                request_method, continue_callback);
+  } else {
+    OnCanDownloadDecided(continue_callback, false);
+  }
+}
+#endif
+
+void CanDownloadOnUIThread(
+    scoped_refptr<DownloadRequestLimiter> limiter,
+    int render_process_id,
+    int render_view_id,
+    const GURL& url,
+    const std::string& request_method,
+    const DownloadRequestLimiter::Callback& continue_callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+#if defined(OS_ANDROID)
+  content::DownloadControllerAndroid::Get()->AcquireFileAccessPermission(
+      render_process_id, render_view_id, base::Bind(
+          &OnAcquireFileAccessPermissionDone, limiter, render_process_id,
+          render_view_id, url, request_method, continue_callback));
+#else
+  CanDownload(limiter, render_process_id, render_view_id, url,
+              request_method, continue_callback);
+#endif
+}
+
+}  // namespace
+
+DownloadResourceThrottle::DownloadResourceThrottle(
+    scoped_refptr<DownloadRequestLimiter> limiter,
+    int render_process_id,
+    int render_view_id,
+    const GURL& url,
     const std::string& request_method)
     : querying_limiter_(true),
       request_allowed_(false),
       request_deferred_(false) {
-  limiter->CanDownloadOnIOThread(
-      render_process_id,
-      render_view_id,
-      url,
-      request_method,
-      base::Bind(&DownloadResourceThrottle::ContinueDownload,
-                 AsWeakPtr()));
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE, base::Bind(
+          &CanDownloadOnUIThread, limiter, render_process_id, render_view_id,

[asanka, 2015/07/21 01:14:00]

Nit: When you find yourself binding / unpacking / repacking the same set of
parameters, it's probably cleaner to put them in a struct and pass around a
scoped_ptr<> to the struct.

That way you'll avoid much of the boilerplate.

[qinmin, 2015/07/21 22:42:17]

Done.

+          url, request_method,base::Bind(

[asanka, 2015/07/21 01:14:00]

Nit: space after ,

Also run through 'git cl format'

[qinmin, 2015/07/21 22:42:17]

Done.

+              &DownloadResourceThrottle::ContinueDownload, AsWeakPtr())));

[asanka, 2015/07/21 01:14:00]

Nit: Rather than passing up a callback that must be invoked on the IO thread,
it'll be cleaner to pass in a callback that can be invoked on the UI thread.
E.g. if was a OnCanDownloadDecided(WeakPtr<DownloadResourceThrottle>, bool) that
can be invoked on the UI thread.

[qinmin, 2015/07/21 22:42:17]

In that case, I need to make DownloadResourceThrottle::ContinueDownload()
public, or pass DownloadResourceThrottle::ContinueDownload() as a callback into
OnCandownloadDecided(), or make OnCanDownloadDecided a member function of
DownloadResourceThrottle.
Since the last one makes DownloadResourceThrottle to have a member function on
UI thread, choosing the 1st option instead.

 }
 
 DownloadResourceThrottle::~DownloadResourceThrottle() {
 }
 
 void DownloadResourceThrottle::WillStartRequest(bool* defer) {
   WillDownload(defer);
 }
 
 void DownloadResourceThrottle::WillRedirectRequest(
@@ skipping 18 matching lines @@
     request_deferred_ = true;
     *defer = true;
     return;
   }
 
   if (!request_allowed_)
     controller()->Cancel();
 }
 
 void DownloadResourceThrottle::ContinueDownload(bool allow) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   querying_limiter_ = false;
   request_allowed_ = allow;
 
   if (allow) {
     // Presumes all downloads initiated by navigation use this throttle and
     // nothing else does.
     RecordDownloadSource(DOWNLOAD_INITIATED_BY_NAVIGATION);
   } else {
     RecordDownloadCount(CHROME_DOWNLOAD_COUNT_BLOCKED_BY_THROTTLING);
   }
 
   if (request_deferred_) {
     request_deferred_ = false;
     if (allow) {
       controller()->Resume();
     } else {
       controller()->Cancel();
     }
   }
 }