Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(208)

Side by Side Diff: components/offline_pages/background/request_coordinator.cc

Issue 2325563003: [Offline Pages] Close race condition of multiple StartProcessing callers. (Closed)
Patch Set: Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/offline_pages/background/request_coordinator.h" 5 #include "components/offline_pages/background/request_coordinator.h"
6 6
7 #include <utility> 7 #include <utility>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/callback.h" 10 #include "base/callback.h"
(...skipping 60 matching lines...) Expand 10 before | Expand all | Expand 10 after
71 // is nothing for it to do. 71 // is nothing for it to do.
72 void EmptySchedulerCallback(bool started) {} 72 void EmptySchedulerCallback(bool started) {}
73 73
74 } // namespace 74 } // namespace
75 75
76 RequestCoordinator::RequestCoordinator(std::unique_ptr<OfflinerPolicy> policy, 76 RequestCoordinator::RequestCoordinator(std::unique_ptr<OfflinerPolicy> policy,
77 std::unique_ptr<OfflinerFactory> factory, 77 std::unique_ptr<OfflinerFactory> factory,
78 std::unique_ptr<RequestQueue> queue, 78 std::unique_ptr<RequestQueue> queue,
79 std::unique_ptr<Scheduler> scheduler) 79 std::unique_ptr<Scheduler> scheduler)
80 : is_busy_(false), 80 : is_busy_(false),
81 is_starting_(false),
81 is_stopped_(false), 82 is_stopped_(false),
82 use_test_connection_type_(false), 83 use_test_connection_type_(false),
83 test_connection_type_(), 84 test_connection_type_(),
84 offliner_(nullptr), 85 offliner_(nullptr),
85 policy_(std::move(policy)), 86 policy_(std::move(policy)),
86 factory_(std::move(factory)), 87 factory_(std::move(factory)),
87 queue_(std::move(queue)), 88 queue_(std::move(queue)),
88 scheduler_(std::move(scheduler)), 89 scheduler_(std::move(scheduler)),
89 active_request_(nullptr), 90 active_request_(nullptr),
90 last_offlining_status_(Offliner::RequestStatus::UNKNOWN), 91 last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
(...skipping 218 matching lines...) Expand 10 before | Expand all | Expand 10 after
309 // Let the scheduler know we are done processing. 310 // Let the scheduler know we are done processing.
310 scheduler_callback_.Run(true); 311 scheduler_callback_.Run(true);
311 } 312 }
312 313
313 // Returns true if the caller should expect a callback, false otherwise. For 314 // Returns true if the caller should expect a callback, false otherwise. For
314 // instance, this would return false if a request is already in progress. 315 // instance, this would return false if a request is already in progress.
315 bool RequestCoordinator::StartProcessing( 316 bool RequestCoordinator::StartProcessing(
316 const DeviceConditions& device_conditions, 317 const DeviceConditions& device_conditions,
317 const base::Callback<void(bool)>& callback) { 318 const base::Callback<void(bool)>& callback) {
318 current_conditions_.reset(new DeviceConditions(device_conditions)); 319 current_conditions_.reset(new DeviceConditions(device_conditions));
319 if (is_busy_) return false; 320 if (is_starting_ || is_busy_)
321 return false;
322 is_starting_ = true;
320 323
321 // Mark the time at which we started processing so we can check our time 324 // Mark the time at which we started processing so we can check our time
322 // budget. 325 // budget.
323 operation_start_time_ = base::Time::Now(); 326 operation_start_time_ = base::Time::Now();
324 327
325 is_stopped_ = false; 328 is_stopped_ = false;
326 scheduler_callback_ = callback; 329 scheduler_callback_ = callback;
327 330
328 TryNextRequest(); 331 TryNextRequest();
329 332
(...skipping 17 matching lines...) Expand all
347 } 350 }
348 351
349 void RequestCoordinator::TryNextRequest() { 352 void RequestCoordinator::TryNextRequest() {
350 // If there is no time left in the budget, return to the scheduler. 353 // If there is no time left in the budget, return to the scheduler.
351 // We do not remove the pending task that was set up earlier in case 354 // We do not remove the pending task that was set up earlier in case
352 // we run out of time, so the background scheduler will return to us 355 // we run out of time, so the background scheduler will return to us
353 // at the next opportunity to run background tasks. 356 // at the next opportunity to run background tasks.
354 if (base::Time::Now() - operation_start_time_ > 357 if (base::Time::Now() - operation_start_time_ >
355 base::TimeDelta::FromSeconds( 358 base::TimeDelta::FromSeconds(
356 policy_->GetBackgroundProcessingTimeBudgetSeconds())) { 359 policy_->GetBackgroundProcessingTimeBudgetSeconds())) {
360 is_starting_ = false;
361
357 // Let the scheduler know we are done processing. 362 // Let the scheduler know we are done processing.
358 scheduler_callback_.Run(true); 363 scheduler_callback_.Run(true);
359 364
360 return; 365 return;
361 } 366 }
362 367
363 // Choose a request to process that meets the available conditions. 368 // Choose a request to process that meets the available conditions.
364 // This is an async call, and returns right away. 369 // This is an async call, and returns right away.
365 picker_->ChooseNextRequest( 370 picker_->ChooseNextRequest(
366 base::Bind(&RequestCoordinator::RequestPicked, 371 base::Bind(&RequestCoordinator::RequestPicked,
367 weak_ptr_factory_.GetWeakPtr()), 372 weak_ptr_factory_.GetWeakPtr()),
368 base::Bind(&RequestCoordinator::RequestQueueEmpty, 373 base::Bind(&RequestCoordinator::RequestQueueEmpty,
369 weak_ptr_factory_.GetWeakPtr()), 374 weak_ptr_factory_.GetWeakPtr()),
370 current_conditions_.get()); 375 current_conditions_.get());
371 } 376 }
372 377
373 // Called by the request picker when a request has been picked. 378 // Called by the request picker when a request has been picked.
374 void RequestCoordinator::RequestPicked(const SavePageRequest& request) { 379 void RequestCoordinator::RequestPicked(const SavePageRequest& request) {
375 // Send the request on to the offliner. 380 is_starting_ = false;
376 SendRequestToOffliner(request); 381
382 // Make sure we were not stopped while picking.
383 if (!is_stopped_) {
384 // Send the request on to the offliner.
385 SendRequestToOffliner(request);
386 }
377 } 387 }
378 388
379 void RequestCoordinator::RequestQueueEmpty() { 389 void RequestCoordinator::RequestQueueEmpty() {
390 is_starting_ = false;
391
380 // Clear the outstanding "safety" task in the scheduler. 392 // Clear the outstanding "safety" task in the scheduler.
381 scheduler_->Unschedule(); 393 scheduler_->Unschedule();
382 // Let the scheduler know we are done processing. 394 // Let the scheduler know we are done processing.
383 scheduler_callback_.Run(true); 395 scheduler_callback_.Run(true);
384 } 396 }
385 397
386 void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) { 398 void RequestCoordinator::SendRequestToOffliner(const SavePageRequest& request) {
387 // Check that offlining didn't get cancelled while performing some async 399 // Check that offlining didn't get cancelled while performing some async
388 // steps. 400 // steps.
389 if (is_stopped_) 401 if (is_stopped_)
(...skipping 135 matching lines...) Expand 10 before | Expand all | Expand 10 after
525 FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request)); 537 FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request));
526 } 538 }
527 539
528 void RequestCoordinator::GetOffliner() { 540 void RequestCoordinator::GetOffliner() {
529 if (!offliner_) { 541 if (!offliner_) {
530 offliner_ = factory_->GetOffliner(policy_.get()); 542 offliner_ = factory_->GetOffliner(policy_.get());
531 } 543 }
532 } 544 }
533 545
534 } // namespace offline_pages 546 } // namespace offline_pages
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698