Changes to fit better with the needs of the download manager.

components/offline_pages/background/request_coordinator.h

 // Copyright 2016 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 COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_
 #define COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_
 
 #include <memory>
 
 #include "base/callback.h"
@@ skipping 32 matching lines @@
                      std::unique_ptr<RequestQueue> queue,
                      std::unique_ptr<Scheduler> scheduler);
 
   ~RequestCoordinator() override;
 
   // Queues |request| to later load and save when system conditions allow.
   // Returns true if the page could be queued successfully.
   bool SavePageLater(
       const GURL& url, const ClientId& client_id, bool user_reqeusted);
 
-  // Remove a list of requests by |client_id|.  This removes requests from the
+  // Remove a list of requests by |request_id|.  This removes requests from the
   // request queue, but does not cancel an in-progress pre-render.
   // TODO(petewil): Add code to cancel an in-progress pre-render.
-  void RemoveRequests(const std::vector<ClientId>& client_ids);
+  void RemoveRequests(const std::vector<int64_t>& request_ids);
 
   // Callback that receives the response for GetQueuedRequests.  Client must
   // copy the result right away, it goes out of scope at the end of the
   // callback.
-  typedef base::Callback<void(const std::vector<ClientId>&)>
+  typedef base::Callback<void(const std::vector<SavePageRequest>&)>

[Dmitry Titov, 2016/08/10 02:52:14]

this is a strange type. A reference to a vector of actual instances. I think it
will take a copy on each SavePageRequest to construct this, and then the
receiver has to copy the whole thing again.

does std::move() work on SavePageRequests? In other words, can receiver do this:
void myCallback(const std::vector<SavePageRequest>& requests) {
  std::vector<SavePageRequest> mystorage;
  mystorage.push_back(std::move(requests[0]));
  ...
so that there is no additional copying?

[Pete Williamson, 2016/08/10 21:15:25]

We copy the SavePageRequest items into the vector, then pass a reference to that
up through several levels of callbacks, so typically the reference is getting
copied, not the vector.  My plan was for the consumer at the top level to then
make another copy.  I don't think that std::move is safe here because there
isn't a smart pointer underlying it, the vector is just on the stack.

If you like, I could change this to return a smart pointer to a const vector of
SavePageResults, that would allow a caller to use std::move

[Pete Williamson, 2016/08/12 00:00:40]

As we dicussed, we'll leave this behavior in for now.  crbug.com/637077 tracks
fixing the copying for both this and OfflinePageItem.

       QueuedRequestCallback;
 
-  // For a client namespace, get the ClientId of all requests for that
-  // namespace.
-  void GetQueuedRequests(const std::string& client_namespace,
-                         const QueuedRequestCallback& callback);
+  // Get all save page request items in the callback.
+  void GetQueuedRequests(const QueuedRequestCallback& callback);

[Dmitry Titov, 2016/08/10 02:52:14]

Can we rename it to GetAllRequests? It does return all, and there are no other
requests other than queued...

[Pete Williamson, 2016/08/10 21:15:25]

Done.

 
   // Starts processing of one or more queued save page later requests.
   // Returns whether processing was started and that caller should expect
   // a callback. If processing was already active, returns false.
   bool StartProcessing(const DeviceConditions& device_conditions,
                        const base::Callback<void(bool)>& callback);
 
   // Stops the current request processing if active. This is a way for
   // caller to abort processing; otherwise, processing will complete on
   // its own. In either case, the callback will be called when processing
@@ skipping 28 matching lines @@
     return is_canceled_;
   }
 
   OfflineEventLogger* GetLogger() {
     return &event_logger_;
   }
 
  private:
   // Receives the results of a get from the request queue, and turns that into
   // ClientId objects for the caller of GetQueuedRequests.
-  void GetQueuedRequestsCallback(const std::string& client_namespace,
-                                 const QueuedRequestCallback& callback,
+  void GetQueuedRequestsCallback(const QueuedRequestCallback& callback,
                                  RequestQueue::GetRequestsResult result,
                                  const std::vector<SavePageRequest>& requests);
 
   // Receives the result of add requests to the request queue.
   void AddRequestResultCallback(RequestQueue::AddRequestResult result,
                                 const SavePageRequest& request);
 
   // Receives the result of update and delete requests to the request queue.
   void UpdateRequestCallback(const ClientId& client_id,
                              RequestQueue::UpdateRequestResult result);
@@ skipping 64 matching lines @@
   base::TimeDelta offliner_timeout_;
   // Allows us to pass a weak pointer to callbacks.
   base::WeakPtrFactory<RequestCoordinator> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RequestCoordinator);
 };
 
 }  // namespace offline_pages
 
 #endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_REQUEST_COORDINATOR_H_