[Offline Pages] Define separate watchdog timeout for concurrent bg loads

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 115 matching lines @@
       test_connection_type_(),
       offliner_(nullptr),
       policy_(std::move(policy)),
       factory_(std::move(factory)),
       queue_(std::move(queue)),
       scheduler_(std::move(scheduler)),
       policy_controller_(new ClientPolicyController()),
       network_quality_estimator_(network_quality_estimator),
       active_request_(nullptr),
       last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
-      offliner_timeout_(base::TimeDelta::FromSeconds(
-          policy_->GetSinglePageTimeLimitInSeconds())),
       weak_ptr_factory_(this) {
   DCHECK(policy_ != nullptr);
   picker_.reset(
       new RequestPicker(queue_.get(), policy_.get(), this, &event_logger_));
 }
 
 RequestCoordinator::~RequestCoordinator() {}
 
 int64_t RequestCoordinator::SavePageLater(const GURL& url,
                                           const ClientId& client_id,
@@ skipping 251 matching lines @@
   scheduler_callback_.Run(true);
 }
 
 void RequestCoordinator::HandleWatchdogTimeout() {
   StopProcessing(Offliner::REQUEST_COORDINATOR_TIMED_OUT);
 }
 
 // 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(
+    bool is_background_scheduled,
     const DeviceConditions& device_conditions,
     const base::Callback<void(bool)>& callback) {
   current_conditions_.reset(new DeviceConditions(device_conditions));
   if (is_starting_ || is_busy_)
     return false;
   is_starting_ = true;
 
   // Mark the time at which we started processing so we can check our time
   // budget.
   operation_start_time_ = base::Time::Now();
+  offliner_timeout_ = base::TimeDelta::FromSeconds(

[Pete Williamson, 2016/10/12 22:33:24]

It's a bit fragile to have this changing constantly. I wonder if there is a way
to tie it to the currently executing request.  (I can't think of any offhand,
but if you can, it would be great).

[dougarnett, 2016/10/13 19:46:45]

It is really tied to the service window though - not a single request - unless
we change somehow to make immediate triggering only apply to one request (which
might be an interesting propsect wrt user control over ordering). 

Another option would be to replace this timeout member with a processing state 
and then call the policy object when setting the watchdog. Trying that.

+      policy_->GetSinglePageTimeLimitInSeconds(is_background_scheduled));
 
   is_stopped_ = false;
   scheduler_callback_ = callback;
 
   TryNextRequest();
 
   return true;
 }
 
 void RequestCoordinator::StartProcessingIfConnected() {
@@ skipping 22 matching lines @@
       return OfflinerImmediateStartStatus::WEAK_CONNECTION;
   } else if (GetConnectionType() ==
              net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE) {
     return OfflinerImmediateStartStatus::NO_CONNECTION;
   }
 
   // Start processing with manufactured conservative battery conditions
   // (i.e., assume no battery).
   // TODO(dougarnett): Obtain actual battery conditions (from Android/Java).
   DeviceConditions device_conditions(false, 0, GetConnectionType());
-  if (StartProcessing(device_conditions, base::Bind(&EmptySchedulerCallback)))
+  if (StartProcessing(false /* is_background_scheduled */, device_conditions,
+                      base::Bind(&EmptySchedulerCallback)))
     return OfflinerImmediateStartStatus::STARTED;
   else
     return OfflinerImmediateStartStatus::NOT_ACCEPTED;
 }
 
 void RequestCoordinator::TryNextRequest() {
   // If there is no time left in the budget, return to the scheduler.
   // We do not remove the pending task that was set up earlier in case
   // we run out of time, so the background scheduler will return to us
   // at the next opportunity to run background tasks.
@@ skipping 221 matching lines @@
 
 ClientPolicyController* RequestCoordinator::GetPolicyController() {
   return policy_controller_.get();
 }
 
 void RequestCoordinator::Shutdown() {
   network_quality_estimator_ = nullptr;
 }
 
 }  // namespace offline_pages