Commit ff3dd021 authored by csharrison's avatar csharrison Committed by Commit bot

This patch uses the WebContents' opener to extract the previous committed url

for the very first navigation within the MetricsWebContentsObserver.

This is used for the FromGWS abort metrics.

This patch was split from the previous CL here:
https://codereview.chromium.org/1901303004/

BUG=605259

Review URL: https://codereview.chromium.org/1924543002

Cr-Commit-Position: refs/heads/master@{#389893}
parent 71361c3f
...@@ -208,7 +208,7 @@ void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url) { ...@@ -208,7 +208,7 @@ void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url) {
PageLoadTracker::PageLoadTracker( PageLoadTracker::PageLoadTracker(
bool in_foreground, bool in_foreground,
PageLoadMetricsEmbedderInterface* embedder_interface, PageLoadMetricsEmbedderInterface* embedder_interface,
PageLoadTracker* const currently_committed_load_or_null, const GURL& currently_committed_url,
content::NavigationHandle* navigation_handle, content::NavigationHandle* navigation_handle,
int aborted_chain_size, int aborted_chain_size,
int aborted_chain_size_same_url) int aborted_chain_size_same_url)
...@@ -222,10 +222,6 @@ PageLoadTracker::PageLoadTracker( ...@@ -222,10 +222,6 @@ PageLoadTracker::PageLoadTracker(
embedder_interface_(embedder_interface) { embedder_interface_(embedder_interface) {
DCHECK(!navigation_handle->HasCommitted()); DCHECK(!navigation_handle->HasCommitted());
embedder_interface_->RegisterObservers(this); 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_) { for (const auto& observer : observers_) {
observer->OnStart(navigation_handle, currently_committed_url); observer->OnStart(navigation_handle, currently_committed_url);
} }
...@@ -462,7 +458,8 @@ MetricsWebContentsObserver::MetricsWebContentsObserver( ...@@ -462,7 +458,8 @@ MetricsWebContentsObserver::MetricsWebContentsObserver(
std::unique_ptr<PageLoadMetricsEmbedderInterface> embedder_interface) std::unique_ptr<PageLoadMetricsEmbedderInterface> embedder_interface)
: content::WebContentsObserver(web_contents), : content::WebContentsObserver(web_contents),
in_foreground_(false), in_foreground_(false),
embedder_interface_(std::move(embedder_interface)) {} embedder_interface_(std::move(embedder_interface)),
has_navigated_(false) {}
MetricsWebContentsObserver* MetricsWebContentsObserver::CreateForWebContents( MetricsWebContentsObserver* MetricsWebContentsObserver::CreateForWebContents(
content::WebContents* web_contents, content::WebContents* web_contents,
...@@ -501,6 +498,8 @@ void MetricsWebContentsObserver::DidStartNavigation( ...@@ -501,6 +498,8 @@ void MetricsWebContentsObserver::DidStartNavigation(
return; return;
if (embedder_interface_->IsPrerendering(web_contents())) if (embedder_interface_->IsPrerendering(web_contents()))
return; return;
if (navigation_handle->GetURL().spec().compare(url::kAboutBlankURL) == 0)
return;
std::unique_ptr<PageLoadTracker> last_aborted = std::unique_ptr<PageLoadTracker> last_aborted =
NotifyAbortedProvisionalLoadsNewNavigation(navigation_handle); NotifyAbortedProvisionalLoadsNewNavigation(navigation_handle);
...@@ -517,6 +516,20 @@ void MetricsWebContentsObserver::DidStartNavigation( ...@@ -517,6 +516,20 @@ void MetricsWebContentsObserver::DidStartNavigation(
chain_size = last_aborted->aborted_chain_size() + 1; 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 // We can have two provisional loads in some cases. E.g. a same-site
// navigation can have a concurrent cross-process navigation started // navigation can have a concurrent cross-process navigation started
// from the omnibox. // from the omnibox.
...@@ -528,7 +541,7 @@ void MetricsWebContentsObserver::DidStartNavigation( ...@@ -528,7 +541,7 @@ void MetricsWebContentsObserver::DidStartNavigation(
provisional_loads_.insert(std::make_pair( provisional_loads_.insert(std::make_pair(
navigation_handle, navigation_handle,
base::WrapUnique(new PageLoadTracker( 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)))); navigation_handle, chain_size, chain_size_same_url))));
} }
......
...@@ -112,7 +112,7 @@ class PageLoadTracker { ...@@ -112,7 +112,7 @@ class PageLoadTracker {
// the constructor. // the constructor.
PageLoadTracker(bool in_foreground, PageLoadTracker(bool in_foreground,
PageLoadMetricsEmbedderInterface* embedder_interface, PageLoadMetricsEmbedderInterface* embedder_interface,
PageLoadTracker* const currently_committed_load_or_null, const GURL& currently_committed_url,
content::NavigationHandle* navigation_handle, content::NavigationHandle* navigation_handle,
int aborted_chain_size, int aborted_chain_size,
int aborted_chain_size_same_url); int aborted_chain_size_same_url);
...@@ -293,6 +293,9 @@ class MetricsWebContentsObserver ...@@ -293,6 +293,9 @@ class MetricsWebContentsObserver
std::unique_ptr<PageLoadTracker> committed_load_; std::unique_ptr<PageLoadTracker> committed_load_;
// Has the MWCO observed at least one navigation?
bool has_navigated_;
DISALLOW_COPY_AND_ASSIGN(MetricsWebContentsObserver); DISALLOW_COPY_AND_ASSIGN(MetricsWebContentsObserver);
}; };
......
...@@ -12,8 +12,6 @@ ...@@ -12,8 +12,6 @@
namespace page_load_metrics { namespace page_load_metrics {
class PageLoadMetricsObservable;
// This enum represents how a page load ends. If the action occurs before the // This enum represents how a page load ends. If the action occurs before the
// page load finishes (or reaches some point like first paint), then we consider // page load finishes (or reaches some point like first paint), then we consider
// the load to be aborted. // the load to be aborted.
......
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