hook up background task's "startProcessing" from Java to request_coordinator

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 51 matching lines @@
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {
 
   // Inform the scheduler that we have an outstanding task.
   // TODO(petewil): Define proper TriggerConditions and set them.
   Scheduler::TriggerCondition conditions;
   scheduler_->Schedule(conditions);
 }
 
 void RequestCoordinator::RequestPicked(const SavePageRequest& request) {
+  Scheduler::TriggerCondition conditions;
+
+  // Inform the scheduler that we have an outstanding task.

[dougarnett, 2016/06/09 21:54:27]

thought this comment wording was confusing - the scheduler is dumb about why it
is scheduled. Isn't this is really about keeping the scheduler active in case we
crash before we know if we need to be rescheduled?

// While we have a pending request, be sure the Scheduler will trigger
// again in case we crash before we know if we are done with requests. 

[Pete Williamson, 2016/06/09 22:09:48]

No, this is really about ensuring that the current request is scheduled. If this
is the first request, and no others are active, the scheduler may not have
anything at all on the list.  This will make sure it has something for the
current request.  True, it may already have something scheduled, so I can change
the wording to "Ensure that..."  The place for making sure we are scheduled is
when a new task arrives from the scheduler, not when we take something off the
request queue (I haven't added that yet).

[dougarnett, 2016/06/09 22:27:05]

Hmm, not sure I understand your response. Might be easier to sync up on in
person sometime. 

It is not clear why we need something scheduled for the request we are
processing (since the scheduler just fired a StartProcessing to do it). We are
dispatching it to offliner below and it will possibly complete and not need to
be tried again. So I think this is all about the covering for the crash scenario
and, if so, then we should identify the purpose better. If not, then I am still
missing something here.

[Pete Williamson, 2016/06/09 22:30:17]

I was thinking that we might also get here via an API call to offline a page
instead of just via the scheduler, but now that I think some more, that will go
through the scheduler too, so this always comes from the scheduler.  In that
case, I will remove this (later), and move the backup reschedule into the
scheduler module.

+  scheduler_->Schedule(conditions);
+
   // Send the request on to the offliner.
   SendRequestToOffliner(request);
 }
 
 void RequestCoordinator::RequestQueueEmpty() {
   // TODO(petewil): return to the BackgroundScheduler by calling
   // ProcessingDoneCallback
 }
 
 bool RequestCoordinator::StartProcessing(
@@ skipping 37 matching lines @@
            << __FUNCTION__;
   last_offlining_status_ = status;
 
   // TODO(petewil): Check time budget.  Start a request if we have time, return
   // to the scheduler if we are out of time.
 
   // TODO(petewil): If the request succeeded, remove it from the Queue.
 }
 
 }  // namespace offline_pages