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

components/offline_pages/background/request_coordinator.cc

Index: components/offline_pages/background/request_coordinator.cc
diff --git a/components/offline_pages/background/request_coordinator.cc b/components/offline_pages/background/request_coordinator.cc
index 5876d8386d79b7c3834f6cf9b7d72bd22f8c38cd..68551e273d06c55ef526f9d609b50e744f240a3d 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -228,7 +228,28 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                    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