(Android) Blocking multiple scheduled downloads for the same URL

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

Index: chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
index f6bfd8eb47488f34ed14db22d72e1518857e0a4d..b0c137707dc6493552640d4d101c0b301351663d 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java
@@ -22,6 +22,7 @@ import android.text.TextUtils;
 
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ApplicationStatus;
+import org.chromium.base.Callback;
 import org.chromium.base.ContentUriUtils;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
@@ -40,6 +41,7 @@ import org.chromium.chrome.browser.download.ui.DownloadFilter;
 import org.chromium.chrome.browser.download.ui.DownloadHistoryItemWrapper;
 import org.chromium.chrome.browser.offlinepages.ClientId;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
+import org.chromium.chrome.browser.offlinepages.SavePageRequest;
 import org.chromium.chrome.browser.offlinepages.downloads.OfflinePageDownloadBridge;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
@@ -180,25 +182,41 @@ public class DownloadUtils {
     }
 
     /**
-     * Trigger the download of an Offline Page.
+     * Trigger the download of an Offline Page
+     * (only when page is not currently downloaded).
      * @param context Context to pull resources from.
      */
     public static void downloadOfflinePage(Context context, Tab tab) {
-        if (tab.isShowingErrorPage()) {
-            // The download needs to be scheduled to happen at later time due to current network
-            // error.
-            ClientId clientId = ClientId.createGuidClientIdForNamespace(
-                    OfflinePageBridge.ASYNC_NAMESPACE);
-            final OfflinePageBridge bridge = OfflinePageBridge.getForProfile(tab.getProfile());
-            bridge.savePageLater(tab.getUrl(), clientId, true /* userRequested */);
-        } else {
-            // Otherwise, the download can be started immediately.
-            final OfflinePageDownloadBridge bridge =
-                    new OfflinePageDownloadBridge(tab.getProfile());
-            bridge.startDownload(tab);
-            bridge.destroy();
-            DownloadUtils.recordDownloadPageMetrics(tab);
-        }
+        final Tab mTab = tab;
+        final Context mContext = context;
+        OfflinePageBridge.getForProfile(mTab.getProfile())
+                .getRequestsInQueue(new Callback<SavePageRequest[]>() {

[Dmitry Titov, 2017/02/17 02:40:59]

This is a SQLLite operation, with disk IO. It can take seconds on a heavy-loaded
low-end device. The chances are, the tab is already killed/navigated away when
it is completed.

Also, the UX is not good - if this is originated from UI, the user has a feeling
his request is ignored. Instead, the user should be prevented from clickign at
the buttons at the first place (by disabling controls). See how the download
buttons are disabled when the page is Incognito etc (isAllowedToDownloadPage()
function below this one)

[marcin, 2017/02/17 08:02:32]

On heavy-loaded low-end devices people the most often don't download many pages
at all.
When user will be prevented from clicking, it can make feeling, that this page
(type)
cannot be downloaded by Chrome.

[marcin, 2017/02/18 22:38:47]

maybe I will do toast in case, when user try the same Url for the same time.
WDYT?

[dgn, 2017/02/21 13:51:14]

Backing this this assumption with numbers would be nice. I'd think low-end
devices and data-conscious go often together, so these users would try to avoid
their phone loading pages from the web, which will also be slower than loading
locally.

[marcin, 2017/02/21 18:24:31]

Patch allows for saving device resources (no multiple downloads) and saving user
data connection.

Cost is one (?) SQL operation, which shouldn't be very expensive on device,
where
not too many  multiple downloads are done (in my opinion - low-end, where it's
even more important not too have useless things).

If you think, that it's better to open many connections, I will have to close
this
topic, because it doesn't have sense.

+                    @Override
+                    public void onResult(SavePageRequest[] requests) {
+                        for (int i = 0, len = requests.length; i < len; i++) {
+                            if (requests[i].getUrl().equals(mTab.getUrl())) {
+                                // Download was scheduled earlier.
+                                return;
+                            }
+                        }
+                        if (mTab.isShowingErrorPage()) {
+                            // The download needs to be scheduled to happen
+                            // at later time due to current network error.
+                            ClientId clientId = ClientId.createGuidClientIdForNamespace(
+                                    OfflinePageBridge.ASYNC_NAMESPACE);
+                            OfflinePageBridge.getForProfile(mTab.getProfile())
+                                    .savePageLater(
+                                            mTab.getUrl(), clientId, true /* userRequested */);
+                        } else {
+                            // Otherwise, the download can be started immediately.
+                            OfflinePageDownloadBridge bridge =
+                                    new OfflinePageDownloadBridge(mTab.getProfile());
+                            bridge.startDownload(mTab);
+                            bridge.destroy();
+                            DownloadUtils.recordDownloadPageMetrics(mTab);
+                        }
+                    }
+                });
     }
 
     /**