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

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

Issue 2361883002: [Offline Pages] Adds classification of some prerender FinalStatus codes as canceled operations or a… (Closed)
Patch Set: Reworked per feedback Created 4 years, 2 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 510 matching lines...) Expand 10 before | Expand all | Expand 10 after
521 status == Offliner::RequestStatus::PRERENDERING_CANCELED) { 521 status == Offliner::RequestStatus::PRERENDERING_CANCELED) {
522 // Update the request for the canceled attempt. 522 // Update the request for the canceled attempt.
523 // TODO(dougarnett): See if we can conclusively identify other attempt 523 // TODO(dougarnett): See if we can conclusively identify other attempt
524 // aborted cases to treat this way (eg, for Render Process Killed). 524 // aborted cases to treat this way (eg, for Render Process Killed).
525 SavePageRequest updated_request(request); 525 SavePageRequest updated_request(request);
526 AbortRequestAttempt(&updated_request); 526 AbortRequestAttempt(&updated_request);
527 NotifyChanged(updated_request); 527 NotifyChanged(updated_request);
528 } else if (status == Offliner::RequestStatus::SAVED) { 528 } else if (status == Offliner::RequestStatus::SAVED) {
529 // Remove the request from the queue if it succeeded. 529 // Remove the request from the queue if it succeeded.
530 RemoveAttemptedRequest(request, BackgroundSavePageResult::SUCCESS); 530 RemoveAttemptedRequest(request, BackgroundSavePageResult::SUCCESS);
531 } else if (status == Offliner::RequestStatus::PRERENDERING_FAILED_NO_RETRY) {
532 RemoveAttemptedRequest(request,
533 BackgroundSavePageResult::PRERENDER_FAILURE);
531 } else if (request.completed_attempt_count() + 1 >= 534 } else if (request.completed_attempt_count() + 1 >=
532 policy_->GetMaxCompletedTries()) { 535 policy_->GetMaxCompletedTries()) {
533 // Remove from the request queue if we exceeded max retries. The +1 536 // Remove from the request queue if we exceeded max retries. The +1
534 // represents the request that just completed. Since we call 537 // represents the request that just completed. Since we call
535 // MarkAttemptCompleted within the if branches, the completed_attempt_count 538 // MarkAttemptCompleted within the if branches, the completed_attempt_count
536 // has not yet been updated when we are checking the if condition. 539 // has not yet been updated when we are checking the if condition.
537 const BackgroundSavePageResult result( 540 const BackgroundSavePageResult result(
538 BackgroundSavePageResult::RETRY_COUNT_EXCEEDED); 541 BackgroundSavePageResult::RETRY_COUNT_EXCEEDED);
539 event_logger_.RecordDroppedSavePageRequest(request.client_id().name_space, 542 event_logger_.RecordDroppedSavePageRequest(request.client_id().name_space,
540 result, request.request_id()); 543 result, request.request_id());
541 RemoveAttemptedRequest(request, result); 544 RemoveAttemptedRequest(request, result);
542 } else { 545 } else {
543 // If we failed, but are not over the limit, update the request in the 546 // If we failed, but are not over the limit, update the request in the
544 // queue. 547 // queue.
545 SavePageRequest updated_request(request); 548 SavePageRequest updated_request(request);
546 updated_request.MarkAttemptCompleted(); 549 updated_request.MarkAttemptCompleted();
547 queue_->UpdateRequest(updated_request, 550 queue_->UpdateRequest(updated_request,
548 base::Bind(&RequestCoordinator::UpdateRequestCallback, 551 base::Bind(&RequestCoordinator::UpdateRequestCallback,
549 weak_ptr_factory_.GetWeakPtr(), 552 weak_ptr_factory_.GetWeakPtr(),
550 updated_request.client_id())); 553 updated_request.client_id()));
551 NotifyChanged(updated_request); 554 NotifyChanged(updated_request);
552 } 555 }
553 556
554 // Determine whether we might try another request in this 557 // Determine whether we might try another request in this
555 // processing window based on how the previous request completed. 558 // processing window based on how the previous request completed.
556 //
557 // TODO(dougarnett): Need to split PRERENDERING_FAILED into separate
558 // codes as to whether we should try another request or not.
559 switch (status) { 559 switch (status) {
560 case Offliner::RequestStatus::SAVED: 560 case Offliner::RequestStatus::SAVED:
561 case Offliner::RequestStatus::SAVE_FAILED: 561 case Offliner::RequestStatus::SAVE_FAILED:
562 case Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED: // timeout 562 case Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED:
563 case Offliner::RequestStatus::REQUEST_COORDINATOR_TIMED_OUT:
564 case Offliner::RequestStatus::PRERENDERING_FAILED_NO_RETRY:
563 // Consider processing another request in this service window. 565 // Consider processing another request in this service window.
564 TryNextRequest(); 566 TryNextRequest();
565 break; 567 break;
566 case Offliner::RequestStatus::FOREGROUND_CANCELED: 568 case Offliner::RequestStatus::FOREGROUND_CANCELED:
567 case Offliner::RequestStatus::PRERENDERING_CANCELED: 569 case Offliner::RequestStatus::PRERENDERING_CANCELED:
568 case Offliner::RequestStatus::PRERENDERING_FAILED: 570 case Offliner::RequestStatus::PRERENDERING_FAILED:
569 // No further processing in this service window. 571 // No further processing in this service window.
570 break; 572 break;
571 default: 573 default:
572 // Make explicit choice about new status codes that actually reach here. 574 // Make explicit choice about new status codes that actually reach here.
(...skipping 36 matching lines...) Expand 10 before | Expand all | Expand 10 after
609 if (!offliner_) { 611 if (!offliner_) {
610 offliner_ = factory_->GetOffliner(policy_.get()); 612 offliner_ = factory_->GetOffliner(policy_.get());
611 } 613 }
612 } 614 }
613 615
614 void RequestCoordinator::Shutdown() { 616 void RequestCoordinator::Shutdown() {
615 network_quality_estimator_ = nullptr; 617 network_quality_estimator_ = nullptr;
616 } 618 }
617 619
618 } // namespace offline_pages 620 } // namespace offline_pages
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698