Switch to use download GUID to indentify download items

Download GUID is introduced by
http://crrev.com/eef62b0282ec19ebc040785d0b7ac36de398cbc1
Unlike download Id, the GUID can live across browser sessions and will
not be reused.
This change switches the java class to also use the
GUID.
Chrome will temporarily use the old download Id as the notification Id.
This is because the android notification requires an int to identify it,
rather than a string.
In the future, we will generate the notification Id from java side.

The download Ids generated by the Android DownloadManager are not
affected by this CL.
However, they are only used in OMA downloads and when we call
addCompletedDownload().

BUG=593020

Review URL: https://codereview.chromium.org/1809203006

Cr-Commit-Position: refs/heads/master@{#383443}

6 files changed, 65 insertions, 48 deletions

diff --git a/content/browser/android/download_controller_android_impl.cc b/content/browser/android/download_controller_android_impl.cc
index a09532f..abbd4c3 100644
--- a/content/browser/android/download_controller_android_impl.cc
+++ b/content/browser/android/download_controller_android_impl.cc
@@ -453,6 +453,8 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) {
     OnDangerousDownload(item);
 
   JNIEnv* env = base::android::AttachCurrentThread();
+  ScopedJavaLocalRef<jstring> jguid =
+      ConvertUTF8ToJavaString(env, item->GetGuid());
   ScopedJavaLocalRef<jstring> jurl =
       ConvertUTF8ToJavaString(env, item->GetURL().spec());
   ScopedJavaLocalRef<jstring> jmime_type =
@@ -473,8 +475,9 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) {
       Java_DownloadController_onDownloadUpdated(
           env, GetJavaObject()->Controller(env).obj(), jurl.obj(),
           jmime_type.obj(), jfilename.obj(), jpath.obj(),
-          item->GetReceivedBytes(), item->GetId(), item->PercentComplete(),
-          time_delta.InMilliseconds(), item->HasUserGesture(), item->IsPaused(),
+          item->GetReceivedBytes(), item->GetId(), jguid.obj(),
+          item->PercentComplete(), time_delta.InMilliseconds(),
+          item->HasUserGesture(), item->IsPaused(),
           // Get all requirements that allows a download to be resumable.
           !item->GetBrowserContext()->IsOffTheRecord());
       break;
@@ -488,12 +491,13 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) {
       Java_DownloadController_onDownloadCompleted(
           env, GetJavaObject()->Controller(env).obj(), jurl.obj(),
           jmime_type.obj(), jfilename.obj(), jpath.obj(),
-          item->GetReceivedBytes(), item->GetId(),
+          item->GetReceivedBytes(), item->GetId(), jguid.obj(),
           joriginal_url.obj(), jreferrer_url.obj(), item->HasUserGesture());
       break;
     case DownloadItem::CANCELLED:
       Java_DownloadController_onDownloadCancelled(
-          env, GetJavaObject()->Controller(env).obj(), item->GetId());
+          env, GetJavaObject()->Controller(env).obj(), item->GetId(),
+          jguid.obj());
       break;
     case DownloadItem::INTERRUPTED:
       // When device loses/changes network, we get a NETWORK_TIMEOUT,
@@ -502,8 +506,9 @@ void DownloadControllerAndroidImpl::OnDownloadUpdated(DownloadItem* item) {
       Java_DownloadController_onDownloadInterrupted(
           env, GetJavaObject()->Controller(env).obj(), jurl.obj(),
           jmime_type.obj(), jfilename.obj(), jpath.obj(),
-          item->GetReceivedBytes(), item->GetId(), item->CanResume(),
-          IsInterruptedDownloadAutoResumable(item));
+          item->GetReceivedBytes(), item->GetId(), jguid.obj(),
+          item->CanResume(), IsInterruptedDownloadAutoResumable(item));
+      item->RemoveObserver(this);
       break;
     case DownloadItem::MAX_DOWNLOAD_STATE:
       NOTREACHED();
@@ -514,12 +519,14 @@ void DownloadControllerAndroidImpl::OnDangerousDownload(DownloadItem* item) {
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> jfilename = ConvertUTF8ToJavaString(
       env, item->GetTargetFilePath().BaseName().value());
+  ScopedJavaLocalRef<jstring> jguid =
+      ConvertUTF8ToJavaString(env, item->GetGuid());
   ScopedJavaLocalRef<jobject> view_core = GetContentViewCoreFromWebContents(
       item->GetWebContents());
   if (!view_core.is_null()) {
     Java_DownloadController_onDangerousDownload(
         env, GetJavaObject()->Controller(env).obj(), view_core.obj(),
-        jfilename.obj(), item->GetId());
+        jfilename.obj(), jguid.obj());
   }
 }
 
@@ -559,12 +566,14 @@ void DownloadControllerAndroidImpl::StartContextMenuDownload(
 }
 
 void DownloadControllerAndroidImpl::DangerousDownloadValidated(
-    WebContents* web_contents, int download_id, bool accept) {
+    WebContents* web_contents,
+    const std::string& download_guid,
+    bool accept) {
   if (!web_contents)
     return;
   DownloadManagerImpl* dlm = static_cast<DownloadManagerImpl*>(
       BrowserContext::GetDownloadManager(web_contents->GetBrowserContext()));
-  DownloadItem* item = dlm->GetDownload(download_id);
+  DownloadItem* item = dlm->GetDownloadByGuid(download_guid);
   if (!item)
     return;
   if (accept)
diff --git a/content/browser/android/download_controller_android_impl.h b/content/browser/android/download_controller_android_impl.h
index 9045d29..78d6544 100644
--- a/content/browser/android/download_controller_android_impl.h
+++ b/content/browser/android/download_controller_android_impl.h
@@ -102,7 +102,7 @@ class DownloadControllerAndroidImpl : public DownloadControllerAndroid {
                                 bool is_link,
                                 const std::string& extra_headers) override;
   void DangerousDownloadValidated(WebContents* web_contents,
-                                  int download_id,
+                                  const std::string& download_guid,
                                   bool accept) override;
 
   // DownloadItem::Observer interface.
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java
index 4caff13..5153062 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewDownloadDelegate.java
@@ -26,9 +26,9 @@ public interface ContentViewDownloadDelegate {
      * Notify the host application that a download has an extension indicating
      * a dangerous file type.
      * @param filename File name of the downloaded file.
-     * @param downloadId The download id.
+     * @param downloadGuid The download GUID.
      */
-    void onDangerousDownload(String filename, int downloadId);
+    void onDangerousDownload(String filename, String downloadGuid);
 
     /**
      * Called when file access has been requested to complete a download.
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 63a5eb7..5e2c29f 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
@@ -118,11 +118,13 @@ public class DownloadController {
     /**
      * Notifies the download delegate that a download completed and passes along info about the
      * download. This can be either a POST download or a GET download with authentication.
+     * TODO(qinmin): Generate the notificationId in the java side, the native downloadId is going
+     * to be deprecated.
      */
     @CalledByNative
     private void onDownloadCompleted(String url, String mimeType, String filename, String path,
-            long contentLength, int downloadId, String originalUrl, String refererUrl,
-            boolean hasUserGesture) {
+            long contentLength, int notificationId, String downloadGuid, String originalUrl,
+            String refererUrl, boolean hasUserGesture) {
         if (sDownloadNotificationService == null) return;
         DownloadInfo downloadInfo = new DownloadInfo.Builder()
                 .setUrl(url)
@@ -131,8 +133,8 @@ public class DownloadController {
                 .setFilePath(path)
                 .setContentLength(contentLength)
                 .setDescription(filename)
-                .setDownloadId(downloadId)
-                .setHasDownloadId(true)
+                .setNotificationId(notificationId)
+                .setDownloadGuid(downloadGuid)
                 .setOriginalUrl(originalUrl)
                 .setReferer(refererUrl)
                 .setHasUserGesture(hasUserGesture)
@@ -146,7 +148,8 @@ public class DownloadController {
      */
     @CalledByNative
     private void onDownloadInterrupted(String url, String mimeType, String filename, String path,
-            long contentLength, int downloadId, boolean isResumable, boolean isAutoResumable) {
+            long contentLength, int notificationId, String downloadGuid, boolean isResumable,
+            boolean isAutoResumable) {
         if (sDownloadNotificationService == null) return;
         DownloadInfo downloadInfo = new DownloadInfo.Builder()
                 .setUrl(url)
@@ -155,8 +158,8 @@ public class DownloadController {
                 .setFilePath(path)
                 .setContentLength(contentLength)
                 .setDescription(filename)
-                .setDownloadId(downloadId)
-                .setHasDownloadId(true)
+                .setNotificationId(notificationId)
+                .setDownloadGuid(downloadGuid)
                 .setIsResumable(isResumable)
                 .build();
         sDownloadNotificationService.onDownloadInterrupted(downloadInfo, isAutoResumable);
@@ -164,14 +167,17 @@ public class DownloadController {
 
     /**
      * Called when a download was cancelled.
-     * @param downloadId Id of the download item.
+     * @param notificationId Notification Id of the download item.
+     * @param downloadGuid GUID of the download item.
+     * TODO(qinmin): Generate the notificationId in the java side, the native downloadId is going
+     * to be deprecated.
      */
     @CalledByNative
-    private void onDownloadCancelled(int downloadId) {
+    private void onDownloadCancelled(int notificationId, String downloadGuid) {
         if (sDownloadNotificationService == null) return;
         DownloadInfo downloadInfo = new DownloadInfo.Builder()
-                .setDownloadId(downloadId)
-                .setHasDownloadId(true)
+                .setNotificationId(notificationId)
+                .setDownloadGuid(downloadGuid)
                 .build();
         sDownloadNotificationService.onDownloadCancelled(downloadInfo);
     }
@@ -179,10 +185,12 @@ public class DownloadController {
     /**
      * Notifies the download delegate about progress of a download. Downloads that use Chrome
      * network stack use custom notification to display the progress of downloads.
+     * TODO(qinmin): Generate the notificationId in the java side, the native downloadId is going
+     * to be deprecated.
      */
     @CalledByNative
-    private void onDownloadUpdated(String url, String mimeType, String filename,
-            String path, long contentLength, int downloadId, int percentCompleted,
+    private void onDownloadUpdated(String url, String mimeType, String filename, String path,
+            long contentLength, int notificationId, String downloadGuid, int percentCompleted,
             long timeRemainingInMs, boolean hasUserGesture, boolean isPaused, boolean isResumable) {
         if (sDownloadNotificationService == null) return;
         DownloadInfo downloadInfo = new DownloadInfo.Builder()
@@ -192,8 +200,8 @@ public class DownloadController {
                 .setFilePath(path)
                 .setContentLength(contentLength)
                 .setDescription(filename)
-                .setDownloadId(downloadId)
-                .setHasDownloadId(true)
+                .setNotificationId(notificationId)
+                .setDownloadGuid(downloadGuid)
                 .setPercentCompleted(percentCompleted)
                 .setTimeRemainingInMillis(timeRemainingInMs)
                 .setHasUserGesture(hasUserGesture)
@@ -207,11 +215,10 @@ public class DownloadController {
      * Notifies the download delegate that a dangerous download started.
      */
     @CalledByNative
-    private void onDangerousDownload(ContentViewCore view, String filename,
-            int downloadId) {
+    private void onDangerousDownload(ContentViewCore view, String filename, String downloadGuid) {
         ContentViewDownloadDelegate downloadDelegate = downloadDelegateFromView(view);
         if (downloadDelegate != null) {
-            downloadDelegate.onDangerousDownload(filename, downloadId);
+            downloadDelegate.onDangerousDownload(filename, downloadGuid);
         }
     }
 
diff --git a/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java b/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java
index 39da754..f6297be 100644
--- a/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java
+++ b/content/public/android/java/src/org/chromium/content/browser/DownloadInfo.java
@@ -18,8 +18,8 @@ public final class DownloadInfo {
     private final String mReferer;
     private final String mOriginalUrl;
     private final long mContentLength;
-    private final boolean mHasDownloadId;
-    private final int mDownloadId;
+    private final int mNotificationId;
+    private final String mDownloadGuid;
     private final boolean mHasUserGesture;
     private final String mContentDisposition;
     private final boolean mIsGETRequest;
@@ -39,8 +39,8 @@ public final class DownloadInfo {
         mReferer = builder.mReferer;
         mOriginalUrl = builder.mOriginalUrl;
         mContentLength = builder.mContentLength;
-        mHasDownloadId = builder.mHasDownloadId;
-        mDownloadId = builder.mDownloadId;
+        mNotificationId = builder.mNotificationId;
+        mDownloadGuid = builder.mDownloadGuid;
         mHasUserGesture = builder.mHasUserGesture;
         mIsGETRequest = builder.mIsGETRequest;
         mContentDisposition = builder.mContentDisposition;
@@ -94,12 +94,12 @@ public final class DownloadInfo {
         return mIsGETRequest;
     }
 
-    public boolean hasDownloadId() {
-        return mHasDownloadId;
+    public int getNotificationId() {
+        return mNotificationId;
     }
 
-    public int getDownloadId() {
-        return mDownloadId;
+    public String getDownloadGuid() {
+        return mDownloadGuid;
     }
 
     public boolean hasUserGesture() {
@@ -144,8 +144,8 @@ public final class DownloadInfo {
         private String mOriginalUrl;
         private long mContentLength;
         private boolean mIsGETRequest;
-        private boolean mHasDownloadId;
-        private int mDownloadId;
+        private int mNotificationId;
+        private String mDownloadGuid;
         private boolean mHasUserGesture;
         private String mContentDisposition;
         private int mPercentCompleted = -1;
@@ -208,13 +208,13 @@ public final class DownloadInfo {
             return this;
         }
 
-        public Builder setHasDownloadId(boolean hasDownloadId) {
-            mHasDownloadId = hasDownloadId;
+        public Builder setNotificationId(int notificationId) {
+            mNotificationId = notificationId;
             return this;
         }
 
-        public Builder setDownloadId(int downloadId) {
-            mDownloadId = downloadId;
+        public Builder setDownloadGuid(String downloadGuid) {
+            mDownloadGuid = downloadGuid;
             return this;
         }
 
@@ -270,8 +270,8 @@ public final class DownloadInfo {
                     .setReferer(downloadInfo.getReferer())
                     .setOriginalUrl(downloadInfo.getOriginalUrl())
                     .setContentLength(downloadInfo.getContentLength())
-                    .setHasDownloadId(downloadInfo.hasDownloadId())
-                    .setDownloadId(downloadInfo.getDownloadId())
+                    .setNotificationId(downloadInfo.getNotificationId())
+                    .setDownloadGuid(downloadInfo.getDownloadGuid())
                     .setHasUserGesture(downloadInfo.hasUserGesture())
                     .setContentDisposition(downloadInfo.getContentDisposition())
                     .setIsGETRequest(downloadInfo.isGETRequest())
diff --git a/content/public/browser/android/download_controller_android.h b/content/public/browser/android/download_controller_android.h
index e048afd..2ef664e 100644
--- a/content/public/browser/android/download_controller_android.h
+++ b/content/public/browser/android/download_controller_android.h
@@ -41,8 +41,9 @@ class CONTENT_EXPORT DownloadControllerAndroid : public DownloadItem::Observer {
       bool is_link, const std::string& extra_headers) = 0;
 
   // Called when a dangerous download item is verified or rejected.
-  virtual void DangerousDownloadValidated(
-      WebContents* web_contents, int download_id, bool accept) = 0;
+  virtual void DangerousDownloadValidated(WebContents* web_contents,
+                                          const std::string& download_guid,
+                                          bool accept) = 0;
 
   // Callback when user permission prompt finishes. Args: whether file access
   // permission is acquired.