Accessing navigation information via webcontents

chrome/browser/net/resource_prefetch_predictor_observer.cc

Index: chrome/browser/net/resource_prefetch_predictor_observer.cc
diff --git a/chrome/browser/net/resource_prefetch_predictor_observer.cc b/chrome/browser/net/resource_prefetch_predictor_observer.cc
index 9a7db58f1f42590bfdd7cf78436b3e54969188f8..3a777a0072e13daa211b0250553725ac21bdd266 100644
--- a/chrome/browser/net/resource_prefetch_predictor_observer.cc
+++ b/chrome/browser/net/resource_prefetch_predictor_observer.cc
@@ -4,17 +4,25 @@
 
 #include "chrome/browser/net/resource_prefetch_predictor_observer.h"
 
+#include <memory>
 #include <string>
+#include <utility>
 
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"

[Benoit L, 2016/12/06 16:09:22]

nit: Is this header needed?

[ahemery, 2016/12/07 09:17:05]

Removed

 #include "content/public/browser/resource_request_info.h"
+#include "content/public/browser/web_contents.h"

[Benoit L, 2016/12/06 16:09:22]

nit: Would a simple forward declaration be enough here?

[ahemery, 2016/12/07 09:17:05]

Moved to forward declaration

 #include "net/url_request/url_request.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 using predictors::ResourcePrefetchPredictor;
+using URLRequestSummary =
+    predictors::ResourcePrefetchPredictor::URLRequestSummary;
+
 
 namespace {
 
@@ -51,6 +59,22 @@ void ReportMainFrameRequestStats(MainFrameRequestStats stat) {
                             MAIN_FRAME_REQUEST_STATS_MAX);
 }
 
