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

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

Issue 2543093002: Split the RequestPicker task into two separate tasks. (Closed)
Patch Set: ADD TODO Created 4 years 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 <limits> 7 #include <limits>
8 #include <utility> 8 #include <utility>
9 9
10 #include "base/bind.h" 10 #include "base/bind.h"
(...skipping 163 matching lines...) Expand 10 before | Expand all | Expand 10 after
174 queue_(std::move(queue)), 174 queue_(std::move(queue)),
175 scheduler_(std::move(scheduler)), 175 scheduler_(std::move(scheduler)),
176 policy_controller_(new ClientPolicyController()), 176 policy_controller_(new ClientPolicyController()),
177 network_quality_estimator_(network_quality_estimator), 177 network_quality_estimator_(network_quality_estimator),
178 active_request_(nullptr), 178 active_request_(nullptr),
179 last_offlining_status_(Offliner::RequestStatus::UNKNOWN), 179 last_offlining_status_(Offliner::RequestStatus::UNKNOWN),
180 scheduler_callback_(base::Bind(&EmptySchedulerCallback)), 180 scheduler_callback_(base::Bind(&EmptySchedulerCallback)),
181 immediate_schedule_callback_(base::Bind(&EmptySchedulerCallback)), 181 immediate_schedule_callback_(base::Bind(&EmptySchedulerCallback)),
182 weak_ptr_factory_(this) { 182 weak_ptr_factory_(this) {
183 DCHECK(policy_ != nullptr); 183 DCHECK(policy_ != nullptr);
184 std::unique_ptr<PickRequestTaskFactory> picker_factory( 184 std::unique_ptr<CleanupTaskFactory> cleanup_factory(
185 new PickRequestTaskFactory(policy_.get(), this, &event_logger_)); 185 new CleanupTaskFactory(policy_.get(), this, &event_logger_));
186 queue_->SetPickerFactory(std::move(picker_factory)); 186 queue_->SetCleanupFactory(std::move(cleanup_factory));
187 // Do a cleanup at startup time.
188 queue_->CleanupRequestQueue();
187 } 189 }
188 190
189 RequestCoordinator::~RequestCoordinator() {} 191 RequestCoordinator::~RequestCoordinator() {}
190 192
191 int64_t RequestCoordinator::SavePageLater(const GURL& url, 193 int64_t RequestCoordinator::SavePageLater(const GURL& url,
192 const ClientId& client_id, 194 const ClientId& client_id,
193 bool user_requested, 195 bool user_requested,
194 RequestAvailability availability) { 196 RequestAvailability availability) {
195 DVLOG(2) << "URL is " << url << " " << __func__; 197 DVLOG(2) << "URL is " << url << " " << __func__;
196 198
(...skipping 437 matching lines...) Expand 10 before | Expand all | Expand 10 after
634 // TODO: Make sure the scheduler callback is valid before running it. 636 // TODO: Make sure the scheduler callback is valid before running it.
635 scheduler_callback_.Run(true); 637 scheduler_callback_.Run(true);
636 DVLOG(2) << " out of time, giving up. " << __func__; 638 DVLOG(2) << " out of time, giving up. " << __func__;
637 639
638 return; 640 return;
639 } 641 }
640 642
641 // Ask request queue to make a new PickRequestTask object, then put it on the 643 // Ask request queue to make a new PickRequestTask object, then put it on the
642 // task queue. 644 // task queue.
643 queue_->PickNextRequest( 645 queue_->PickNextRequest(
644 base::Bind(&RequestCoordinator::RequestPicked, 646 policy_.get(), base::Bind(&RequestCoordinator::RequestPicked,
645 weak_ptr_factory_.GetWeakPtr()), 647 weak_ptr_factory_.GetWeakPtr()),
646 base::Bind(&RequestCoordinator::RequestNotPicked, 648 base::Bind(&RequestCoordinator::RequestNotPicked,
647 weak_ptr_factory_.GetWeakPtr()), 649 weak_ptr_factory_.GetWeakPtr()),
648 base::Bind(&RequestCoordinator::RequestCounts, 650 base::Bind(&RequestCoordinator::RequestCounts,
649 weak_ptr_factory_.GetWeakPtr(), is_start_of_processing), 651 weak_ptr_factory_.GetWeakPtr(), is_start_of_processing),
650 *current_conditions_.get(), disabled_requests_); 652 *current_conditions_.get(), disabled_requests_);
651 // TODO(petewil): Verify current_conditions has a good value on all calling 653 // TODO(petewil): Verify current_conditions has a good value on all calling
652 // paths. It is really more of a "last known conditions" than "current 654 // paths. It is really more of a "last known conditions" than "current
653 // conditions". Consider having a call to Java to check the current 655 // conditions". Consider having a call to Java to check the current
654 // conditions. 656 // conditions.
655 } 657 }
656 658
657 // Called by the request picker when a request has been picked. 659 // Called by the request picker when a request has been picked.
658 void RequestCoordinator::RequestPicked(const SavePageRequest& request) { 660 void RequestCoordinator::RequestPicked(const SavePageRequest& request,
661 bool cleanup_needed) {
659 DVLOG(2) << request.url() << " " << __func__; 662 DVLOG(2) << request.url() << " " << __func__;
660 is_starting_ = false; 663 is_starting_ = false;
661 664
662 // Make sure we were not stopped while picking. 665 // Make sure we were not stopped while picking.
663 if (processing_state_ != ProcessingWindowState::STOPPED) { 666 if (processing_state_ != ProcessingWindowState::STOPPED) {
664 // Send the request on to the offliner. 667 // Send the request on to the offliner.
665 SendRequestToOffliner(request); 668 SendRequestToOffliner(request);
666 } 669 }
670
671 // Schedule a queue cleanup if needed.
672 if (cleanup_needed)
673 queue_->CleanupRequestQueue();
667 } 674 }
668 675
669 void RequestCoordinator::RequestNotPicked( 676 void RequestCoordinator::RequestNotPicked(
670 bool non_user_requested_tasks_remaining) { 677 bool non_user_requested_tasks_remaining,
678 bool cleanup_needed) {
671 DVLOG(2) << __func__; 679 DVLOG(2) << __func__;
672 is_starting_ = false; 680 is_starting_ = false;
673 681
674 // Clear the outstanding "safety" task in the scheduler. 682 // Clear the outstanding "safety" task in the scheduler.
675 scheduler_->Unschedule(); 683 scheduler_->Unschedule();
676 684
677 // If disabled tasks remain, post a new safety task for 5 sec from now. 685 // If disabled tasks remain, post a new safety task for 5 sec from now.
678 if (disabled_requests_.size() > 0) { 686 if (disabled_requests_.size() > 0) {
679 scheduler_->BackupSchedule(GetTriggerConditions(kUserRequest), 687 scheduler_->BackupSchedule(GetTriggerConditions(kUserRequest),
680 kDisabledTaskRecheckSeconds); 688 kDisabledTaskRecheckSeconds);
681 } else if (non_user_requested_tasks_remaining) { 689 } else if (non_user_requested_tasks_remaining) {
682 // If we don't have any of those, check for non-user-requested tasks. 690 // If we don't have any of those, check for non-user-requested tasks.
683 scheduler_->Schedule(GetTriggerConditions(!kUserRequest)); 691 scheduler_->Schedule(GetTriggerConditions(!kUserRequest));
684 } 692 }
685 693
694 // Schedule a queue cleanup if needed.
695 if (cleanup_needed)
696 queue_->CleanupRequestQueue();
697
686 // Let the scheduler know we are done processing. 698 // Let the scheduler know we are done processing.
687 scheduler_callback_.Run(true); 699 scheduler_callback_.Run(true);
688 } 700 }
689 701
690 void RequestCoordinator::RequestCounts(bool is_start_of_processing, 702 void RequestCoordinator::RequestCounts(bool is_start_of_processing,
691 size_t total_requests, 703 size_t total_requests,
692 size_t available_requests) { 704 size_t available_requests) {
693 // Only capture request counts for the start of processing (not for 705 // Only capture request counts for the start of processing (not for
694 // continued processing in the same window). 706 // continued processing in the same window).
695 if (!is_start_of_processing) 707 if (!is_start_of_processing)
(...skipping 286 matching lines...) Expand 10 before | Expand all | Expand 10 after
982 994
983 ClientPolicyController* RequestCoordinator::GetPolicyController() { 995 ClientPolicyController* RequestCoordinator::GetPolicyController() {
984 return policy_controller_.get(); 996 return policy_controller_.get();
985 } 997 }
986 998
987 void RequestCoordinator::Shutdown() { 999 void RequestCoordinator::Shutdown() {
988 network_quality_estimator_ = nullptr; 1000 network_quality_estimator_ = nullptr;
989 } 1001 }
990 1002
991 } // namespace offline_pages 1003 } // namespace offline_pages
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698