More detailed implementation of the RequestPicker

components/offline_pages/background/request_coordinator.cc

 // 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.
 
 #include "components/offline_pages/background/request_coordinator.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 42 matching lines @@
       is_canceled_(false),
       offliner_timeout_(base::TimeDelta::FromSeconds(OFFLINER_TIMEOUT_SECONDS)),
       offliner_(nullptr),
       policy_(std::move(policy)),
       factory_(std::move(factory)),
       queue_(std::move(queue)),
       scheduler_(std::move(scheduler)),
       last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
       weak_ptr_factory_(this) {
   DCHECK(policy_ != nullptr);
-  picker_.reset(new RequestPicker(queue_.get()));
+  picker_.reset(new RequestPicker(queue_.get(), policy.get()));

[fgorski, 2016/07/20 16:23:31]

policy_.get()
This is happening after the std::move above, I suspect you are assigning
nullptr.

[Pete Williamson, 2016/07/20 19:50:43]

Nice catch!  This was supposed to be policy_

 }
 
 RequestCoordinator::~RequestCoordinator() {}
 
 bool RequestCoordinator::SavePageLater(
-    const GURL& url, const ClientId& client_id) {
+    const GURL& url, const ClientId& client_id, bool was_user_requested) {
   DVLOG(2) << "URL is " << url << " " << __FUNCTION__;
 
   // TODO(petewil): We need a robust scheme for allocating new IDs.

[fgorski, 2016/07/20 16:23:31]

Note: We may have to switch for GUIDs for this give downloads home design. You
can capture this in a todo.

[Pete Williamson, 2016/07/20 19:50:43]

Todo elaborated a bit more to mention GUIDS and downloads home.

   static int64_t id = 0;
 
   // Build a SavePageRequest.
   // TODO(petewil): Use something like base::Clock to help in testing.
   offline_pages::SavePageRequest request(
-      id++, url, client_id, base::Time::Now());
+      id++, url, client_id, base::Time::Now(), was_user_requested);
 
   // Put the request on the request queue.
   queue_->AddRequest(request,
                      base::Bind(&RequestCoordinator::AddRequestResultCallback,
                                 weak_ptr_factory_.GetWeakPtr()));
   return true;
 }
 
 void RequestCoordinator::AddRequestResultCallback(
     RequestQueue::AddRequestResult result,
@@ skipping 20 matching lines @@
 
   // Let the scheduler know we are done processing.
   scheduler_callback_.Run(true);
 }
 
 // Returns true if the caller should expect a callback, false otherwise. For
 // instance, this would return false if a request is already in progress.
 bool RequestCoordinator::StartProcessing(
     const DeviceConditions& device_conditions,
     const base::Callback<void(bool)>& callback) {
+  current_conditions_.reset(new DeviceConditions(device_conditions));
   if (is_busy_) return false;
 
   is_canceled_ = false;
   scheduler_callback_ = callback;
   // TODO(petewil): Check existing conditions (should be passed down from
   // BackgroundTask)
 
   TryNextRequest();
 
   return true;
 }
 
 void RequestCoordinator::TryNextRequest() {
   // Choose a request to process that meets the available conditions.
   // This is an async call, and returns right away.
   picker_->ChooseNextRequest(
       base::Bind(&RequestCoordinator::RequestPicked,
                  weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&RequestCoordinator::RequestQueueEmpty,
-                 weak_ptr_factory_.GetWeakPtr()));
+                 weak_ptr_factory_.GetWeakPtr()),
+      current_conditions_.get());
 }
 
 // Called by the request picker when a request has been picked.
 void RequestCoordinator::RequestPicked(const SavePageRequest& request) {
   // Send the request on to the offliner.
   SendRequestToOffliner(request);
 }
 
 void RequestCoordinator::RequestQueueEmpty() {
   // Clear the outstanding "safety" task in the scheduler.
@@ skipping 64 matching lines @@
   return kUserRequestTriggerConditions;
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages