[Offline Pages] Adds event logs for requests dropped due to number of start or complete attempts.

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 76 matching lines @@
       policy_(std::move(policy)),
       factory_(std::move(factory)),
       queue_(std::move(queue)),
       scheduler_(std::move(scheduler)),
       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));
+  picker_.reset(
+      new RequestPicker(queue_.get(), policy_.get(), this, &event_logger_));
 }
 
 RequestCoordinator::~RequestCoordinator() {}
 
 bool RequestCoordinator::SavePageLater(
     const GURL& url, const ClientId& client_id, bool user_requested) {
   DVLOG(2) << "URL is " << url << " " << __func__;
 
   if (!OfflinePageModel::CanSaveURL(url)) {
     DVLOG(1) << "Not able to save page for requested url: " << url;
@@ skipping 81 matching lines @@
   }
 
   return false;
 }
 
 void RequestCoordinator::AbortRequestAttempt(SavePageRequest* request) {
   request->MarkAttemptAborted();
   if (request->started_attempt_count() >= policy_->GetMaxStartedTries()) {
     RemoveAttemptedRequest(*request,
                            BackgroundSavePageResult::START_COUNT_EXCEEDED);
+    event_logger_.RecordDroppedSavePageRequest(
+        request->client_id().name_space,
+        BackgroundSavePageResult::START_COUNT_EXCEEDED, request->request_id());

[Dmitry Titov, 2016/09/12 19:43:13]

I'd pull the constant into a local const variable and then passed it in both
places to avoid duplication and guarantee sameness. Same below.

[dougarnett, 2016/09/12 20:51:49]

Done.

   } else {
     queue_->UpdateRequest(
         *request,
         base::Bind(&RequestCoordinator::UpdateRequestCallback,
                    weak_ptr_factory_.GetWeakPtr(), request->client_id()));
   }
 }
 
 void RequestCoordinator::RemoveAttemptedRequest(
-    const SavePageRequest& request, BackgroundSavePageResult status) {
+    const SavePageRequest& request,
+    BackgroundSavePageResult result) {
   std::vector<int64_t> remove_requests;
   remove_requests.push_back(request.request_id());
   queue_->RemoveRequests(remove_requests,
                          base::Bind(&RequestCoordinator::HandleRemovedRequests,
-                                    weak_ptr_factory_.GetWeakPtr(), status));
-  RecordAttemptCount(request, status);
+                                    weak_ptr_factory_.GetWeakPtr(), result));
+  RecordAttemptCount(request, result);
 }
 
 void RequestCoordinator::RemoveRequests(
     const std::vector<int64_t>& request_ids,
     const RemoveRequestsCallback& callback) {
   bool canceled = CancelActiveRequestIfItMatches(request_ids);
   queue_->RemoveRequests(
       request_ids,
       base::Bind(&RequestCoordinator::HandleRemovedRequestsAndCallback,
                  weak_ptr_factory_.GetWeakPtr(), callback,
@@ skipping 244 matching lines @@
   }
 }
 
 void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
                                               Offliner::RequestStatus status) {
   DVLOG(2) << "offliner finished, saved: "
            << (status == Offliner::RequestStatus::SAVED)
            << ", status: " << static_cast<int>(status) << ", " << __func__;
   DCHECK_NE(status, Offliner::RequestStatus::UNKNOWN);
   DCHECK_NE(status, Offliner::RequestStatus::LOADED);
-  event_logger_.RecordSavePageRequestUpdated(request.client_id().name_space,
-                                             status, request.request_id());
+  event_logger_.RecordOfflinerResult(request.client_id().name_space, status,
+                                     request.request_id());
   last_offlining_status_ = status;
   RecordOfflinerResultUMA(request.client_id(), last_offlining_status_);
   watchdog_timer_.Stop();
 
   is_busy_ = false;
   active_request_.reset(nullptr);
 
   if (status == Offliner::RequestStatus::FOREGROUND_CANCELED ||
       status == Offliner::RequestStatus::PRERENDERING_CANCELED) {
     // Update the request for the canceled attempt.
     // TODO(dougarnett): See if we can conclusively identify other attempt
     // aborted cases to treat this way (eg, for Render Process Killed).
     SavePageRequest updated_request(request);
     AbortRequestAttempt(&updated_request);
     NotifyChanged(updated_request);
   } else if (status == Offliner::RequestStatus::SAVED) {
     // Remove the request from the queue if it succeeded.
     RemoveAttemptedRequest(request, BackgroundSavePageResult::SUCCESS);
   } else if (request.completed_attempt_count() + 1 >=
              policy_->GetMaxCompletedTries()) {
     // Remove from the request queue if we exceeded max retries. The +1
     // represents the request that just completed. Since we call
     // MarkAttemptCompleted within the if branches, the completed_attempt_count
     // has not yet been updated when we are checking the if condition.
     RemoveAttemptedRequest(request,
                            BackgroundSavePageResult::RETRY_COUNT_EXCEEDED);
+    event_logger_.RecordDroppedSavePageRequest(
+        request.client_id().name_space,
+        BackgroundSavePageResult::RETRY_COUNT_EXCEEDED, request.request_id());
   } else {
     // If we failed, but are not over the limit, update the request in the
     // queue.
     SavePageRequest updated_request(request);
     updated_request.MarkAttemptCompleted();
     queue_->UpdateRequest(updated_request,
                           base::Bind(&RequestCoordinator::UpdateRequestCallback,
                                      weak_ptr_factory_.GetWeakPtr(),
                                      updated_request.client_id()));
     NotifyChanged(updated_request);
@@ skipping 53 matching lines @@
   FOR_EACH_OBSERVER(Observer, observers_, OnChanged(request));
 }
 
 void RequestCoordinator::GetOffliner() {
   if (!offliner_) {
     offliner_ = factory_->GetOffliner(policy_.get());
   }
 }
 
 }  // namespace offline_pages