[Offline Pages] Close race condition of multiple StartProcessing callers.

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 ffd1768e056cb126da16755b518dbb9ae50a69f8..1e5240ff9135b56522a46c33d9c3cc89e5d9e9b5 100644
--- a/components/offline_pages/background/request_coordinator.cc
+++ b/components/offline_pages/background/request_coordinator.cc
@@ -79,6 +79,7 @@ RequestCoordinator::RequestCoordinator(std::unique_ptr<OfflinerPolicy> policy,
                                        std::unique_ptr<RequestQueue> queue,
                                        std::unique_ptr<Scheduler> scheduler)
     : is_busy_(false),
+      is_starting_(false),
       is_stopped_(false),
       use_test_connection_type_(false),
       test_connection_type_(),
@@ -345,7 +346,9 @@ bool RequestCoordinator::StartProcessing(
     const DeviceConditions& device_conditions,
     const base::Callback<void(bool)>& callback) {
   current_conditions_.reset(new DeviceConditions(device_conditions));
-  if (is_busy_) return false;
+  if (is_starting_ || is_busy_)

[Pete Williamson, 2016/09/06 22:56:44]

Please add a bug to refactor to a single state variable.

[dougarnett, 2016/09/06 23:39:10]

Done. https://bugs.chromium.org/p/chromium/issues/detail?id=644519

+    return false;

[Pete Williamson, 2016/09/06 22:56:44]

(as we discussed in person) BackgroundOfflinerTask.java is not checking the
return value, we should do that.

[dougarnett, 2016/09/06 23:39:10]

Done.

+  is_starting_ = true;
 
   // Mark the time at which we started processing so we can check our time
   // budget.
@@ -400,12 +403,19 @@ void RequestCoordinator::TryNextRequest() {
 
 // Called by the request picker when a request has been picked.
 void RequestCoordinator::RequestPicked(const SavePageRequest& request) {
-  // Send the request on to the offliner.
-  SendRequestToOffliner(request);
+  is_starting_ = false;
+
+  // Make sure we were not stopped while picking.
+  if (!is_stopped_) {
+    // Send the request on to the offliner.
+    SendRequestToOffliner(request);
+  }
 }
 
 void RequestCoordinator::RequestNotPicked(
     bool non_user_requested_tasks_remaining) {
+  is_starting_ = false;
+
   // Clear the outstanding "safety" task in the scheduler.
   scheduler_->Unschedule();