(Android) Blocking multiple scheduled downloads for the same URL

chrome/browser/net/net_error_tab_helper.cc

Index: chrome/browser/net/net_error_tab_helper.cc
diff --git a/chrome/browser/net/net_error_tab_helper.cc b/chrome/browser/net/net_error_tab_helper.cc
index 2cac81cd148ec5ff5d0b390d9df393c532cb4515..54dbc5ea181344b855107bb137dfa406c2ad6690 100644
--- a/chrome/browser/net/net_error_tab_helper.cc
+++ b/chrome/browser/net/net_error_tab_helper.cc
@@ -29,6 +29,7 @@
 #include "base/guid.h"
 #include "chrome/browser/android/offline_pages/request_coordinator_factory.h"
 #include "components/offline_pages/core/background/request_coordinator.h"
+#include "components/offline_pages/core/background/save_page_request.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #endif  // defined(OS_ANDROID)
 
@@ -296,12 +297,30 @@ void NetErrorTabHelper::DownloadPageLaterHelper(const GURL& page_url) {
       offline_pages::RequestCoordinatorFactory::GetForBrowserContext(
           web_contents()->GetBrowserContext());
   DCHECK(request_coordinator) << "No RequestCoordinator for SavePageLater";
-  offline_pages::ClientId client_id(
-      offline_pages::kAsyncNamespace, base::GenerateGUID());
-  request_coordinator->SavePageLater(
-      page_url, client_id, true /*user_requested*/,
-      offline_pages::RequestCoordinator::RequestAvailability::
-          ENABLED_FOR_OFFLINER);
+
+  auto request_coordinator_continuation = [](

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

I think this will be a suboptimal UX - there will be a blue (active) button,
which would do nothing when clicked. At least today it'll do something, even if
it is to create a duplicate download.

I think we should instead disable the button from the start (if the download
with the same URL is already active) and make it say "Downloading" - exactly the
same as it looks when pressed for the first time...

Also, this (creating auto lambda variable) is not a typical style of doing this
in Chrome, usually the better way is to mimic the style already in the code
around.

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

Button will do something -> will change to Downloading after pressing.
It means, that you will need to make check on the tab opening. On low-end
heavy-loaded devices...
I just used style from some Download/Offline files around. I don't see anything
wrong with this.

+      offline_pages::RequestCoordinator* req_coordinator,
+      const GURL& download_url,
+      std::vector<std::unique_ptr<offline_pages::SavePageRequest>> requests) {
+    bool found = false;
+    for (auto& request : requests) {
+      if (request->url() == download_url) {
+        found = true;
+        break;
+      }
+    }
+    if (!found) {
+      offline_pages::ClientId client_id(offline_pages::kAsyncNamespace,
+                                        base::GenerateGUID());
+      req_coordinator->SavePageLater(
+          download_url, client_id, true /*user_requested*/,
+          offline_pages::RequestCoordinator::RequestAvailability::
+              ENABLED_FOR_OFFLINER);
+    }
+  };
+
+  request_coordinator->GetAllRequests(base::Bind(
+      request_coordinator_continuation, request_coordinator, page_url));
 }
 #endif  // defined(OS_ANDROID)