count timeout as a completed attempt.

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 <limits>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 193 matching lines @@
     const GetRequestsCallback& callback,
     RequestQueue::GetRequestsResult result,
     std::vector<std::unique_ptr<SavePageRequest>> requests) {
   callback.Run(std::move(requests));
 }
 
 void RequestCoordinator::StopPrerendering(Offliner::RequestStatus stop_status) {
   if (offliner_ && is_busy_) {
     DCHECK(active_request_.get());
     offliner_->Cancel();
+
+    // If we timed out, let the offliner done callback handle it.

[dougarnett, 2016/10/28 16:13:11]

This will dramatically diminish the success rate for svelte devices
on 2G (arguably, our primary target) since they will see a 3 minute watchdog not
an 8 minute one (and we have data showing many pages 
that succeeded took 3 or 4 two minutes attempts before they succeeded).

We could make this conditional on IMMEDIATE_WINDOW because we can
tune its timeout to be a full page budget => complete attempt. But
if a full page budget is 8 minutes (or 5 minutes), then we can't say
that a 3 minute attempt (SCHEDULED_WINDOW) was a complete attempt.
We need to account for giving it a fair time window in some way. I'm
not sure just allowing one retry is the right approach but it is 
another mitigation option until we decide the right approach.

We have to be careful about this scheduled window behavior if the
immediate behavior will hide seeing its behavior most of the time
in our ad hoc use. We might need a test harness mode that turns
off immediate mode (can be via svelte device for now probably) to
be able to evaluate its tuning separate from the immediate mode. 
 
I suggest making conditional on processing state for now with TODO
to address timeouts and retries for scheduled window (or across 
modes).

[Pete Williamson, 2016/10/28 18:32:38]

Done.

+    if (stop_status == Offliner::RequestStatus::REQUEST_COORDINATOR_TIMED_OUT)
+      return;
+
+    // Otherwise, this attempt never really had a chance to run, mark it
+    // aborted.
     AbortRequestAttempt(active_request_.get());
   }
 
   // Stopping offliner means it will not call callback so set last status.
   last_offlining_status_ = stop_status;
 
   if (active_request_) {
     event_logger_.RecordOfflinerResult(active_request_->client_id().name_space,
                                        last_offlining_status_,
                                        active_request_->request_id());
@@ skipping 585 matching lines @@
 
 ClientPolicyController* RequestCoordinator::GetPolicyController() {
   return policy_controller_.get();
 }
 
 void RequestCoordinator::Shutdown() {
   network_quality_estimator_ = nullptr;
 }
 
 }  // namespace offline_pages