[page_load_metrics] use data from WebContents::GetOpener

components/page_load_metrics/browser/metrics_web_contents_observer.cc

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..c34edfb2e37924640ec4011d77f1f1a2574afc59 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_commit_(false) {}
 
 MetricsWebContentsObserver* MetricsWebContentsObserver::CreateForWebContents(
     content::WebContents* web_contents,
@@ -517,6 +514,20 @@ void MetricsWebContentsObserver::DidStartNavigation(
     chain_size = last_aborted->aborted_chain_size() + 1;
   }
 
+  // Pass in the last committed url to the PageLoadTracker. This is used for
+  // FromGWS metrics. 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_commit_ && opener
+          ? web_contents()->GetOpener()->GetLastCommittedURL()
+          : GURL::EmptyGURL();
+  const GURL currently_committed_url =

[Bryan McQuade, 2016/04/26 18:36:39]

do you want this to be const GURL or const GURL& ?

[Charlie Harrison, 2016/04/26 19:02:41]

const GURL&. Good catch.

+      committed_load_ ? committed_load_->committed_url() : opener_url;
+
   // 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 +539,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))));
 }
 
@@ -585,6 +596,7 @@ void MetricsWebContentsObserver::DidFinishNavigation(
       navigation_handle->NavigationStart());
 
   committed_load_ = std::move(finished_nav);
+  has_commit_ = true;
   aborted_provisional_loads_.clear();
 
   const GURL& browser_url = web_contents()->GetLastCommittedURL();