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

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

Issue 2104393002: Adds UMA for PrerenderingOffliner request processing result status. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Adds note about interim offliner status codes and performs DCHECK that offliner does not return the… Created 4 years, 5 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"
11 #include "base/logging.h" 11 #include "base/logging.h"
12 #include "base/metrics/histogram_macros.h"
12 #include "base/time/time.h" 13 #include "base/time/time.h"
13 #include "components/offline_pages/background/offliner_factory.h" 14 #include "components/offline_pages/background/offliner_factory.h"
14 #include "components/offline_pages/background/offliner_policy.h" 15 #include "components/offline_pages/background/offliner_policy.h"
15 #include "components/offline_pages/background/request_picker.h" 16 #include "components/offline_pages/background/request_picker.h"
16 #include "components/offline_pages/background/save_page_request.h" 17 #include "components/offline_pages/background/save_page_request.h"
17 #include "components/offline_pages/offline_page_item.h" 18 #include "components/offline_pages/offline_page_item.h"
18 19
19 namespace offline_pages { 20 namespace offline_pages {
20 21
21 namespace { 22 namespace {
23
24 // Records the final request status UMA for an offlining request. This should
25 // only be called once per Offliner::LoadAndSave request.
26 void RecordOfflinerResultUMA(Offliner::RequestStatus request_status) {
27 UMA_HISTOGRAM_ENUMERATION("OfflinePages.Background.OfflinerRequestStatus",
28 request_status,
29 Offliner::RequestStatus::STATUS_COUNT);
30 }
31
22 // TODO(dougarnett/petewil): Move to Policy object. Also consider lower minimum 32 // TODO(dougarnett/petewil): Move to Policy object. Also consider lower minimum
23 // battery percentage once there is some processing time limits in place. 33 // battery percentage once there is some processing time limits in place.
24 const Scheduler::TriggerConditions kUserRequestTriggerConditions( 34 const Scheduler::TriggerConditions kUserRequestTriggerConditions(
25 false /* require_power_connected */, 35 false /* require_power_connected */,
26 50 /* minimum_battery_percentage */, 36 50 /* minimum_battery_percentage */,
27 false /* require_unmetered_network */); 37 false /* require_unmetered_network */);
28 38
29 // Timeout is 2.5 minutes based on the size of Marshmallow doze mode 39 // Timeout is 2.5 minutes based on the size of Marshmallow doze mode
30 // maintenance window (3 minutes) 40 // maintenance window (3 minutes)
31 // TODO(petewil): Find the optimal timeout based on data for 2G connections and 41 // TODO(petewil): Find the optimal timeout based on data for 2G connections and
(...skipping 53 matching lines...) Expand 10 before | Expand all | Expand 10 after
85 95
86 // Called in response to updating a request in the request queue. 96 // Called in response to updating a request in the request queue.
87 void RequestCoordinator::UpdateRequestCallback( 97 void RequestCoordinator::UpdateRequestCallback(
88 RequestQueue::UpdateRequestResult result) {} 98 RequestQueue::UpdateRequestResult result) {}
89 99
90 void RequestCoordinator::StopProcessing() { 100 void RequestCoordinator::StopProcessing() {
91 is_canceled_ = true; 101 is_canceled_ = true;
92 if (offliner_ && is_busy_) 102 if (offliner_ && is_busy_)
93 offliner_->Cancel(); 103 offliner_->Cancel();
94 104
105 // Stopping offliner means it will not call callback.
106 last_offlining_status_ =
107 Offliner::RequestStatus::REQUEST_COORDINATOR_CANCELED;
108 RecordOfflinerResultUMA(last_offlining_status_);
109
95 // Let the scheduler know we are done processing. 110 // Let the scheduler know we are done processing.
96 scheduler_callback_.Run(true); 111 scheduler_callback_.Run(true);
97 } 112 }
98 113
99 // Returns true if the caller should expect a callback, false otherwise. For 114 // Returns true if the caller should expect a callback, false otherwise. For
100 // instance, this would return false if a request is already in progress. 115 // instance, this would return false if a request is already in progress.
101 bool RequestCoordinator::StartProcessing( 116 bool RequestCoordinator::StartProcessing(
102 const DeviceConditions& device_conditions, 117 const DeviceConditions& device_conditions,
103 const base::Callback<void(bool)>& callback) { 118 const base::Callback<void(bool)>& callback) {
104 if (is_busy_) return false; 119 if (is_busy_) return false;
(...skipping 55 matching lines...) Expand 10 before | Expand all | Expand 10 after
160 // Start a watchdog timer to catch pre-renders running too long 175 // Start a watchdog timer to catch pre-renders running too long
161 watchdog_timer_.Start(FROM_HERE, offliner_timeout_, this, 176 watchdog_timer_.Start(FROM_HERE, offliner_timeout_, this,
162 &RequestCoordinator::StopProcessing); 177 &RequestCoordinator::StopProcessing);
163 } 178 }
164 179
165 void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request, 180 void RequestCoordinator::OfflinerDoneCallback(const SavePageRequest& request,
166 Offliner::RequestStatus status) { 181 Offliner::RequestStatus status) {
167 DVLOG(2) << "offliner finished, saved: " 182 DVLOG(2) << "offliner finished, saved: "
168 << (status == Offliner::RequestStatus::SAVED) << ", status: " 183 << (status == Offliner::RequestStatus::SAVED) << ", status: "
169 << (int) status << ", " << __FUNCTION__; 184 << (int) status << ", " << __FUNCTION__;
185 DCHECK_NE(status, Offliner::RequestStatus::UNKNOWN);
186 DCHECK_NE(status, Offliner::RequestStatus::LOADED);
170 event_logger_.RecordSavePageRequestUpdated( 187 event_logger_.RecordSavePageRequestUpdated(
171 request.client_id().name_space, 188 request.client_id().name_space,
172 "Saved", 189 "Saved",
173 request.request_id()); 190 request.request_id());
174 last_offlining_status_ = status; 191 last_offlining_status_ = status;
192 RecordOfflinerResultUMA(last_offlining_status_);
175 watchdog_timer_.Stop(); 193 watchdog_timer_.Stop();
176 194
177 is_busy_ = false; 195 is_busy_ = false;
178 196
179 // If the request succeeded, remove it from the Queue and maybe schedule 197 // If the request succeeded, remove it from the Queue and maybe schedule
180 // another one. 198 // another one.
181 if (status == Offliner::RequestStatus::SAVED) { 199 if (status == Offliner::RequestStatus::SAVED) {
182 queue_->RemoveRequest(request.request_id(), 200 queue_->RemoveRequest(request.request_id(),
183 base::Bind(&RequestCoordinator::UpdateRequestCallback, 201 base::Bind(&RequestCoordinator::UpdateRequestCallback,
184 weak_ptr_factory_.GetWeakPtr())); 202 weak_ptr_factory_.GetWeakPtr()));
(...skipping 10 matching lines...) Expand all
195 return kUserRequestTriggerConditions; 213 return kUserRequestTriggerConditions;
196 } 214 }
197 215
198 void RequestCoordinator::GetOffliner() { 216 void RequestCoordinator::GetOffliner() {
199 if (!offliner_) { 217 if (!offliner_) {
200 offliner_ = factory_->GetOffliner(policy_.get()); 218 offliner_ = factory_->GetOffliner(policy_.get());
201 } 219 }
202 } 220 }
203 221
204 } // namespace offline_pages 222 } // namespace offline_pages
OLDNEW
« no previous file with comments | « components/offline_pages/background/offliner.h ('k') | components/offline_pages/background/request_coordinator_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698