Move DownloadControllerAndroid from content/ to chrome/

content/public/android/java/src/org/chromium/content/browser/DownloadController.java

Index: content/public/android/java/src/org/chromium/content/browser/DownloadController.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/DownloadController.java b/content/public/android/java/src/org/chromium/content/browser/DownloadController.java
index 8affb6bf95911c178cc26a5778df69c373039a35..07faa71b78ba5ad406b27696b8b29fdd127c7ad0 100644
--- a/content/public/android/java/src/org/chromium/content/browser/DownloadController.java
+++ b/content/public/android/java/src/org/chromium/content/browser/DownloadController.java
@@ -9,6 +9,10 @@ import android.content.pm.PackageManager;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
+import org.chromium.content_public.browser.ContentViewDownloadDelegate;
+import org.chromium.content_public.browser.DownloadInfo;
+import org.chromium.content_public.browser.WebContents;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.base.WindowAndroid.PermissionCallback;
 
 /**
@@ -62,8 +66,9 @@ public class DownloadController {
         nativeInit();
     }
 
-    private static ContentViewDownloadDelegate downloadDelegateFromView(ContentViewCore view) {
-        return view.getDownloadDelegate();
+    private static ContentViewDownloadDelegate downloadDelegateFromWebContents(
+            WebContents webContents) {
+        return webContents.getDownloadDelegate();
     }
 
     public static void setDownloadNotificationService(DownloadNotificationService service) {
@@ -77,11 +82,11 @@ public class DownloadController {
      * The download delegate is expected to handle the download.
      */
     @CalledByNative
-    private void newHttpGetDownload(ContentViewCore view, String url,
+    private void newHttpGetDownload(WebContents webContents, String url,
             String userAgent, String contentDisposition, String mimeType,
             String cookie, String referer, boolean hasUserGesture,
             String filename, long contentLength, boolean mustDownload) {
-        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromView(view);
+        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromWebContents(webContents);
 
         if (downloadDelegate == null) return;
         DownloadInfo downloadInfo = new DownloadInfo.Builder()
@@ -102,13 +107,13 @@ public class DownloadController {
     /**
      * Notifies the download delegate that a new download has started. This can
      * be either a POST download or a GET download with authentication.
-     * @param view ContentViewCore associated with the download item.
+     * @param webContents WebContents associated with the download item.
      * @param filename File name of the downloaded file.
      * @param mimeType Mime of the downloaded item.
      */
     @CalledByNative
-    private void onDownloadStarted(ContentViewCore view, String filename, String mimeType) {
-        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromView(view);
+    private void onDownloadStarted(WebContents webContents, String filename, String mimeType) {
+        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromWebContents(webContents);
 
         if (downloadDelegate != null) {
             downloadDelegate.onDownloadStarted(filename, mimeType);
@@ -207,8 +212,9 @@ public class DownloadController {
      * Notifies the download delegate that a dangerous download started.
      */
     @CalledByNative
-    private void onDangerousDownload(ContentViewCore view, String filename, String downloadGuid) {
-        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromView(view);
+    private void onDangerousDownload(WebContents webContents, String filename,
+            String downloadGuid) {
+        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromWebContents(webContents);

[no sievers, 2016/05/27 19:09:26]

This shim layer in DownloadController to forward callbacks out to the delegate
from native still seems a bit convoluted and unnecessary.
Would it be easier to just forward that from native to the delegate?

And why is the delegate not a native interface? Wouldn't that make things
simpler?

Same for the static notifications that are routed through here. Then this Java
class can really go away.

[Jinsuk Kim, 2016/05/30 01:20:57]

That will indeed make the flow simpler. I'd like to split the work on it into a
new CL and keep this one for removing the dependency on ContentViewCore.

[Jinsuk Kim, 2016/05/31 22:40:15]

One question - having a native download delegate interface can let the native
download controller call native download delegate -> java download delegate,
rather than routing through native download controller -> java download
controller -> download delegate like it is done now. Then we don't pass java
layer download delegate but need to create a native and java delegate from
native side . Is this a common way to make use of content module from chrome? So
far I see Java layer create its native counterpart instance in most cases (via
nativeInit), not the other way around. Or is there other way to have your
suggestion work?

         if (downloadDelegate != null) {
             downloadDelegate.onDangerousDownload(filename, downloadGuid);
         }
@@ -217,23 +223,25 @@ public class DownloadController {
     /**
      * Returns whether file access is allowed.
      *
-     * @param view The ContentViewCore to access file system.
+     * @param windowAndroid WindowAndroid to access file system.
      * @return true if allowed, or false otherwise.
      */
     @CalledByNative
-    private boolean hasFileAccess(ContentViewCore view) {
-        return view.getWindowAndroid().hasPermission(permission.WRITE_EXTERNAL_STORAGE);
+    private boolean hasFileAccess(WindowAndroid windowAndroid) {
+        return windowAndroid.hasPermission(permission.WRITE_EXTERNAL_STORAGE);
     }
 
     /**
      * Called to prompt user with the file access permission.
      *
-     * @param view The ContentViewCore to access file system.
+     * @param webContents WebContents associated with the file access.
+     * @param windowAndroid WindowAndroid to access file system.
      * @param callbackId The native callback function pointer.
      */
     @CalledByNative
-    private void requestFileAccess(final ContentViewCore view, final long callbackId) {
-        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromView(view);
+    private void requestFileAccess(WebContents webContents, final WindowAndroid windowAndroid,
+            final long callbackId) {
+        ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromWebContents(webContents);
         if (downloadDelegate != null) {
             downloadDelegate.requestFileAccess(callbackId);
         } else {
@@ -244,7 +252,7 @@ public class DownloadController {
                             && grantResults[0] == PackageManager.PERMISSION_GRANTED);
                 }
             };
-            view.getWindowAndroid().requestPermissions(
+            windowAndroid.requestPermissions(
                     new String[] {android.Manifest.permission.WRITE_EXTERNAL_STORAGE},
                     permissionCallback);
         }