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..91771f0e28c9a55b1b19b0eb531b5d4dbcb2f4ac 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -228,7 +228,22 @@ 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:

[fgorski, 2016/08/03 03:40:04]

It is a bit worrying that you have two scope operators here. Perhaps the enum
does not belong inside offliner?

[dougarnett, 2016/08/03 15:33:26]

worrying? This is part of the Offliner interface so not sure of better place to
define it. Happy to hear more specific recommendation for reducing scope or just
using "using".

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

Acknowledged.

+    case Offliner::RequestStatus::SAVE_FAILED:
+    case Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED:  // timeout
+    case Offliner::RequestStatus::PRERENDERING_CANCELED:

[Pete Williamson, 2016/08/02 22:20:00]

Good!
Let's be a bit more explicit for the cases where we don't keep processing by
calling out the cases:

   case Offliner::RequestStatus::PRERENDER_FAILED:
   case Offliner::RequestStatus::FOREGOUND_CANCELLED:
...
  default:
      break;

[dougarnett, 2016/08/03 15:33:26]

Done.

+      TryNextRequest();
+      break;
+    default:
+      // Stop further processing in this service window.

[fgorski, 2016/08/03 03:40:05]

https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements

[dougarnett, 2016/08/03 15:33:26]

Done.

+      break;
+  }
 }
 
 const Scheduler::TriggerConditions