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

Unified Diff: components/page_load_metrics/browser/metrics_web_contents_observer.cc

Issue 1924543002: [page_load_metrics] use data from WebContents::GetOpener (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@just_gws_aborts
Patch Set: final review 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 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 540646e5dbc58ebf2c742a523d8a2df364d11b86..c0da0d4e8bea0b866280b95ebd29982e26f91d5d 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -208,7 +208,7 @@ void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url) {
PageLoadTracker::PageLoadTracker(
bool in_foreground,
PageLoadMetricsEmbedderInterface* embedder_interface,
- PageLoadTracker* const currently_committed_load_or_null,
+ const GURL& currently_committed_url,
content::NavigationHandle* navigation_handle,
int aborted_chain_size,
int aborted_chain_size_same_url)
@@ -222,10 +222,6 @@ PageLoadTracker::PageLoadTracker(
embedder_interface_(embedder_interface) {
DCHECK(!navigation_handle->HasCommitted());
embedder_interface_->RegisterObservers(this);
- const GURL& currently_committed_url =
- currently_committed_load_or_null
- ? currently_committed_load_or_null->committed_url()
- : GURL::EmptyGURL();
for (const auto& observer : observers_) {
observer->OnStart(navigation_handle, currently_committed_url);
}
@@ -462,7 +458,8 @@ MetricsWebContentsObserver::MetricsWebContentsObserver(
std::unique_ptr<PageLoadMetricsEmbedderInterface> embedder_interface)
: content::WebContentsObserver(web_contents),
in_foreground_(false),
- embedder_interface_(std::move(embedder_interface)) {}
+ embedder_interface_(std::move(embedder_interface)),
+ has_navigated_(false) {}
MetricsWebContentsObserver* MetricsWebContentsObserver::CreateForWebContents(
content::WebContents* web_contents,
@@ -501,6 +498,8 @@ void MetricsWebContentsObserver::DidStartNavigation(
return;
if (embedder_interface_->IsPrerendering(web_contents()))
return;
+ if (navigation_handle->GetURL().spec().compare(url::kAboutBlankURL) == 0)
+ return;
std::unique_ptr<PageLoadTracker> last_aborted =
NotifyAbortedProvisionalLoadsNewNavigation(navigation_handle);
@@ -517,6 +516,20 @@ void MetricsWebContentsObserver::DidStartNavigation(
chain_size = last_aborted->aborted_chain_size() + 1;
}
+ // Pass in the last committed url to the PageLoadTracker. If the MWCO has
+ // never observed a committed load, use the last committed url from this
+ // WebContent's opener. This is more accurate than using referrers due to
+ // referrer sanitizing and origin referrers. Note that this could potentially
+ // be inaccurate if the opener has since navigated.
+ content::WebContents* opener = web_contents()->GetOpener();
+ const GURL& opener_url =
+ !has_navigated_ && opener
+ ? web_contents()->GetOpener()->GetLastCommittedURL()
+ : GURL::EmptyGURL();
+ const GURL& currently_committed_url =
+ committed_load_ ? committed_load_->committed_url() : opener_url;
+ has_navigated_ = true;
+
// We can have two provisional loads in some cases. E.g. a same-site
// navigation can have a concurrent cross-process navigation started
// from the omnibox.
@@ -528,7 +541,7 @@ void MetricsWebContentsObserver::DidStartNavigation(
provisional_loads_.insert(std::make_pair(
navigation_handle,
base::WrapUnique(new PageLoadTracker(
- in_foreground_, embedder_interface_.get(), committed_load_.get(),
+ in_foreground_, embedder_interface_.get(), currently_committed_url,
navigation_handle, chain_size, chain_size_same_url))));
}

Powered by Google App Engine
This is Rietveld 408576698