Index: components/offline_pages/core/background/request_coordinator.cc |
diff --git a/components/offline_pages/core/background/request_coordinator.cc b/components/offline_pages/core/background/request_coordinator.cc |
index 8060d6af4ab0678672c381b76ee821aabf821870..ebc3db1ce147b9db4e28c1f823c0fb47e86c6d0b 100644 |
--- a/components/offline_pages/core/background/request_coordinator.cc |
+++ b/components/offline_pages/core/background/request_coordinator.cc |
@@ -197,8 +197,7 @@ RequestCoordinator::RequestCoordinator( |
net::NetworkQualityEstimator::NetworkQualityProvider* |
network_quality_estimator) |
: is_low_end_device_(base::SysInfo::IsLowEndDevice()), |
- is_busy_(false), |
- is_starting_(false), |
+ state_(RequestCoordinatorState::IDLE), |
processing_state_(ProcessingWindowState::STOPPED), |
use_test_device_conditions_(false), |
offliner_(std::move(offliner)), |
@@ -287,7 +286,7 @@ void RequestCoordinator::GetQueuedRequestsCallback( |
void RequestCoordinator::StopPrerendering( |
const Offliner::CancelCallback& final_callback, |
Offliner::RequestStatus stop_status) { |
- if (offliner_ && is_busy_) { |
+ if (offliner_ && state_ == RequestCoordinatorState::OFFLINING) { |
Pete Williamson
2017/03/28 00:35:13
What should we do if the state is STARTING? We pr
romax
2017/03/28 01:09:56
I think this is guarded by ProcessingWindowState::
Pete Williamson
2017/03/28 17:12:43
My concern is that we might have a call to StopPre
romax
2017/03/29 03:02:09
Done.
|
DCHECK(active_request_.get()); |
offliner_->Cancel(base::Bind( |
&RequestCoordinator::HandleCancelUpdateStatusCallback, |
@@ -555,7 +554,7 @@ void RequestCoordinator::UpdateStatusForCancel( |
RecordOfflinerResultUMA(active_request_->client_id(), |
active_request_->creation_time(), |
last_offlining_status_); |
- is_busy_ = false; |
+ state_ = RequestCoordinatorState::IDLE; |
active_request_.reset(); |
} |
} |
@@ -624,7 +623,7 @@ bool RequestCoordinator::StartImmediateProcessing( |
bool RequestCoordinator::StartProcessingInternal( |
const ProcessingWindowState processing_state, |
const base::Callback<void(bool)>& callback) { |
- if (is_starting_ || is_busy_) |
+ if (state_ != RequestCoordinatorState::IDLE) |
return false; |
processing_state_ = processing_state; |
scheduler_callback_ = callback; |
@@ -647,7 +646,7 @@ RequestCoordinator::TryImmediateStart( |
const base::Callback<void(bool)>& callback) { |
DVLOG(2) << "Immediate " << __func__; |
// Make sure not already busy processing. |
- if (is_busy_) |
+ if (state_ == RequestCoordinatorState::OFFLINING) |
return OfflinerImmediateStartStatus::BUSY; |
// Make sure we are not on svelte device to start immediately. |
@@ -702,7 +701,7 @@ void RequestCoordinator::UpdateCurrentConditionsFromAndroid() { |
} |
void RequestCoordinator::TryNextRequest(bool is_start_of_processing) { |
- is_starting_ = true; |
+ state_ = RequestCoordinatorState::STARTING; |
// If this is the first call, the device conditions are current, no need to |
// update them. |
@@ -737,7 +736,7 @@ void RequestCoordinator::TryNextRequest(bool is_start_of_processing) { |
if (connection_type == |
net::NetworkChangeNotifier::ConnectionType::CONNECTION_NONE || |
(base::Time::Now() - operation_start_time_) > processing_time_budget) { |
- is_starting_ = false; |
+ state_ = RequestCoordinatorState::IDLE; |
// If we were doing immediate processing, try to start it again |
// when we get connected. |
@@ -773,12 +772,15 @@ void RequestCoordinator::TryNextRequest(bool is_start_of_processing) { |
void RequestCoordinator::RequestPicked(const SavePageRequest& request, |
bool cleanup_needed) { |
DVLOG(2) << request.url() << " " << __func__; |
- is_starting_ = false; |
+ state_ = RequestCoordinatorState::OFFLINING; |
// Make sure we were not stopped while picking. |
if (processing_state_ != ProcessingWindowState::STOPPED) { |
// Send the request on to the offliner. |
- SendRequestToOffliner(request); |
+ if (!SendRequestToOffliner(request)) |
Pete Williamson
2017/03/28 00:35:13
Why check the return value, and then do the same t
romax
2017/03/28 01:09:56
Seems like you mismatched the braces? :)
I've pull
|
+ state_ = RequestCoordinatorState::IDLE; |
+ } else { |
+ state_ = RequestCoordinatorState::IDLE; |
} |
// Schedule a queue cleanup if needed. |
@@ -790,7 +792,7 @@ void RequestCoordinator::RequestNotPicked( |
bool non_user_requested_tasks_remaining, |
bool cleanup_needed) { |
DVLOG(2) << __func__; |
- is_starting_ = false; |
+ state_ = RequestCoordinatorState::IDLE; |
// Clear the outstanding "safety" task in the scheduler. |
scheduler_->Unschedule(); |
@@ -859,22 +861,21 @@ void RequestCoordinator::RequestCounts(bool is_start_of_processing, |
} |
} |
-void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) { |
+bool RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) { |
+ DCHECK(state_ == RequestCoordinatorState::OFFLINING); |
+ |
// Check that offlining didn't get cancelled while performing some async |
// steps. |
if (processing_state_ == ProcessingWindowState::STOPPED) |
- return; |
+ return false; |
if (!offliner_) { |
// TODO(chili,petewil): We should have UMA here to track frequency of this, |
// if it happens at all. |
DVLOG(0) << "Offliner crashed. Cannot background offline page."; |
- return; |
+ return false; |
} |
- DCHECK(!is_busy_); |
- is_busy_ = true; |
- |
// Record start time if this is first attempt. |
if (request.started_attempt_count() == 0) { |
RecordStartTimeUMA(request); |
@@ -886,6 +887,7 @@ void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) { |
base::Bind(&RequestCoordinator::StartOffliner, |
weak_ptr_factory_.GetWeakPtr(), request.request_id(), |
request.client_id().name_space)); |
+ return true; |
} |
void RequestCoordinator::StartOffliner( |
@@ -896,7 +898,7 @@ void RequestCoordinator::StartOffliner( |
update_result->item_statuses.size() != 1 || |
update_result->item_statuses.at(0).first != request_id || |
update_result->item_statuses.at(0).second != ItemActionStatus::SUCCESS) { |
- is_busy_ = false; |
+ state_ = RequestCoordinatorState::IDLE; |
StopProcessing(Offliner::QUEUE_UPDATE_FAILED); |
DVLOG(1) << "Failed to mark attempt started: " << request_id; |
UpdateRequestResult request_result = |
@@ -936,7 +938,7 @@ void RequestCoordinator::StartOffliner( |
watchdog_timer_.Start(FROM_HERE, timeout, this, |
&RequestCoordinator::HandleWatchdogTimeout); |
} else { |
- is_busy_ = false; |
+ state_ = RequestCoordinatorState::IDLE; |
DVLOG(0) << "Unable to start LoadAndSave"; |
StopProcessing(Offliner::LOADING_NOT_ACCEPTED); |
@@ -959,7 +961,7 @@ void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request, |
RecordOfflinerResultUMA(request.client_id(), request.creation_time(), |
last_offlining_status_); |
watchdog_timer_.Stop(); |
- is_busy_ = false; |
+ state_ = RequestCoordinatorState::IDLE; |
active_request_.reset(nullptr); |
UpdateRequestForCompletedAttempt(request, status); |