Commit 46703da0 authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot

Fix the crash in LiteVideoObserver

(i) If the mainframe is ineligible (e.g., if it's a non-http/https)
page, then skip recording metrics.

(ii) Update main frame navigation ID on same-document main frame
navigations

Bug: 1109068
Change-Id: I839393b607b6c2d090ff75ae927be7049fa4cd69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317167Reviewed-by: default avatarMichael Crouse <mcrouse@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791425}
parent 411dc874
...@@ -79,8 +79,12 @@ void LiteVideoObserver::DidStartNavigation( ...@@ -79,8 +79,12 @@ void LiteVideoObserver::DidStartNavigation(
void LiteVideoObserver::DidFinishNavigation( void LiteVideoObserver::DidFinishNavigation(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
DCHECK(navigation_handle); DCHECK(navigation_handle);
if (navigation_handle->IsInMainFrame()) {
ineligible_main_frame_ = !navigation_handle->HasCommitted() ||
!navigation_handle->GetURL().SchemeIsHTTPOrHTTPS();
}
if (!navigation_handle->HasCommitted() || if (!navigation_handle->HasCommitted() ||
navigation_handle->IsSameDocument() ||
!navigation_handle->GetURL().SchemeIsHTTPOrHTTPS()) { !navigation_handle->GetURL().SchemeIsHTTPOrHTTPS()) {
return; return;
} }
...@@ -148,7 +152,12 @@ lite_video::LiteVideoDecision LiteVideoObserver::MakeLiteVideoDecision( ...@@ -148,7 +152,12 @@ lite_video::LiteVideoDecision LiteVideoObserver::MakeLiteVideoDecision(
void LiteVideoObserver::RecordUKMMetrics( void LiteVideoObserver::RecordUKMMetrics(
lite_video::LiteVideoDecision decision, lite_video::LiteVideoDecision decision,
lite_video::LiteVideoBlocklistReason blocklist_reason) { lite_video::LiteVideoBlocklistReason blocklist_reason) {
DCHECK(current_mainframe_navigation_id_); // |current_mainframe_navigation_id_| may be null (e.g., if the mainframe was
// non-http/https).
DCHECK(current_mainframe_navigation_id_ || ineligible_main_frame_);
if (!current_mainframe_navigation_id_) {
return;
}
ukm::SourceId ukm_source_id = ukm::ConvertToSourceId( ukm::SourceId ukm_source_id = ukm::ConvertToSourceId(
*current_mainframe_navigation_id_, ukm::SourceIdType::NAVIGATION_ID); *current_mainframe_navigation_id_, ukm::SourceIdType::NAVIGATION_ID);
ukm::builders::LiteVideo builder(ukm_source_id); ukm::builders::LiteVideo builder(ukm_source_id);
......
...@@ -64,7 +64,8 @@ class LiteVideoObserver ...@@ -64,7 +64,8 @@ class LiteVideoObserver
base::Optional<lite_video::LiteVideoHint> hint) const; base::Optional<lite_video::LiteVideoHint> hint) const;
// Records the metrics for LiteVideos applied to any frames associated with // Records the metrics for LiteVideos applied to any frames associated with
// the current mainframe navigation id. Called once per frame. // the current mainframe navigation id. Called once per frame. Also, called
// for frames in the same document navigations.
void RecordUKMMetrics(lite_video::LiteVideoDecision decision, void RecordUKMMetrics(lite_video::LiteVideoDecision decision,
lite_video::LiteVideoBlocklistReason blocklist_reason); lite_video::LiteVideoBlocklistReason blocklist_reason);
...@@ -82,6 +83,9 @@ class LiteVideoObserver ...@@ -82,6 +83,9 @@ class LiteVideoObserver
// commits. // commits.
bool is_coinflip_holdback_ = false; bool is_coinflip_holdback_ = false;
// True if the main frame was not eligible for LiteVideo.
bool ineligible_main_frame_ = false;
WEB_CONTENTS_USER_DATA_KEY_DECL(); WEB_CONTENTS_USER_DATA_KEY_DECL();
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment