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

Side by Side Diff: components/page_load_metrics/browser/metrics_web_contents_observer.cc

Issue 1837233002: Optional <TimeDelta> since they may be non existent. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Need to add Optional changes as well in this issue otherwise bots fail in compilation Created 4 years, 8 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 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 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/page_load_metrics/browser/metrics_web_contents_observer.h" 5 #include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <string> 8 #include <string>
9 #include <utility> 9 #include <utility>
10 10
(...skipping 173 matching lines...) Expand 10 before | Expand all | Expand 10 after
184 aborted_chain_size_same_url_(aborted_chain_size_same_url), 184 aborted_chain_size_same_url_(aborted_chain_size_same_url),
185 embedder_interface_(embedder_interface) { 185 embedder_interface_(embedder_interface) {
186 DCHECK(!navigation_handle->HasCommitted()); 186 DCHECK(!navigation_handle->HasCommitted());
187 embedder_interface_->RegisterObservers(this); 187 embedder_interface_->RegisterObservers(this);
188 for (const auto& observer : observers_) { 188 for (const auto& observer : observers_) {
189 observer->OnStart(navigation_handle); 189 observer->OnStart(navigation_handle);
190 } 190 }
191 } 191 }
192 192
193 PageLoadTracker::~PageLoadTracker() { 193 PageLoadTracker::~PageLoadTracker() {
194 const PageLoadExtraInfo info = GetPageLoadMetricsInfo(); 194 const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
195 if (!info.time_to_commit.is_zero() && renderer_tracked() && 195
196 timing_.IsEmpty()) { 196 if (!info.time_to_commit && renderer_tracked() && timing_.IsEmpty()) {
197 RecordInternalError(ERR_NO_IPCS_RECEIVED); 197 RecordInternalError(ERR_NO_IPCS_RECEIVED);
198 } 198 }
199 // Recall that trackers that are given ABORT_UNKNOWN_NAVIGATION have their 199 // Recall that trackers that are given ABORT_UNKNOWN_NAVIGATION have their
200 // chain length added to the next navigation. Take care not to double count 200 // chain length added to the next navigation. Take care not to double count
201 // them. Also do not double count committed loads, which call this already. 201 // them. Also do not double count committed loads, which call this already.
202 if (commit_time_.is_null() && abort_type_ != ABORT_UNKNOWN_NAVIGATION) 202 if (commit_time_.is_null() && abort_type_ != ABORT_UNKNOWN_NAVIGATION)
203 LogAbortChainHistograms(nullptr); 203 LogAbortChainHistograms(nullptr);
204 204
205 for (const auto& observer : observers_) { 205 for (const auto& observer : observers_) {
206 observer->OnComplete(timing_, info); 206 observer->OnComplete(timing_, info);
(...skipping 112 matching lines...) Expand 10 before | Expand all | Expand 10 after
319 319
320 void PageLoadTracker::set_renderer_tracked(bool renderer_tracked) { 320 void PageLoadTracker::set_renderer_tracked(bool renderer_tracked) {
321 renderer_tracked_ = renderer_tracked; 321 renderer_tracked_ = renderer_tracked;
322 } 322 }
323 323
324 void PageLoadTracker::AddObserver( 324 void PageLoadTracker::AddObserver(
325 scoped_ptr<PageLoadMetricsObserver> observer) { 325 scoped_ptr<PageLoadMetricsObserver> observer) {
326 observers_.push_back(std::move(observer)); 326 observers_.push_back(std::move(observer));
327 } 327 }
328 328
329 PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() { 329 PageLoadExtraInfo PageLoadTracker::ComputePageLoadExtraInfo() {
330 base::TimeDelta first_background_time; 330 base::Optional<base::TimeDelta> first_background_time;
331 base::TimeDelta first_foreground_time; 331 base::Optional<base::TimeDelta> first_foreground_time;
332 base::TimeDelta time_to_abort; 332 base::Optional<base::TimeDelta> time_to_abort;
333 base::TimeDelta time_to_commit; 333 base::Optional<base::TimeDelta> time_to_commit;
334 if (!background_time_.is_null()) 334 if (!background_time_.is_null())
335 first_background_time = background_time_ - navigation_start_; 335 first_background_time = background_time_ - navigation_start_;
336 if (!foreground_time_.is_null()) 336 if (!foreground_time_.is_null())
337 first_foreground_time = foreground_time_ - navigation_start_; 337 first_foreground_time = foreground_time_ - navigation_start_;
338 if (abort_type_ != ABORT_NONE) { 338 if (abort_type_ != ABORT_NONE) {
339 DCHECK_GT(abort_time_, navigation_start_); 339 DCHECK_GE(abort_time_, navigation_start_);
340 time_to_abort = abort_time_ - navigation_start_; 340 time_to_abort = abort_time_ - navigation_start_;
341 } else { 341 } else {
342 DCHECK(abort_time_.is_null()); 342 DCHECK(abort_time_.is_null());
343 } 343 }
344 344
345 if (!commit_time_.is_null()) { 345 if (!commit_time_.is_null()) {
346 DCHECK_GT(commit_time_, navigation_start_); 346 DCHECK_GE(commit_time_, navigation_start_);
347 time_to_commit = commit_time_ - navigation_start_; 347 time_to_commit = commit_time_ - navigation_start_;
348 } else {
349 DCHECK(commit_time_.is_null());
350 } 348 }
351 return PageLoadExtraInfo(first_background_time, first_foreground_time, 349 return PageLoadExtraInfo(first_background_time, first_foreground_time,
352 started_in_foreground_, 350 started_in_foreground_,
353 commit_time_.is_null() ? GURL() : url_, 351 commit_time_.is_null() ? GURL() : url_,
354 time_to_commit, abort_type_, time_to_abort); 352 time_to_commit, abort_type_, time_to_abort);
355 } 353 }
356 354
357 void PageLoadTracker::NotifyAbort(UserAbortType abort_type, 355 void PageLoadTracker::NotifyAbort(UserAbortType abort_type,
358 base::TimeTicks timestamp) { 356 base::TimeTicks timestamp) {
359 DCHECK_NE(abort_type, ABORT_NONE); 357 DCHECK_NE(abort_type, ABORT_NONE);
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after
395 base::TimeTicks timestamp) { 393 base::TimeTicks timestamp) {
396 // When a provisional navigation commits, that navigation's start time is 394 // When a provisional navigation commits, that navigation's start time is
397 // interpreted as the abort time for other provisional loads in the tab. 395 // interpreted as the abort time for other provisional loads in the tab.
398 // However, this only makes sense if the committed load started after the 396 // However, this only makes sense if the committed load started after the
399 // aborted provisional loads started. Thus we ignore cases where the committed 397 // aborted provisional loads started. Thus we ignore cases where the committed
400 // load started before the aborted provisional load, as this would result in 398 // load started before the aborted provisional load, as this would result in
401 // recording a negative time-to-abort. The real issue here is that we have to 399 // recording a negative time-to-abort. The real issue here is that we have to
402 // infer the cause of aborts. It would be better if the navigation code could 400 // infer the cause of aborts. It would be better if the navigation code could
403 // instead report the actual cause of an aborted navigation. See crbug/571647 401 // instead report the actual cause of an aborted navigation. See crbug/571647
404 // for details. 402 // for details.
405 if (timestamp <= navigation_start_) { 403 if (timestamp < navigation_start_) {
406 RecordInternalError(ERR_ABORT_BEFORE_NAVIGATION_START); 404 RecordInternalError(ERR_ABORT_BEFORE_NAVIGATION_START);
407 abort_type_ = ABORT_NONE; 405 abort_type_ = ABORT_NONE;
408 abort_time_ = base::TimeTicks(); 406 abort_time_ = base::TimeTicks();
409 return; 407 return;
410 } 408 }
411 abort_type_ = abort_type; 409 abort_type_ = abort_type;
412 abort_time_ = timestamp; 410 abort_time_ = timestamp;
413 } 411 }
414 412
415 // static 413 // static
(...skipping 264 matching lines...) Expand 10 before | Expand all | Expand 10 after
680 678
681 if (!committed_load_->UpdateTiming(timing)) { 679 if (!committed_load_->UpdateTiming(timing)) {
682 // If the page load tracker cannot update its timing, something is wrong 680 // If the page load tracker cannot update its timing, something is wrong
683 // with the IPC (it's from another load, or it's invalid in some other way). 681 // with the IPC (it's from another load, or it's invalid in some other way).
684 // We expect this to be a rare occurrence. 682 // We expect this to be a rare occurrence.
685 RecordInternalError(ERR_BAD_TIMING_IPC); 683 RecordInternalError(ERR_BAD_TIMING_IPC);
686 } 684 }
687 } 685 }
688 686
689 } // namespace page_load_metrics 687 } // namespace page_load_metrics
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698