Remove DownloadController java weak reference

All DownloadController java method can be static.
So there is no point to hold a java reference.

Review-Url: https://codereview.chromium.org/2902453003
Cr-Commit-Position: refs/heads/master@{#473722}

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadController.java

@@ -30,7 +30,6 @@ import org.chromium.ui.base.WindowAndroid.PermissionCallback;
  */
 public class DownloadController {
     private static final String LOGTAG = "DownloadController";
-    private static final DownloadController sInstance = new DownloadController();
 
     /**
      * Class for notifying the application that download has completed.
@@ -64,15 +63,6 @@ public class DownloadController {
 
     private static DownloadNotificationService sDownloadNotificationService;
 
-    @CalledByNative
-    public static DownloadController getInstance() {
-        return sInstance;
-    }
-
-    private DownloadController() {
-        nativeInit();
-    }
-
     public static void setDownloadNotificationService(DownloadNotificationService service) {
         sDownloadNotificationService = service;
     }
@@ -82,7 +72,7 @@ public class DownloadController {
      * download. This can be either a POST download or a GET download with authentication.
      */
     @CalledByNative
-    private void onDownloadCompleted(DownloadInfo downloadInfo) {
+    private static void onDownloadCompleted(DownloadInfo downloadInfo) {
         if (sDownloadNotificationService == null) return;
         sDownloadNotificationService.onDownloadCompleted(downloadInfo);
     }
@@ -92,7 +82,7 @@ public class DownloadController {
      * download. This can be either a POST download or a GET download with authentication.
      */
     @CalledByNative
-    private void onDownloadInterrupted(DownloadInfo downloadInfo, boolean isAutoResumable) {
+    private static void onDownloadInterrupted(DownloadInfo downloadInfo, boolean isAutoResumable) {
         if (sDownloadNotificationService == null) return;
         sDownloadNotificationService.onDownloadInterrupted(downloadInfo, isAutoResumable);
     }
@@ -101,7 +91,7 @@ public class DownloadController {
      * Called when a download was cancelled.
      */
     @CalledByNative
-    private void onDownloadCancelled(DownloadInfo downloadInfo) {
+    private static void onDownloadCancelled(DownloadInfo downloadInfo) {
         if (sDownloadNotificationService == null) return;
         sDownloadNotificationService.onDownloadCancelled(downloadInfo);
     }
@@ -111,7 +101,7 @@ public class DownloadController {
      * network stack use custom notification to display the progress of downloads.
      */
     @CalledByNative
-    private void onDownloadUpdated(DownloadInfo downloadInfo) {
+    private static void onDownloadUpdated(DownloadInfo downloadInfo) {
         if (sDownloadNotificationService == null) return;
         sDownloadNotificationService.onDownloadUpdated(downloadInfo);
     }
@@ -123,7 +113,7 @@ public class DownloadController {
      * @return true if allowed, or false otherwise.
      */
     @CalledByNative
-    private boolean hasFileAccess() {
+    private static boolean hasFileAccess() {
         Activity activity = ApplicationStatus.getLastTrackedFocusedActivity();
         if (activity instanceof ChromeActivity) {
             return ((ChromeActivity) activity)
@@ -134,7 +124,7 @@ public class DownloadController {
     }
 
     @CalledByNative
-    private void requestFileAccess(final long callbackId) {
+    private static void requestFileAccess(final long callbackId) {
         Activity activity = ApplicationStatus.getLastTrackedFocusedActivity();
         if (!(activity instanceof ChromeActivity)) {
             nativeOnAcquirePermissionResult(callbackId, false, null);
@@ -228,7 +218,7 @@ public class DownloadController {
      * Called when a download is started.
      */
     @CalledByNative
-    private void onDownloadStarted() {
+    private static void onDownloadStarted() {
         DownloadUtils.showDownloadStartToast(ContextUtils.getApplicationContext());
     }
 
@@ -256,7 +246,6 @@ public class DownloadController {
     }
 
     // native methods
-    private native void nativeInit();
-    private native void nativeOnAcquirePermissionResult(
+    private static native void nativeOnAcquirePermissionResult(
             long callbackId, boolean granted, String permissionToUpdate);
 }

chrome/browser/android/download/download_controller.cc

@@ -107,14 +107,9 @@ bool IsInterruptedDownloadAutoResumable(content::DownloadItem* download_item) {
 
 }  // namespace
 
-// JNI methods
-static void Init(JNIEnv* env, const JavaParamRef<jobject>& obj) {
-  DownloadController::GetInstance()->Init(env, obj);
-}
-
 static void OnAcquirePermissionResult(
     JNIEnv* env,
-    const JavaParamRef<jobject>& obj,
+    const JavaParamRef<jclass>& clazz,
     jlong callback_id,
     jboolean granted,
     const JavaParamRef<jstring>& jpermission_to_update) {
@@ -155,13 +150,6 @@ static void OnRequestFileAccessResult(
   cb.Run(granted);
 }
 
-struct DownloadController::JavaObject {
-  ScopedJavaLocalRef<jobject> Controller(JNIEnv* env) {
-    return GetRealObject(env, obj_);
-  }
-  jweak obj_;
-};
-
 // static
 bool DownloadController::RegisterDownloadController(JNIEnv* env) {
   return RegisterNativesImpl(env);
@@ -194,24 +182,9 @@ DownloadController* DownloadController::GetInstance() {
   return base::Singleton<DownloadController>::get();
 }
 
-DownloadController::DownloadController()
-    : java_object_(NULL) {
-}
-
-DownloadController::~DownloadController() {
-  if (java_object_) {
-    JNIEnv* env = base::android::AttachCurrentThread();
-    env->DeleteWeakGlobalRef(java_object_->obj_);
-    delete java_object_;
-    base::android::CheckException(env);
-  }
-}
+DownloadController::DownloadController() = default;
 
-// Initialize references to Java object.
-void DownloadController::Init(JNIEnv* env, jobject obj) {
-  java_object_ = new JavaObject;
-  java_object_->obj_ = env->NewWeakGlobalRef(obj);
-}
+DownloadController::~DownloadController() = default;
 
 void DownloadController::AcquireFileAccessPermission(
     const content::ResourceRequestInfo::WebContentsGetter& web_contents_getter,
@@ -230,8 +203,7 @@ void DownloadController::AcquireFileAccessPermission(
   intptr_t callback_id =
       reinterpret_cast<intptr_t>(new AcquirePermissionCallback(callback));
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_DownloadController_requestFileAccess(
-      env, GetJavaObject()->Controller(env), callback_id);
+  Java_DownloadController_requestFileAccess(env, callback_id);
 }
 
 void DownloadController::CreateAndroidDownload(
@@ -287,9 +259,8 @@ void DownloadController::StartAndroidDownloadInternal(
   WebContents* web_contents = wc_getter.Run();
   if (web_contents) {
     TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
-    if (tab && !tab->GetJavaObject().is_null()) {
+    if (tab && !tab->GetJavaObject().is_null())
       Java_DownloadController_closeTabIfBlank(env, tab->GetJavaObject());
-    }
   }
 }
 
@@ -297,8 +268,7 @@ bool DownloadController::HasFileAccessPermission() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   JNIEnv* env = base::android::AttachCurrentThread();
-  return Java_DownloadController_hasFileAccess(
-      env, GetJavaObject()->Controller(env));
+  return Java_DownloadController_hasFileAccess(env);
 }
 
 void DownloadController::OnDownloadStarted(
@@ -308,10 +278,8 @@ void DownloadController::OnDownloadStarted(
   // For dangerous item, we need to show the dangerous infobar before the
   // download can start.
   JNIEnv* env = base::android::AttachCurrentThread();
-  if (!download_item->IsDangerous()) {
-    Java_DownloadController_onDownloadStarted(
-        env, GetJavaObject()->Controller(env));
-  }
+  if (!download_item->IsDangerous())
+    Java_DownloadController_onDownloadStarted(env);
 
   WebContents* web_contents = download_item->GetWebContents();
   if (web_contents) {
@@ -341,8 +309,7 @@ void DownloadController::OnDownloadUpdated(DownloadItem* item) {
       DownloadManagerService::CreateJavaDownloadInfo(env, item);
   switch (item->GetState()) {
     case DownloadItem::IN_PROGRESS: {
-      Java_DownloadController_onDownloadUpdated(
-          env, GetJavaObject()->Controller(env), j_item);
+      Java_DownloadController_onDownloadUpdated(env, j_item);
       break;
     }
     case DownloadItem::COMPLETE:
@@ -351,14 +318,12 @@ void DownloadController::OnDownloadUpdated(DownloadItem* item) {
       item->RemoveObserver(this);
 
       // Call onDownloadCompleted
-      Java_DownloadController_onDownloadCompleted(
-          env, GetJavaObject()->Controller(env), j_item);
+      Java_DownloadController_onDownloadCompleted(env, j_item);
       DownloadController::RecordDownloadCancelReason(
              DownloadController::CANCEL_REASON_NOT_CANCELED);
       break;
     case DownloadItem::CANCELLED:
-      Java_DownloadController_onDownloadCancelled(
-          env, GetJavaObject()->Controller(env), j_item);
+      Java_DownloadController_onDownloadCancelled(env, j_item);
       DownloadController::RecordDownloadCancelReason(
           DownloadController::CANCEL_REASON_OTHER_NATIVE_RESONS);
       break;
@@ -366,8 +331,7 @@ void DownloadController::OnDownloadUpdated(DownloadItem* item) {
       // When device loses/changes network, we get a NETWORK_TIMEOUT,
       // NETWORK_FAILED or NETWORK_DISCONNECTED error. Download should auto
       // resume in this case.
-      Java_DownloadController_onDownloadInterrupted(
-          env, GetJavaObject()->Controller(env), j_item,
+      Java_DownloadController_onDownloadInterrupted(env, j_item,
           IsInterruptedDownloadAutoResumable(item));
       item->RemoveObserver(this);
       break;
@@ -387,20 +351,6 @@ void DownloadController::OnDangerousDownload(DownloadItem* item) {
         InfoBarService::FromWebContents(web_contents), item);
 }
 
-DownloadController::JavaObject*
-    DownloadController::GetJavaObject() {
-  if (!java_object_) {
-    // Initialize Java DownloadController by calling
-    // DownloadController.getInstance(), which will call Init()
-    // if Java DownloadController is not instantiated already.
-    JNIEnv* env = base::android::AttachCurrentThread();
-    Java_DownloadController_getInstance(env);
-  }
-
-  DCHECK(java_object_);
-  return java_object_;
-}
-
 void DownloadController::StartContextMenuDownload(
     const ContextMenuParams& params, WebContents* web_contents, bool is_link,
     const std::string& extra_headers) {

chrome/browser/android/download/download_controller.h

@@ -33,9 +33,6 @@ class DownloadController : public DownloadControllerBase {
 
   static bool RegisterDownloadController(JNIEnv* env);
 
-  // Called when DownloadController Java object is instantiated.
-  void Init(JNIEnv* env, jobject obj);
-
   // DownloadControllerBase implementation.
   void AcquireFileAccessPermission(
       const content::ResourceRequestInfo::WebContentsGetter& wc_getter,
@@ -67,7 +64,6 @@ class DownloadController : public DownloadControllerBase {
       AcquirePermissionCallback;
 
  private:
-  struct JavaObject;
   friend struct base::DefaultSingletonTraits<DownloadController>;
   DownloadController();
   ~DownloadController() override;
@@ -99,11 +95,6 @@ class DownloadController : public DownloadControllerBase {
       const content::ResourceRequestInfo::WebContentsGetter& wc_getter,
       const DownloadInfo& info, bool allowed);
 
-  // Creates Java object if it is not created already and returns it.
-  JavaObject* GetJavaObject();
-
-  JavaObject* java_object_;
-
   std::string default_file_name_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadController);