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

Unified 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: Review comments incorporation Created 4 years, 9 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 side-by-side diff with in-line comments
Download patch
Index: components/page_load_metrics/browser/metrics_web_contents_observer.cc
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.cc b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
index 94528773f8549445bc1188541819f3a6ebf81317..b0815342ed5259f53455439171542bd602fc1350 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -191,9 +191,9 @@ PageLoadTracker::PageLoadTracker(
}
PageLoadTracker::~PageLoadTracker() {
- const PageLoadExtraInfo info = GetPageLoadMetricsInfo();
- if (!info.time_to_commit.is_zero() && renderer_tracked() &&
- timing_.IsEmpty()) {
+ const PageLoadExtraInfo info = ComputePageLoadExtraInfo();
+
+ if (!info.time_to_commit && renderer_tracked() && timing_.IsEmpty()) {
RecordInternalError(ERR_NO_IPCS_RECEIVED);
}
// Recall that trackers that are given ABORT_UNKNOWN_NAVIGATION have their
@@ -326,27 +326,25 @@ void PageLoadTracker::AddObserver(
observers_.push_back(std::move(observer));
}
-PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() {
- base::TimeDelta first_background_time;
- base::TimeDelta first_foreground_time;
- base::TimeDelta time_to_abort;
- base::TimeDelta time_to_commit;
+PageLoadExtraInfo PageLoadTracker::ComputePageLoadExtraInfo() {
+ base::Optional<base::TimeDelta> first_background_time;
+ base::Optional<base::TimeDelta> first_foreground_time;
+ base::Optional<base::TimeDelta> time_to_abort;
+ base::Optional<base::TimeDelta> time_to_commit;
if (!background_time_.is_null())
first_background_time = background_time_ - navigation_start_;
if (!foreground_time_.is_null())
first_foreground_time = foreground_time_ - navigation_start_;
if (abort_type_ != ABORT_NONE) {
- DCHECK_GT(abort_time_, navigation_start_);
+ DCHECK_GE(abort_time_, navigation_start_);
time_to_abort = abort_time_ - navigation_start_;
} else {
DCHECK(abort_time_.is_null());
}
if (!commit_time_.is_null()) {
- DCHECK_GT(commit_time_, navigation_start_);
+ DCHECK_GE(commit_time_, navigation_start_);
time_to_commit = commit_time_ - navigation_start_;
- } else {
- DCHECK(commit_time_.is_null());
}
return PageLoadExtraInfo(first_background_time, first_foreground_time,
started_in_foreground_,
@@ -402,7 +400,7 @@ void PageLoadTracker::UpdateAbortInternal(UserAbortType abort_type,
// infer the cause of aborts. It would be better if the navigation code could
// instead report the actual cause of an aborted navigation. See crbug/571647
// for details.
- if (timestamp <= navigation_start_) {
+ if (timestamp < navigation_start_) {
RecordInternalError(ERR_ABORT_BEFORE_NAVIGATION_START);
abort_type_ = ABORT_NONE;
abort_time_ = base::TimeTicks();

Powered by Google App Engine
This is Rietveld 408576698