Added calls to DownloadManagerBridge from native

Added calls to android DownloadManager (AddCompletedDownload and
RemoveCompletedDownload) from native offline content provider.
Also added a feature to selectively disable the native path for now.

Bug: 933854
Change-Id: I178ab17063f6914b523219c308a6abaa3133452c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1422747
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637907}

chrome/android/BUILD.gn

@@ -2187,6 +2187,7 @@ generate_jni("jni_headers") {
     "java/src/org/chromium/chrome/browser/download/DownloadInfo.java",
     "java/src/org/chromium/chrome/browser/download/DownloadItem.java",
     "java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java",
+    "java/src/org/chromium/chrome/browser/download/DownloadManagerBridge.java",
     "java/src/org/chromium/chrome/browser/download/DownloadManagerService.java",
     "java/src/org/chromium/chrome/browser/download/DownloadMediaData.java",
     "java/src/org/chromium/chrome/browser/download/DownloadMediaParserBridge.java",

chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java

@@ -219,6 +219,8 @@ public abstract class ChromeFeatureList {
     public static final String DOWNLOAD_HOME_V2 = "DownloadHomeV2";
     public static final String DOWNLOADS_FOREGROUND = "DownloadsForeground";
     public static final String DOWNLOADS_AUTO_RESUMPTION_NATIVE = "DownloadsAutoResumptionNative";
+    public static final String DOWNLOAD_OFFLINE_CONTENT_PROVIDER =
+            "UseDownloadOfflineContentProvider";
     public static final String DOWNLOADS_LOCATION_CHANGE = "DownloadsLocationChange";
     public static final String EPHEMERAL_TAB = "EphemeralTab";
     public static final String EXPERIMENTAL_APP_BANNERS = "ExperimentalAppBanners";

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

@@ -17,12 +17,15 @@ import android.text.TextUtils;
 import org.chromium.base.Callback;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.base.StrictModeContext;
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.task.AsyncTask;
 
 import java.io.File;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.concurrent.RejectedExecutionException;
 
 /**
  * A wrapper for Android DownloadManager to provide utility functions.
@@ -90,7 +93,9 @@ public class DownloadManagerBridge {
         NotificationManagerCompat notificationManager =
                 NotificationManagerCompat.from(getContext());
         boolean useSystemNotification = !notificationManager.areNotificationsEnabled();
-        long downloadId = -1;
+        long downloadId = getDownloadIdForDownloadGuid(downloadGuid);
+        if (downloadId != DownloadItem.INVALID_DOWNLOAD_ID) return downloadId;
+
         if (Build.VERSION.SDK_INT > Build.VERSION_CODES.M) {
             Class<?> c = manager.getClass();
             try {
@@ -124,6 +129,7 @@ public class DownloadManagerBridge {
      * @param downloadGuid The GUID of the download.
      * @param externallyRemoved If download is externally removed in other application.
      */
+    @CalledByNative
     public static void removeCompletedDownload(String downloadGuid, boolean externallyRemoved) {
         long downloadId = removeDownloadIdMapping(downloadGuid);
 
@@ -201,6 +207,13 @@ public class DownloadManagerBridge {
         return result;
     }
 
+    /** @return The android DownloadManager's download ID for the given download. */
+    public static long getDownloadIdForDownloadGuid(String downloadGuid) {
+        try (StrictModeContext unused = StrictModeContext.allowDiskReads()) {
+            return getSharedPreferences().getLong(downloadGuid, INVALID_SYSTEM_DOWNLOAD_ID);
+        }
+    }
+
     /**
      * Inserts a new download ID mapping into the SharedPreferences
      * @param downloadId system download ID from Android DownloadManager.
@@ -235,11 +248,6 @@ public class DownloadManagerBridge {
         return downloadId;
     }
 
-    /** @return The android DownloadManager's download ID for the given download. */
-    private static long getDownloadIdForDownloadGuid(String downloadGuid) {
-        return getSharedPreferences().getLong(downloadGuid, INVALID_SYSTEM_DOWNLOAD_ID);
-    }
-
     /**
      * Lazily retrieve the SharedPreferences when needed. Since download operations are not very
      * frequent, no need to load all SharedPreference entries into a hashmap in the memory.
@@ -254,6 +262,31 @@ public class DownloadManagerBridge {
         return ContextUtils.getApplicationContext();
     }
 
+    @CalledByNative
+    private static void addCompletedDownload(String fileName, String description, String mimeType,
+            String filePath, long fileSizeBytes, String originalUrl, String referrer,
+            String downloadGuid, long callbackId) {
+        AsyncTask<Long> task = new AsyncTask<Long>() {
+            @Override
+            protected Long doInBackground() {
+                return addCompletedDownload(fileName, description, mimeType, filePath,
+                        fileSizeBytes, originalUrl, referrer, downloadGuid);
+            }
+
+            @Override
+            protected void onPostExecute(Long downloadId) {
+                nativeOnAddCompletedDownloadDone(callbackId, downloadId);
+            }
+        };
+        try {
+            task.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
+        } catch (RejectedExecutionException e) {
+            // Reaching thread limit, update will be reschduled for the next run.
+            Log.e(TAG, "Thread limit reached, reschedule notification update later.");
+            nativeOnAddCompletedDownloadDone(callbackId, DownloadItem.INVALID_DOWNLOAD_ID);
+        }
+    }
+
     private static int getDownloadStatus(int downloadManagerStatus) {
         switch (downloadManagerStatus) {
             case DownloadManager.STATUS_SUCCESSFUL:
@@ -390,4 +423,6 @@ public class DownloadManagerBridge {
             mCallback.onResult(enqueueResult);
         }
     }
+
+    private static native void nativeOnAddCompletedDownloadDone(long callbackId, long downloadId);
 }

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

@@ -36,6 +36,7 @@ import org.chromium.base.task.AsyncTask;
 import org.chromium.base.task.PostTask;
 import org.chromium.base.task.TaskTraits;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.download.DownloadManagerBridge.DownloadEnqueueRequest;
 import org.chromium.chrome.browser.download.DownloadManagerBridge.DownloadEnqueueResponse;
 import org.chromium.chrome.browser.download.DownloadMetrics.DownloadOpenSource;
@@ -329,6 +330,8 @@ public class DownloadManagerService
         DownloadInfo newInfo =
                 DownloadInfo.Builder.fromDownloadInfo(downloadInfo).setMimeType(mimeType).build();
         DownloadItem downloadItem = new DownloadItem(false, newInfo);
+        downloadItem.setSystemDownloadId(
+                DownloadManagerBridge.getDownloadIdForDownloadGuid(downloadInfo.getDownloadGuid()));
         updateDownloadProgress(downloadItem, status);
     }
 
@@ -537,7 +540,9 @@ public class DownloadManagerService
             public Pair<Boolean, Boolean> doInBackground() {
                 boolean success =
                         ContentUriUtils.isContentUri(item.getDownloadInfo().getFilePath());
-                if (!success) {
+                if (!success
+                        && !ChromeFeatureList.isEnabled(
+                                ChromeFeatureList.DOWNLOAD_OFFLINE_CONTENT_PROVIDER)) {
                     long systemDownloadId = DownloadManagerBridge.addCompletedDownload(
                             info.getFileName(), info.getDescription(), info.getMimeType(),
                             info.getFilePath(), info.getBytesReceived(), info.getOriginalUrl(),
@@ -1111,6 +1116,10 @@ public class DownloadManagerService
             removeDownloadProgress(downloadGuid);
         });
 
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.DOWNLOAD_OFFLINE_CONTENT_PROVIDER)) {
+            return;
+        }
+
         PostTask.postTask(TaskTraits.BEST_EFFORT_MAY_BLOCK, () -> {
             DownloadManagerBridge.removeCompletedDownload(downloadGuid, externallyRemoved);
         });

chrome/browser/BUILD.gn

@@ -2263,6 +2263,8 @@ jumbo_split_static_library("browser") {
       "android/download/download_location_dialog_bridge.h",
       "android/download/download_location_dialog_bridge_impl.cc",
       "android/download/download_location_dialog_bridge_impl.h",
+      "android/download/download_manager_bridge.cc",
+      "android/download/download_manager_bridge.h",
       "android/download/download_manager_service.cc",
       "android/download/download_manager_service.h",
       "android/download/download_media_parser.cc",

chrome/browser/android/chrome_feature_list.cc

@@ -64,6 +64,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &contextual_suggestions::kContextualSuggestionsIPHReverseScroll,
     &contextual_suggestions::kContextualSuggestionsOptOut,
     &download::features::kDownloadAutoResumptionNative,
+    &download::features::kUseDownloadOfflineContentProvider,
     &features::kAllowStartingServiceManagerOnly,
     &features::kAppNotificationStatusMessaging,
     &features::kClearOldBrowsingData,

chrome/browser/android/download/download_manager_bridge.cc

+// Copyright 2019 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/android/download/download_manager_bridge.h"
+
+#include <memory>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/feature_list.h"
+#include "components/download/public/common/download_features.h"
+#include "content/public/browser/browser_thread.h"
+#include "jni/DownloadManagerBridge_jni.h"
+#include "url/gurl.h"
+
+using base::android::ConvertUTF8ToJavaString;
+using base::android::JavaParamRef;
+using base::android::ScopedJavaLocalRef;
+using content::BrowserThread;
+using download::DownloadItem;
+
+static void JNI_DownloadManagerBridge_OnAddCompletedDownloadDone(
+    JNIEnv* env,
+    jlong callback_id,
+    jlong download_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(callback_id);
+
+  // Convert java long long int to c++ pointer, take ownership.
+  std::unique_ptr<AddCompletedDownloadCallback> cb(
+      reinterpret_cast<AddCompletedDownloadCallback*>(callback_id));
+  std::move(*cb).Run(download_id);
+}
+
+void DownloadManagerBridge::AddCompletedDownload(
+    download::DownloadItem* download,
+    AddCompletedDownloadCallback callback) {
+  if (!base::FeatureList::IsEnabled(
+          download::features::kUseDownloadOfflineContentProvider)) {
+    return;
+  }
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> jfile_name =
+      ConvertUTF8ToJavaString(env, download->GetFileNameToReportUser().value());
+  ScopedJavaLocalRef<jstring> jmime_type =
+      ConvertUTF8ToJavaString(env, download->GetMimeType());
+  ScopedJavaLocalRef<jstring> jfile_path =
+      ConvertUTF8ToJavaString(env, download->GetTargetFilePath().value());
+  int64_t file_size = download->GetReceivedBytes();
+  ScopedJavaLocalRef<jstring> joriginal_url =
+      ConvertUTF8ToJavaString(env, download->GetOriginalUrl().spec());
+  ScopedJavaLocalRef<jstring> jreferer = base::android::ConvertUTF8ToJavaString(
+      env, download->GetReferrerUrl().spec());
+  ScopedJavaLocalRef<jstring> jdownload_guid =
+      base::android::ConvertUTF8ToJavaString(env, download->GetGuid());
+
+  // Make copy on the heap so we can pass the pointer through JNI.
+  intptr_t callback_id = reinterpret_cast<intptr_t>(
+      new AddCompletedDownloadCallback(std::move(callback)));
+
+  Java_DownloadManagerBridge_addCompletedDownload(
+      env, jfile_name, jfile_name, jmime_type, jfile_path, file_size,
+      joriginal_url, jreferer, jdownload_guid, callback_id);
+}
+
+void DownloadManagerBridge::RemoveCompletedDownload(
+    download::DownloadItem* download) {
+  if (!base::FeatureList::IsEnabled(
+          download::features::kUseDownloadOfflineContentProvider)) {
+    return;
+  }
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> jdownload_guid =
+      base::android::ConvertUTF8ToJavaString(env, download->GetGuid());
+  Java_DownloadManagerBridge_removeCompletedDownload(
+      env, jdownload_guid, download->GetFileExternallyRemoved());
+}

chrome/browser/android/download/download_manager_bridge.h

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_MANAGER_BRIDGE_H_
+#define CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_MANAGER_BRIDGE_H_
+
+#include "base/callback.h"
+#include "components/download/public/common/download_item.h"
+
+using DownloadItem = download::DownloadItem;
+using AddCompletedDownloadCallback = base::OnceCallback<void(int64_t)>;
+
+// This class pairs with DownloadManagerBridge on Java side, that handles all
+// the android DownloadManager related functionalities. Both classes have only
+// static functions.
+class DownloadManagerBridge {
+ public:
+  static void AddCompletedDownload(DownloadItem* download,
+                                   AddCompletedDownloadCallback callback);
+  static void RemoveCompletedDownload(DownloadItem* download);
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadManagerBridge);
+};
+
+#endif  // CHROME_BROWSER_ANDROID_DOWNLOAD_DOWNLOAD_MANAGER_BRIDGE_H_

chrome/browser/download/download_offline_content_provider.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/time/time.h"
+#include "build/build_config.h"
 #include "chrome/browser/download/image_thumbnail_request.h"
 #include "chrome/browser/download/offline_item_utils.h"
 #include "chrome/browser/offline_items_collection/offline_content_aggregator_factory.h"
@@ -21,6 +22,10 @@
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 
+#if defined(OS_ANDROID)
+#include "chrome/browser/android/download/download_manager_bridge.h"
+#endif
+
 using OfflineItemFilter = offline_items_collection::OfflineItemFilter;
 using OfflineItemState = offline_items_collection::OfflineItemState;
 using OfflineItemProgressUnit =
@@ -182,10 +187,13 @@ void DownloadOfflineContentProvider::OnDownloadUpdated(DownloadManager* manager,
   if (!ShouldShowDownloadItem(item))
     return;
 
-  for (auto& observer : observers_) {
-    observer.OnItemUpdated(
-        OfflineItemUtils::CreateOfflineItem(name_space_, item));
+  if (item->GetState() == DownloadItem::COMPLETE) {
+    // TODO(crbug.com/938152): May be move this to DownloadItem.
+    AddCompletedDownload(item);
+    return;
   }
+
+  UpdateObservers(item);
 }
 
 void DownloadOfflineContentProvider::OnDownloadRemoved(DownloadManager* manager,
@@ -193,7 +201,39 @@ void DownloadOfflineContentProvider::OnDownloadRemoved(DownloadManager* manager,
   if (!ShouldShowDownloadItem(item))
     return;
 
+#if defined(OS_ANDROID)
+  DownloadManagerBridge::RemoveCompletedDownload(item);
+#endif
+
   ContentId contentId(name_space_, item->GetGuid());
   for (auto& observer : observers_)
     observer.OnItemRemoved(contentId);
 }
+
+void DownloadOfflineContentProvider::AddCompletedDownload(DownloadItem* item) {
+#if defined(OS_ANDROID)
+  if (!item->GetTargetFilePath().IsContentUri()) {
+    DownloadManagerBridge::AddCompletedDownload(
+        item, base::BindOnce(
+                  &DownloadOfflineContentProvider::AddCompletedDownloadDone,
+                  weak_ptr_factory_.GetWeakPtr(), item));
+  } else {
+    AddCompletedDownloadDone(item, -1);
+  }
+#else
+  AddCompletedDownloadDone(item, -1);
+#endif
+}
+
+void DownloadOfflineContentProvider::AddCompletedDownloadDone(
+    DownloadItem* item,
+    int64_t system_download_id) {
+  UpdateObservers(item);
+}
+
+void DownloadOfflineContentProvider::UpdateObservers(DownloadItem* item) {
+  for (auto& observer : observers_) {
+    observer.OnItemUpdated(
+        OfflineItemUtils::CreateOfflineItem(name_space_, item));
+  }
+}

chrome/browser/download/download_offline_content_provider.h

@@ -60,6 +60,9 @@ class DownloadOfflineContentProvider
   void OnThumbnailRetrieved(const ContentId& id,
                             VisualsCallback callback,
                             const SkBitmap& bitmap);
+  void AddCompletedDownload(DownloadItem* item);
+  void AddCompletedDownloadDone(DownloadItem* item, int64_t system_download_id);
+  void UpdateObservers(DownloadItem* item);
 
   DownloadManager* manager_;
   download::AllDownloadItemNotifier download_notifier_;

components/download/public/common/download_features.cc

@@ -9,6 +9,9 @@
 namespace download {
 namespace features {
 
+const base::Feature kUseDownloadOfflineContentProvider{
+    "UseDownloadOfflineContentProvider", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kDownloadAutoResumptionNative {
   "DownloadsAutoResumptionNative",
 #if defined(OS_ANDROID)

components/download/public/common/download_features.h

@@ -11,6 +11,10 @@
 namespace download {
 namespace features {
 
+// Whether offline content provider should be used for the downloads UI..
+COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature
+    kUseDownloadOfflineContentProvider;
+
 // Whether download auto-resumptions are enabled in native.
 COMPONENTS_DOWNLOAD_EXPORT extern const base::Feature
     kDownloadAutoResumptionNative;