[OfflinePages] Resuming a SavePageRequest now may start processing if network is connected.

BUG=641411

Review-Url: https://codereview.chromium.org/2282973003
Cr-Commit-Position: refs/heads/master@{#414887}

components/offline_pages/background/request_coordinator.cc

@@ -207,21 +207,8 @@ void RequestCoordinator::AddRequestResultCallback(
   // Inform the scheduler that we have an outstanding task..
   scheduler_->Schedule(GetTriggerConditionsForUserRequest());
 
-  // If it makes sense, start processing now.
-  if (is_busy_) return;
-
-  // Check for network
-  net::NetworkChangeNotifier::ConnectionType connection = GetConnectionType();
-
-  if ((connection !=
-       net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE) &&
-      request.user_requested()) {
-    // Create device conditions.
-    DeviceConditions device_conditions(false, 0, connection);
-
-    // Start processing if it makes sense. (net, user requested)
-    StartProcessing(device_conditions, base::Bind(&EmptySchedulerCallback));
-  }
+  if (request.user_requested())
+    StartProcessingIfConnected();
 }
 
 // Called in response to updating a request in the request queue.
@@ -241,8 +228,27 @@ void RequestCoordinator::UpdateRequestCallback(
 void RequestCoordinator::UpdateMultipleRequestsCallback(
     const RequestQueue::UpdateMultipleRequestResults& results,
     const std::vector<SavePageRequest>& requests) {
-  for (SavePageRequest request : requests)
+  bool available_user_request = false;
+  for (SavePageRequest request : requests) {
     NotifyChanged(request);
+    if (!available_user_request && request.user_requested() &&
+        request.request_state() == SavePageRequest::RequestState::AVAILABLE) {
+      // TODO(dougarnett): Consider avoiding prospect of N^2 in case
+      // size of bulk requests can get large (perhaps with easier to consume
+      // callback interface).
+      for (std::pair<int64_t, RequestQueue::UpdateRequestResult> pair :
+           results) {
+        if (pair.first == request.request_id() &&
+            pair.second == RequestQueue::UpdateRequestResult::SUCCESS) {
+          // We have a successfully updated, available, user request.
+          available_user_request = true;
+        }
+      }
+    }
+  }
+
+  if (available_user_request)
+    StartProcessingIfConnected();
 }
 
 void RequestCoordinator::HandleRemovedRequestsAndCallback(
@@ -290,6 +296,22 @@ bool RequestCoordinator::StartProcessing(
   return true;
 }
 
+void RequestCoordinator::StartProcessingIfConnected() {
+  // Makes sure not already busy processing.
+  if (is_busy_) return;
+
+  // Check for network connectivity.
+  net::NetworkChangeNotifier::ConnectionType connection = GetConnectionType();
+
+  if ((connection !=
+       net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE)) {
+    // Create conservative device conditions for the connectivity
+    // (assume no battery).
+    DeviceConditions device_conditions(false, 0, connection);
+    StartProcessing(device_conditions, base::Bind(&EmptySchedulerCallback));
+  }
+}
+
 void RequestCoordinator::TryNextRequest() {
   // If there is no time left in the budget, return to the scheduler.
   // We do not remove the pending task that was set up earlier in case

components/offline_pages/background/request_coordinator.h

@@ -186,6 +186,10 @@ class RequestCoordinator : public KeyedService,
       const RequestQueue::UpdateMultipleRequestResults& results,
       const std::vector<SavePageRequest>& requests);
 
+  // Start processing now if connected (but with conservative assumption
+  // as to other device conditions).
+  void StartProcessingIfConnected();
+
   // Callback from the request picker when it has chosen our next request.
   void RequestPicked(const SavePageRequest& request);
 

components/offline_pages/background/request_coordinator_unittest.cc

@@ -851,4 +851,43 @@ TEST_F(RequestCoordinatorTest, SavePageDoesntStartProcessingWhenDisconnected) {
   EXPECT_FALSE(is_busy());
 }
 
+TEST_F(RequestCoordinatorTest, ResumeStartsProcessingWhenConnected) {
+  SetNetworkConditionsForTest(
+      net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE);
+
+  // Add a request to the queue.
+  offline_pages::SavePageRequest request1(kRequestId1, kUrl1, kClientId1,
+                                          base::Time::Now(), kUserRequested);
+  coordinator()->queue()->AddRequest(
+      request1, base::Bind(&RequestCoordinatorTest::AddRequestDone,
+                           base::Unretained(this)));
+  PumpLoop();
+  EXPECT_FALSE(is_busy());
+
+  // Pause the request.
+  std::vector<int64_t> request_ids;
+  request_ids.push_back(kRequestId1);
+  coordinator()->PauseRequests(request_ids);
+  PumpLoop();
+
+  // Resume the request while disconnected.
+  coordinator()->ResumeRequests(request_ids);
+  PumpLoop();
+  EXPECT_FALSE(is_busy());
+
+  // Pause the request again.
+  coordinator()->PauseRequests(request_ids);
+  PumpLoop();
+
+  // Now simulate being connected.
+  SetNetworkConditionsForTest(
+      net::NetworkChangeNotifier::ConnectionType::CONNECTION_3G);
+
+  // Resume the request while connected.
+  coordinator()->ResumeRequests(request_ids);
+  EXPECT_FALSE(is_busy());
+  PumpLoop();
+  EXPECT_TRUE(is_busy());
+}
+
 }  // namespace offline_pages