Skips processing more SavePageLater requests in current processing window when one completes unsucc…

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 210 matching lines @@
     updated_request.set_last_attempt_time(base::Time::Now());
     RequestQueue::UpdateRequestCallback update_callback =
         base::Bind(&RequestCoordinator::UpdateRequestCallback,
                    weak_ptr_factory_.GetWeakPtr());
     queue_->UpdateRequest(
         updated_request,
         base::Bind(&RequestCoordinator::UpdateRequestCallback,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-  TryNextRequest();
+  // Determine whether we might try another request in this
+  // processing window based on how the previous request completed.
+  //
+  // TODO(dougarnett): Need to split PRERENDERING_FAILED into separate
+  // codes as to whether we should try another request or not.
+  switch (status) {
+    case Offliner::RequestStatus::SAVED:
+    case Offliner::RequestStatus::SAVE_FAILED:
+    case Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED:  // timeout
+    case Offliner::RequestStatus::PRERENDERING_CANCELED:
+      // Consider processing another request in this service window.
+      TryNextRequest();
+      break;
+    case Offliner::RequestStatus::FOREGROUND_CANCELED:

[fgorski, 2016/08/03 16:18:59]

I think you might be missing UNKNOWN and LOADED.

[dougarnett, 2016/08/03 16:44:08]

Right, actually intentional here as those are interim codes that should not be
received by callback. I'm a bit torn about including them explicitly and
asserting vs. just let default do it and not bothering the Coordinator
maintainer with these interim codes. Also, I have a TODO supported by pasko to
merge the PrerendingOffliner and PrerenderingLoader which would allow me to stop
using LOADED. UNKNOWN is still needed for initialization and testing.

+    case Offliner::RequestStatus::PRERENDERING_FAILED:
+      // No further processing in this service window.
+      break;
+    default:
+      // Make explicit choice about new status codes that actually reach here.
+      // Their default is no further processing in this service window.
+      DCHECK(false);
+  }
 }
 
 const Scheduler::TriggerConditions
 RequestCoordinator::GetTriggerConditionsForUserRequest() {
   Scheduler::TriggerConditions trigger_conditions(
       policy_->PowerRequiredForUserRequestedPage(),
       policy_->BatteryPercentageRequiredForUserRequestedPage(),
       policy_->UnmeteredNetworkRequiredForUserRequestedPage());
   return trigger_conditions;
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages