(Android) Blocking multiple scheduled downloads for the same URL

chrome/browser/net/net_error_tab_helper.cc

 // Copyright (c) 2012 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/net/net_error_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/io_thread.h"
@@ skipping 11 matching lines @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/common/associated_interface_provider.h"
 #include "ipc/ipc_message_macros.h"
 #include "net/base/net_errors.h"
 #include "url/gurl.h"
 
 #if defined(OS_ANDROID)
 #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)
 
 using content::BrowserContext;
 using content::BrowserThread;
 using content::WebContents;
 using content::WebContentsObserver;
 using error_page::DnsProbeStatus;
 using error_page::DnsProbeStatusToString;
 using ui::PageTransition;
@@ skipping 247 matching lines @@
 
   ShowNetworkDiagnosticsDialog(web_contents(), sanitized_url);
 }
 
 #if defined(OS_ANDROID)
 void NetErrorTabHelper::DownloadPageLaterHelper(const GURL& page_url) {
   offline_pages::RequestCoordinator* request_coordinator =
       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)
 
 }  // namespace chrome_browser_net