+bool TryToFillNavigationID(
+    predictors::NavigationID* navigation_id,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  content::WebContents* web_contents = web_contents_getter.Run();
+  if (!web_contents)
+    return false;
+  *navigation_id = predictors::NavigationID(web_contents,

[alexilin, 2016/12/06 15:51:54]

Would you run git cl format?

[ahemery, 2016/12/07 09:17:05]

Done

+                                            main_frame_url,
+                                            creation_time);
+  return true;
+}
+
 }  // namespace
 
 namespace chrome_browser_net {
@@ -69,8 +93,8 @@ ResourcePrefetchPredictorObserver::~ResourcePrefetchPredictorObserver() {
 void ResourcePrefetchPredictorObserver::OnRequestStarted(
     net::URLRequest* request,
     content::ResourceType resource_type,
-    int child_id,
-    int frame_id) {
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (resource_type == content::RESOURCE_TYPE_MAIN_FRAME)
@@ -79,28 +103,26 @@ void ResourcePrefetchPredictorObserver::OnRequestStarted(
   if (!ResourcePrefetchPredictor::ShouldRecordRequest(request, resource_type))
     return;
 
-  ResourcePrefetchPredictor::URLRequestSummary summary;
-  summary.navigation_id.render_process_id = child_id;
-  summary.navigation_id.render_frame_id = frame_id;
-  summary.navigation_id.main_frame_url = request->first_party_for_cookies();
-  summary.navigation_id.creation_time = request->creation_time();
-  summary.resource_url = request->original_url();
-  summary.resource_type = resource_type;
+  auto summary = base::MakeUnique<URLRequestSummary>();
+  summary->resource_url = request->original_url();
+  summary->resource_type = resource_type;
 
   BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&ResourcePrefetchPredictor::RecordURLRequest,
-                 predictor_,
-                 summary));
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ResourcePrefetchPredictorObserver::OnRequestStartedOnUIThread,
+                 base::Unretained(this), base::Passed(std::move(summary)),
+                 web_contents_getter, request->first_party_for_cookies(),
+                 request->creation_time()));
 
   if (resource_type == content::RESOURCE_TYPE_MAIN_FRAME)
     ReportMainFrameRequestStats(MAIN_FRAME_REQUEST_STATS_PROCESSED_REQUESTS);
 }
 
 void ResourcePrefetchPredictorObserver::OnRequestRedirected(
+    net::URLRequest* request,
     const GURL& redirect_url,
-    net::URLRequest* request) {
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   const content::ResourceRequestInfo* request_info =
@@ -113,20 +135,21 @@ void ResourcePrefetchPredictorObserver::OnRequestRedirected(
   if (!ResourcePrefetchPredictor::ShouldRecordRedirect(request))
     return;
 
-  ResourcePrefetchPredictor::URLRequestSummary summary;
+  auto summary = base::MakeUnique<URLRequestSummary>();
   if (!ResourcePrefetchPredictor::URLRequestSummary::SummarizeResponse(
-          *request, &summary)) {
+          *request, summary.get())) {
     return;
   }
+  summary->redirect_url = redirect_url;
 
-  summary.redirect_url = redirect_url;
 
   BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&ResourcePrefetchPredictor::RecordURLRedirect,
-                 predictor_,
-                 summary));
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(
+          &ResourcePrefetchPredictorObserver::OnRequestRedirectedOnUIThread,
+          base::Unretained(this), base::Passed(std::move(summary)),
+          web_contents_getter, request->first_party_for_cookies(),
+          request->creation_time()));
 
   if (request_info &&
       request_info->GetResourceType() == content::RESOURCE_TYPE_MAIN_FRAME) {
@@ -135,7 +158,9 @@ void ResourcePrefetchPredictorObserver::OnRequestRedirected(
 }
 
 void ResourcePrefetchPredictorObserver::OnResponseStarted(
-    net::URLRequest* request) {
+    net::URLRequest* request,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   ReportRequestStats(REQUEST_STATS_TOTAL_RESPONSES);
@@ -149,18 +174,19 @@ void ResourcePrefetchPredictorObserver::OnResponseStarted(
 
   if (!ResourcePrefetchPredictor::ShouldRecordResponse(request))
     return;
-  ResourcePrefetchPredictor::URLRequestSummary summary;
+  auto summary = base::MakeUnique<URLRequestSummary>();
   if (!ResourcePrefetchPredictor::URLRequestSummary::SummarizeResponse(
-          *request, &summary)) {
+          *request, summary.get())) {
     return;
   }
 
   BrowserThread::PostTask(
-      BrowserThread::UI,
-      FROM_HERE,
-      base::Bind(&ResourcePrefetchPredictor::RecordURLResponse,
-                 predictor_,
-                 summary));
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(
+          &ResourcePrefetchPredictorObserver::OnResponseStartedOnUIThread,

[Benoit L, 2016/12/06 16:09:22]

Question: is it necessary to fully qualify the name here?

+          base::Unretained(this), base::Passed(std::move(summary)),
+          web_contents_getter, request->first_party_for_cookies(),
+          request->creation_time()));
 
   ReportRequestStats(REQUEST_STATS_TOTAL_PROCESSED_RESPONSES);
   if (request_info &&
@@ -169,4 +195,46 @@ void ResourcePrefetchPredictorObserver::OnResponseStarted(
   }
 }
 
+void ResourcePrefetchPredictorObserver::OnRequestStartedOnUIThread(
+    std::unique_ptr<URLRequestSummary> summary,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!TryToFillNavigationID(&summary->navigation_id,
+                             web_contents_getter, main_frame_url,
+                             creation_time))

[alexilin, 2016/12/06 15:51:54]

nit: git cl format here too
+ There is no strict rule but we prefer to put curly braces even for single-line
conditional statements if the condition takes more than one line. The same thing
about when the conditional block itself is single statement but takes two or
more lines.
https://google.github.io/styleguide/cppguide.html#Conditionals

[ahemery, 2016/12/07 09:17:05]

Modified to prefered format

+    return;
+  predictor_->RecordURLRequest(*summary);
+}
+
+void ResourcePrefetchPredictorObserver::OnRequestRedirectedOnUIThread(
+    std::unique_ptr<URLRequestSummary> summary,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!TryToFillNavigationID(&summary->navigation_id,

[alexilin, 2016/12/06 15:51:54]

ditto

+                             web_contents_getter, main_frame_url,
+                             creation_time))
+    return;
+  predictor_->RecordURLRedirect(*summary);
+}
+
+void ResourcePrefetchPredictorObserver::OnResponseStartedOnUIThread(
+    std::unique_ptr<URLRequestSummary> summary,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!TryToFillNavigationID(&summary->navigation_id,

[alexilin, 2016/12/06 15:51:54]

ditto

+                             web_contents_getter, main_frame_url,
+                             creation_time))
+    return;
+  predictor_->RecordURLResponse(*summary);
+}
+
 }  // namespace chrome_browser_net