Skip to content

Commit

Permalink
Cleanup: Rename Navigator::navigation_data_ to metrics_data_.
Browse files Browse the repository at this point in the history 
This more clearly indicates what the member is used for, since it
exists within the Navigator class already.

There is no behavior change in this CL.

Bug: 1394513
Change-Id: I800e4e253d8e3a59565986f0359996b6f9eae238
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4126173
Reviewed-by: Sharon Yang <yangsharon@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1088898}
  • Loading branch information
@creis
creis authored and Chromium LUCI CQ committed Jan 4, 2023
1 parent b1fa19a commit 76bf805
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 45 deletions.
88 changes: 44 additions & 44 deletions content/browser/renderer_host/navigator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ void Navigator::Navigate(std::unique_ptr<NavigationRequest> request,
FrameTreeNode* frame_tree_node = request->frame_tree_node();
DCHECK_EQ(&(frame_tree_node->frame_tree()), &controller_.frame_tree());

navigation_data_ = std::make_unique<NavigationMetricsData>(
metrics_data_ = std::make_unique<NavigationMetricsData>(
request->common_params().navigation_start, request->common_params().url,
frame_tree_node->current_frame_host()->GetPageUkmSourceId(),
true /* is_browser_initiated_before_unload */);
Expand Down Expand Up @@ -1052,7 +1052,7 @@ void Navigator::OnBeginNavigation(
std::move(renderer_cancellation_listener)));
NavigationRequest* navigation_request = frame_tree_node->navigation_request();

navigation_data_ = std::make_unique<NavigationMetricsData>(
metrics_data_ = std::make_unique<NavigationMetricsData>(
navigation_request->common_params().navigation_start,
navigation_request->common_params().url,
frame_tree_node->current_frame_host()->GetPageUkmSourceId(),
Expand Down Expand Up @@ -1119,23 +1119,25 @@ void Navigator::CancelNavigation(FrameTreeNode* frame_tree_node,
frame_tree_node->navigation_request()->set_net_error(net::ERR_ABORTED);
frame_tree_node->ResetNavigationRequest(reason);
if (frame_tree_node->IsMainFrame())
navigation_data_.reset();
metrics_data_.reset();
}

void Navigator::LogCommitNavigationSent() {
if (!navigation_data_)
if (!metrics_data_) {
return;
}

navigation_data_->commit_navigation_sent_ = base::TimeTicks::Now();
metrics_data_->commit_navigation_sent_ = base::TimeTicks::Now();
}

void Navigator::LogBeforeUnloadTime(
base::TimeTicks renderer_before_unload_start_time,
base::TimeTicks renderer_before_unload_end_time,
base::TimeTicks before_unload_sent_time,
bool for_legacy) {
if (!navigation_data_)
if (!metrics_data_) {
return;
}

// LogBeforeUnloadTime is called once for each cross-process frame. Once all
// beforeunloads complete, the timestamps in navigation_data will be the
Expand All @@ -1158,21 +1160,21 @@ void Navigator::LogBeforeUnloadTime(
blink::LocalTimeTicks converted_renderer_before_unload_end =
converter.ToLocalTimeTicks(blink::RemoteTimeTicks::FromTimeTicks(
renderer_before_unload_end_time));
navigation_data_->before_unload_start_ =
metrics_data_->before_unload_start_ =
converted_renderer_before_unload_start.ToTimeTicks();
navigation_data_->before_unload_end_ =
metrics_data_->before_unload_end_ =
converted_renderer_before_unload_end.ToTimeTicks();
} else {
navigation_data_->before_unload_start_ = renderer_before_unload_start_time;
navigation_data_->before_unload_end_ = renderer_before_unload_end_time;
metrics_data_->before_unload_start_ = renderer_before_unload_start_time;
metrics_data_->before_unload_end_ = renderer_before_unload_end_time;
}
navigation_data_->before_unload_sent_ = before_unload_sent_time;
metrics_data_->before_unload_sent_ = before_unload_sent_time;
}

void Navigator::LogRendererInitiatedBeforeUnloadTime(
base::TimeTicks renderer_before_unload_start_time,
base::TimeTicks renderer_before_unload_end_time) {
DCHECK(navigation_data_);
DCHECK(metrics_data_);

if (renderer_before_unload_start_time == base::TimeTicks() ||
renderer_before_unload_end_time == base::TimeTicks())
Expand All @@ -1193,14 +1195,14 @@ void Navigator::LogRendererInitiatedBeforeUnloadTime(
blink::LocalTimeTicks converted_renderer_before_unload_end =
converter.ToLocalTimeTicks(blink::RemoteTimeTicks::FromTimeTicks(
renderer_before_unload_end_time));
navigation_data_->renderer_before_unload_start_ =
metrics_data_->renderer_before_unload_start_ =
converted_renderer_before_unload_start.ToTimeTicks();
navigation_data_->renderer_before_unload_end_ =
metrics_data_->renderer_before_unload_end_ =
converted_renderer_before_unload_end.ToTimeTicks();
} else {
navigation_data_->renderer_before_unload_start_ =
metrics_data_->renderer_before_unload_start_ =
renderer_before_unload_start_time;
navigation_data_->renderer_before_unload_end_ =
metrics_data_->renderer_before_unload_end_ =
renderer_before_unload_end_time;
}
}
Expand All @@ -1212,40 +1214,39 @@ void Navigator::RecordNavigationMetrics(
const GURL& original_request_url) {
DCHECK(site_instance->HasProcess());

if (!details.is_main_frame || !navigation_data_ ||
navigation_data_->url_ != original_request_url) {
if (!details.is_main_frame || !metrics_data_ ||
metrics_data_->url_ != original_request_url) {
return;
}

ukm::builders::Unload builder(navigation_data_->ukm_source_id_);
ukm::builders::Unload builder(metrics_data_->ukm_source_id_);
base::TimeTicks first_before_unload_start_time;

if (navigation_data_->is_browser_initiated_before_unload_) {
if (navigation_data_->before_unload_start_ &&
navigation_data_->before_unload_end_) {
if (metrics_data_->is_browser_initiated_before_unload_) {
if (metrics_data_->before_unload_start_ &&
metrics_data_->before_unload_end_) {
first_before_unload_start_time =
navigation_data_->before_unload_start_.value();
metrics_data_->before_unload_start_.value();
builder.SetBeforeUnloadDuration(
(navigation_data_->before_unload_end_.value() -
navigation_data_->before_unload_start_.value())
(metrics_data_->before_unload_end_.value() -
metrics_data_->before_unload_start_.value())
.InMilliseconds());
}
} else {
if (navigation_data_->renderer_before_unload_start_ &&
navigation_data_->renderer_before_unload_end_) {
if (metrics_data_->renderer_before_unload_start_ &&
metrics_data_->renderer_before_unload_end_) {
first_before_unload_start_time =
navigation_data_->renderer_before_unload_start_.value();
metrics_data_->renderer_before_unload_start_.value();
base::TimeDelta before_unload_duration =
navigation_data_->renderer_before_unload_end_.value() -
navigation_data_->renderer_before_unload_start_.value();
metrics_data_->renderer_before_unload_end_.value() -
metrics_data_->renderer_before_unload_start_.value();

// If we had to dispatch beforeunload handlers for OOPIFs from the
// browser, add those into the beforeunload duration as they contributed
// to the total beforeunload latency.
if (navigation_data_->before_unload_sent_) {
before_unload_duration +=
navigation_data_->before_unload_end_.value() -
navigation_data_->before_unload_start_.value();
if (metrics_data_->before_unload_sent_) {
before_unload_duration += metrics_data_->before_unload_end_.value() -
metrics_data_->before_unload_start_.value();
}
builder.SetBeforeUnloadDuration(before_unload_duration.InMilliseconds());
}
Expand All @@ -1256,18 +1257,17 @@ void Navigator::RecordNavigationMetrics(
// renderer or browser initiated navigation and could mean a long queuing time
// blocked the navigation or a long beforeunload. Records nothing if none were
// sent.
if (navigation_data_->before_unload_sent_) {
if (metrics_data_->before_unload_sent_) {
builder.SetBeforeUnloadQueueingDuration(
(navigation_data_->before_unload_start_.value() -
navigation_data_->before_unload_sent_.value())
(metrics_data_->before_unload_start_.value() -
metrics_data_->before_unload_sent_.value())
.InMilliseconds());
}

// If this is a same-process navigation and we have timestamps for unload
// durations, fill those metrics out as well.
if (params.unload_start && params.unload_end &&
params.commit_navigation_end &&
navigation_data_->commit_navigation_sent_) {
params.commit_navigation_end && metrics_data_->commit_navigation_sent_) {
base::TimeTicks unload_start = params.unload_start.value();
base::TimeTicks unload_end = params.unload_end.value();
// Note: we expect `commit_navigation_end` to be later than `unload_end`.
Expand Down Expand Up @@ -1298,24 +1298,24 @@ void Navigator::RecordNavigationMetrics(
}
builder.SetUnloadDuration((unload_end - unload_start).InMilliseconds());
builder.SetUnloadQueueingDuration(
(unload_start - navigation_data_->commit_navigation_sent_.value())
(unload_start - metrics_data_->commit_navigation_sent_.value())
.InMilliseconds());
if (navigation_data_->commit_navigation_sent_) {
if (metrics_data_->commit_navigation_sent_) {
builder.SetBeforeUnloadToCommit_SameProcess(
(commit_navigation_end - first_before_unload_start_time)
.InMilliseconds());
}
} else if (navigation_data_->commit_navigation_sent_) {
} else if (metrics_data_->commit_navigation_sent_) {
// The navigation is cross-process and we don't have unload timings as they
// are run in the old process and don't block the navigation.
builder.SetBeforeUnloadToCommit_CrossProcess(
(navigation_data_->commit_navigation_sent_.value() -
(metrics_data_->commit_navigation_sent_.value() -
first_before_unload_start_time)
.InMilliseconds());
}

builder.Record(ukm::UkmRecorder::Get());
navigation_data_.reset();
metrics_data_.reset();
}

NavigationEntryImpl*
Expand Down
3 changes: 2 additions & 1 deletion content/browser/renderer_host/navigator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ class CONTENT_EXPORT Navigator {
// events. Can be nullptr in tests.
raw_ptr<NavigatorDelegate> delegate_;

std::unique_ptr<Navigator::NavigationMetricsData> navigation_data_;
// Tracks metrics for each navigation.
std::unique_ptr<Navigator::NavigationMetricsData> metrics_data_;
};

} // namespace content
Expand Down

0 comments on commit 76bf805

Please sign in to comment.