[OfflinePages] Classifies PRERENDERING_FAILED cases whether to TryNext

components/offline_pages/background/request_coordinator_unittest.cc

Index: components/offline_pages/background/request_coordinator_unittest.cc
diff --git a/components/offline_pages/background/request_coordinator_unittest.cc b/components/offline_pages/background/request_coordinator_unittest.cc
index c91909c80abcc34c0b14921c5293a677312ebc37..9dce21b072e66e132e531efe0e8096321cb09ff8 100644
--- a/components/offline_pages/background/request_coordinator_unittest.cc
+++ b/components/offline_pages/background/request_coordinator_unittest.cc
@@ -589,13 +589,12 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {
                            Offliner::RequestStatus::PRERENDERING_FAILED);
   PumpLoop();
 
-  // For retriable failure, processing should stop and scheduler callback
-  // called (so that request can be retried first next processing window).
-  EXPECT_TRUE(immediate_schedule_callback_called());
+  // For retriable failure, processing should continue to 2nd request so
+  // no scheduler callback yet.
+  EXPECT_FALSE(immediate_schedule_callback_called());
 
-  // TODO(dougarnett): Consider injecting mock RequestPicker for this test
-  // and verifying that there is no attempt to pick another request following
-  // this failure code.
+  // Busy processing 2nd request.
+  EXPECT_TRUE(is_busy());
 
   coordinator()->queue()->GetRequests(
       base::Bind(&RequestCoordinatorTest::GetRequestsDone,
@@ -606,7 +605,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailed) {
   // (max number of attempts exceeded).
   EXPECT_EQ(1UL, last_requests().size());
   // Check that the observer got the notification that we failed (and the
-  // subsequent notification that the request was removed).
+  // subsequent notification that the request was removed) since we exceeded
+  // retry count.
   EXPECT_TRUE(observer().completed_called());
   EXPECT_EQ(RequestCoordinator::BackgroundSavePageResult::RETRY_COUNT_EXCEEDED,
             observer().last_status());
@@ -633,9 +633,8 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {
   // no scheduler callback yet.
   EXPECT_FALSE(immediate_schedule_callback_called());
 
-  // TODO(dougarnett): Consider injecting mock RequestPicker for this test
-  // and verifying that there is as attempt to pick another request following
-  // this non-retryable failure code.
+  // Busy processing 2nd request.
+  EXPECT_TRUE(is_busy());
 
   coordinator()->queue()->GetRequests(base::Bind(
       &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
@@ -650,6 +649,38 @@ TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoRetryFailure) {
             observer().last_status());
 }
 
+TEST_F(RequestCoordinatorTest, OfflinerDoneRequestFailedNoNextFailure) {
+  // Add a request to the queue, wait for callbacks to finish.
+  offline_pages::SavePageRequest request(kRequestId1, kUrl1, kClientId1,
+                                         base::Time::Now(), kUserRequested);
+  SetupForOfflinerDoneCallbackTest(&request);
+  EnableOfflinerCallback(false);
+
+  // Add second request to the queue to check handling when first fails.
+  AddRequest2();
+  PumpLoop();
+
+  // Call the OfflinerDoneCallback to simulate the request failed, wait
+  // for callbacks.
+  SendOfflinerDoneCallback(
+      request, Offliner::RequestStatus::PRERENDERING_FAILED_NO_NEXT);
+  PumpLoop();
+
+  // For no next failure, processing should not continue to 2nd request so
+  // expect scheduler callback.
+  EXPECT_TRUE(immediate_schedule_callback_called());
+
+  // Not busy for NO_NEXT failure.
+  EXPECT_FALSE(is_busy());
+
+  coordinator()->queue()->GetRequests(base::Bind(
+      &RequestCoordinatorTest::GetRequestsDone, base::Unretained(this)));
+  PumpLoop();
+
+  // Both requests still in queue.
+  EXPECT_EQ(2UL, last_requests().size());
+}
+
 TEST_F(RequestCoordinatorTest, OfflinerDoneForegroundCancel) {
   // Add a request to the queue, wait for callbacks to finish.
   offline_pages::SavePageRequest request(