[Offline Pages] Define separate watchdog timeout for concurrent bg loads

components/offline_pages/background/request_coordinator.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/background/request_coordinator.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 103 matching lines @@
 
 RequestCoordinator::RequestCoordinator(
     std::unique_ptr<OfflinerPolicy> policy,
     std::unique_ptr<OfflinerFactory> factory,
     std::unique_ptr<RequestQueue> queue,
     std::unique_ptr<Scheduler> scheduler,
     net::NetworkQualityEstimator::NetworkQualityProvider*
         network_quality_estimator)
     : is_busy_(false),
       is_starting_(false),
-      is_stopped_(false),
+      processing_state_(ProcessingWindowState::STOPPED),
       use_test_connection_type_(false),
       test_connection_type_(),
       offliner_(nullptr),
       policy_(std::move(policy)),
       factory_(std::move(factory)),
       queue_(std::move(queue)),
       scheduler_(std::move(scheduler)),
       policy_controller_(new ClientPolicyController()),
       network_quality_estimator_(network_quality_estimator),
       active_request_(nullptr),
       last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
-      offliner_timeout_(base::TimeDelta::FromSeconds(
-          policy_->GetSinglePageTimeLimitInSeconds())),
       weak_ptr_factory_(this) {
   DCHECK(policy_ != nullptr);
   picker_.reset(
       new RequestPicker(queue_.get(), policy_.get(), this, &event_logger_));
 }
 
 RequestCoordinator::~RequestCoordinator() {}
 
 int64_t RequestCoordinator::SavePageLater(const GURL& url,
                                           const ClientId& client_id,
@@ skipping 169 matching lines @@
 }
 
 void RequestCoordinator::AddRequestResultCallback(
     RequestQueue::AddRequestResult result,
     const SavePageRequest& request) {
   NotifyAdded(request);
   // Inform the scheduler that we have an outstanding task.
   scheduler_->Schedule(GetTriggerConditions(kUserRequest));
 
   if (request.user_requested())
-    StartProcessingIfConnected();
+    StartImmediatelyIfConnected();
 }
 
 // Called in response to updating a request in the request queue.
 void RequestCoordinator::UpdateRequestCallback(
     const ClientId& client_id,
     RequestQueue::UpdateRequestResult result) {
   // If the request succeeded, nothing to do.  If it failed, we can't really do
   // much, so just log it.
   if (result != RequestQueue::UpdateRequestResult::SUCCESS) {
     DVLOG(1) << "Failed to update request attempt details. "
              << static_cast<int>(result);
     event_logger_.RecordUpdateRequestFailed(client_id.name_space, result);
   }
 }
 
 void RequestCoordinator::UpdateMultipleRequestsCallback(
     std::unique_ptr<UpdateRequestsResult> result) {
   for (const auto& request : result->updated_items)
     NotifyChanged(request);
 
   bool available_user_request = false;
   for (const auto& request : result->updated_items) {
     if (!available_user_request && request.user_requested() &&
         request.request_state() == SavePageRequest::RequestState::AVAILABLE) {
       available_user_request = true;
     }
   }
 
   if (available_user_request)
-    StartProcessingIfConnected();
+    StartImmediatelyIfConnected();
 }
 
 // When we successfully remove a request that completed successfully, move on to
 // the next request.
 void RequestCoordinator::CompletedRequestCallback(
     const MultipleItemStatuses& status) {
   TryNextRequest();
 }
 
 void RequestCoordinator::HandleRemovedRequestsAndCallback(
@@ skipping 17 matching lines @@
 
 void RequestCoordinator::ScheduleAsNeeded() {
   // Get all requests from queue (there is no filtering mechanism).
   queue_->GetRequests(
       base::Bind(&RequestCoordinator::GetRequestsForSchedulingCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void RequestCoordinator::StopProcessing(
     Offliner::RequestStatus stop_status) {
-  is_stopped_ = true;
+  processing_state_ = ProcessingWindowState::STOPPED;
   StopPrerendering(stop_status);
 
   // Let the scheduler know we are done processing.
   scheduler_callback_.Run(true);
 }
 
 void RequestCoordinator::HandleWatchdogTimeout() {
   StopProcessing(Offliner::REQUEST_COORDINATOR_TIMED_OUT);
 }
 
 // Returns true if the caller should expect a callback, false otherwise. For
 // instance, this would return false if a request is already in progress.
 bool RequestCoordinator::StartProcessing(
     const DeviceConditions& device_conditions,
     const base::Callback<void(bool)>& callback) {
+  return StartProcessingInternal(ProcessingWindowState::SCHEDULED_WINDOW,
+                                 device_conditions, callback);
+}
+
+bool RequestCoordinator::StartProcessingInternal(
+    const ProcessingWindowState processing_state,
+    const DeviceConditions& device_conditions,
+    const base::Callback<void(bool)>& callback) {
   current_conditions_.reset(new DeviceConditions(device_conditions));
   if (is_starting_ || is_busy_)
     return false;
   is_starting_ = true;
+  processing_state_ = processing_state;
+  scheduler_callback_ = callback;
 
   // Mark the time at which we started processing so we can check our time
   // budget.
   operation_start_time_ = base::Time::Now();
 
-  is_stopped_ = false;
-  scheduler_callback_ = callback;
-
   TryNextRequest();
 
   return true;
 }
 
-void RequestCoordinator::StartProcessingIfConnected() {
+void RequestCoordinator::StartImmediatelyIfConnected() {
   OfflinerImmediateStartStatus immediate_start_status = TryImmediateStart();
   UMA_HISTOGRAM_ENUMERATION(
       "OfflinePages.Background.ImmediateStartStatus", immediate_start_status,
       RequestCoordinator::OfflinerImmediateStartStatus::STATUS_COUNT);
 }
 
 RequestCoordinator::OfflinerImmediateStartStatus
 RequestCoordinator::TryImmediateStart() {
   // Make sure not already busy processing.
   if (is_busy_)
@@ skipping 12 matching lines @@
       return OfflinerImmediateStartStatus::WEAK_CONNECTION;
   } else if (GetConnectionType() ==
              net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE) {
     return OfflinerImmediateStartStatus::NO_CONNECTION;
   }
 
   // Start processing with manufactured conservative battery conditions
   // (i.e., assume no battery).
   // TODO(dougarnett): Obtain actual battery conditions (from Android/Java).
   DeviceConditions device_conditions(false, 0, GetConnectionType());
-  if (StartProcessing(device_conditions, base::Bind(&EmptySchedulerCallback)))
+  if (StartProcessingInternal(ProcessingWindowState::IMMEDIATE_WINDOW,
+                              device_conditions,
+                              base::Bind(&EmptySchedulerCallback)))
     return OfflinerImmediateStartStatus::STARTED;
   else
     return OfflinerImmediateStartStatus::NOT_ACCEPTED;
 }
 
 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
   // we run out of time, so the background scheduler will return to us
   // at the next opportunity to run background tasks.
@@ skipping 16 matching lines @@
                                         weak_ptr_factory_.GetWeakPtr()),
                              current_conditions_.get(),
                              disabled_requests_);
 }
 
 // Called by the request picker when a request has been picked.
 void RequestCoordinator::RequestPicked(const SavePageRequest& request) {
   is_starting_ = false;
 
   // Make sure we were not stopped while picking.
-  if (!is_stopped_) {
+  if (processing_state_ != ProcessingWindowState::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();
 
   // If disabled tasks remain, post a new safety task for 5 sec from now.
   if (disabled_requests_.size() > 0) {
     scheduler_->BackupSchedule(GetTriggerConditions(kUserRequest),
                                kDisabledTaskRecheckSeconds);
   } else if (non_user_requested_tasks_remaining) {
     // If we don't have any of those, check for non-user-requested tasks.
     scheduler_->Schedule(GetTriggerConditions(!kUserRequest));
   }
 
   // Let the scheduler know we are done processing.
   scheduler_callback_.Run(true);
 }
 
 void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) {
   // Check that offlining didn't get cancelled while performing some async
   // steps.
-  if (is_stopped_)
+  if (processing_state_ == ProcessingWindowState::STOPPED)
     return;
 
   GetOffliner();
   if (!offliner_) {
     DVLOG(0) << "Unable to create Offliner. "
              << "Cannot background offline page.";
     return;
   }
 
   DCHECK(!is_busy_);
   is_busy_ = true;
 
   // Update the request for this attempt to start it.
   SavePageRequest updated_request(request);
   updated_request.MarkAttemptStarted(base::Time::Now());
   queue_->UpdateRequest(
       updated_request,
       base::Bind(&RequestCoordinator::UpdateRequestCallback,
                  weak_ptr_factory_.GetWeakPtr(), updated_request.client_id()));
   active_request_.reset(new SavePageRequest(updated_request));
 
   // Start the load and save process in the offliner (Async).
   if (offliner_->LoadAndSave(
           updated_request, base::Bind(&RequestCoordinator::OfflinerDoneCallback,
                                       weak_ptr_factory_.GetWeakPtr()))) {
     // Start a watchdog timer to catch pre-renders running too long
-    watchdog_timer_.Start(FROM_HERE, offliner_timeout_, this,
+    //  offliner_timeout_ = base::TimeDelta::FromSeconds(
+    //      policy_->GetSinglePageTimeLimitInSeconds(is_background_scheduled));

[Pete Williamson, 2016/10/13 22:58:17]

Remove commented out code before checkin

[dougarnett, 2016/10/14 16:27:01]

Done.

+    base::TimeDelta timeout;
+    if (processing_state_ == ProcessingWindowState::SCHEDULED_WINDOW) {
+      timeout = base::TimeDelta::FromSeconds(
+          policy_->GetSinglePageTimeLimitWhenBackgroundScheduledInSeconds());
+    } else {
+      DCHECK(processing_state_ == ProcessingWindowState::IMMEDIATE_WINDOW);
+      timeout = base::TimeDelta::FromSeconds(
+          policy_->GetSinglePageTimeLimitForImmediateLoadInSeconds());
+    }
+    watchdog_timer_.Start(FROM_HERE, timeout, this,

[Pete Williamson, 2016/10/13 22:58:18]

Nice, much less fragile, I like it.

[dougarnett, 2016/10/14 16:27:01]

Acknowledged.

                           &RequestCoordinator::HandleWatchdogTimeout);
   } else {
     is_busy_ = false;
     DVLOG(0) << "Unable to start LoadAndSave";
     StopProcessing(Offliner::PRERENDERING_NOT_STARTED);
   }
 }
 
 void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                                               Offliner::RequestStatus status) {
@@ skipping 72 matching lines @@
   }
 }
 
 void RequestCoordinator::EnableForOffliner(int64_t request_id) {
   // Since the recent tab helper might call multiple times, ignore subsequent
   // calls for a particular request_id.
   if (disabled_requests_.find(request_id) == disabled_requests_.end())
     return;
   disabled_requests_.erase(request_id);
   // If we are not busy, start processing right away.
-  StartProcessingIfConnected();
+  StartImmediatelyIfConnected();
 }
 
 void RequestCoordinator::MarkRequestCompleted(int64_t request_id) {
   // Since the recent tab helper might call multiple times, ignore subsequent
   // calls for a particular request_id.
   if (disabled_requests_.find(request_id) == disabled_requests_.end())
     return;
   disabled_requests_.erase(request_id);
 
   // Remove the request, but send out SUCCEEDED instead of removed.
@@ skipping 45 matching lines @@
 
 ClientPolicyController* RequestCoordinator::GetPolicyController() {
   return policy_controller_.get();
 }
 
 void RequestCoordinator::Shutdown() {
   network_quality_estimator_ = nullptr;
 }
 
 }  // namespace offline_pages