Accessing navigation information via webcontents

chrome/browser/net/resource_prefetch_predictor_observer.h

Index: chrome/browser/net/resource_prefetch_predictor_observer.h
diff --git a/chrome/browser/net/resource_prefetch_predictor_observer.h b/chrome/browser/net/resource_prefetch_predictor_observer.h
index 9bd1694b6961bab67e1b01823ff3a9cc58faaa72..708c5650291b137b38cfc4f8b73c0f4f5199f3e6 100644
--- a/chrome/browser/net/resource_prefetch_predictor_observer.h
+++ b/chrome/browser/net/resource_prefetch_predictor_observer.h
@@ -5,9 +5,12 @@
 #ifndef CHROME_BROWSER_NET_RESOURCE_PREFETCH_PREDICTOR_OBSERVER_H_
 #define CHROME_BROWSER_NET_RESOURCE_PREFETCH_PREDICTOR_OBSERVER_H_
 
+#include <memory>

[ahemery, 2016/12/06 14:31:12]

For unique_ptr

[Benoit L, 2016/12/06 14:58:18]

Thanks for adding the comments in the review, but these are not required, as we
trust you to have run "git cl lint".

+
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
+#include "content/public/browser/resource_request_info.h"
 #include "content/public/common/resource_type.h"
 
 namespace net {
@@ -22,8 +25,9 @@ namespace chrome_browser_net {
 // the ResourcePrefetchPredictor about the ones it is interested in.
 //  - Has an instance per profile, and is owned by the corresponding
 //    ProfileIOData.
-//  - Needs to be constructed on UI thread. Rest of the functions can only be
-//    called on the IO thread. Can be destroyed on UI or IO thread.
+//  - Needs to be constructed on UI thread. Can be destroyed on UI or IO thread.
+//    As for member functions, public members are meant to be called on the IO
+//    thread and private members from the UI thread.
 class ResourcePrefetchPredictorObserver {
  public:
   explicit ResourcePrefetchPredictorObserver(
@@ -31,14 +35,51 @@ class ResourcePrefetchPredictorObserver {
   ~ResourcePrefetchPredictorObserver();
 
   // Parts of the ResourceDispatcherHostDelegate that we want to observe.
-  void OnRequestStarted(net::URLRequest* request,
-                        content::ResourceType resource_type,
-                        int child_id,
-                        int frame_id);
-  void OnRequestRedirected(const GURL& redirect_url, net::URLRequest* request);
-  void OnResponseStarted(net::URLRequest* request);
+  void OnRequestStarted(
+      net::URLRequest* request,
+      content::ResourceType resource_type,
+      const content::ResourceRequestInfo::WebContentsGetter&
+          web_contents_getter);
+  void OnRequestRedirected(
+      net::URLRequest* request,
+      const GURL& redirect_url,
+      const content::ResourceRequestInfo::WebContentsGetter&
+          web_contents_getter);
+  void OnResponseStarted(
+      net::URLRequest* request,
+      const content::ResourceRequestInfo::WebContentsGetter&
+          web_contents_getter);
 

[ahemery, 2016/12/06 14:31:11]

Comment about the additional parameters. WebContents is only accessible from UI
thread, and request is tightly coupled to the IO thread. Therefore we extract
necessary information in the IO thread and pass it along to the UI thread to be
used. We completely fill the navigation_id in a single place.

  private:
+  void OnRequestStartedOnUIThread(
+    std::unique_ptr<predictors::ResourcePrefetchPredictor::URLRequestSummary>
+        summary,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const;
+  void OnRequestRedirectedOnUIThread(
+    std::unique_ptr<predictors::ResourcePrefetchPredictor::URLRequestSummary>
+        summary,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const;
+  void OnResponseStartedOnUIThread(
+    std::unique_ptr<predictors::ResourcePrefetchPredictor::URLRequestSummary>
+        summary,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const;
+
+  bool TryToFillNavigationID(

[Benoit L, 2016/12/06 14:58:18]

This can be made static, right?
If so, please make it a standalone function in an anonymous namespace in the .cc
file.

[ahemery, 2016/12/06 15:17:38]

Now an implementation detail

+    predictors::NavigationID* navigation_id,
+    const content::ResourceRequestInfo::WebContentsGetter&
+        web_contents_getter,
+    const GURL& main_frame_url,
+    const base::TimeTicks& creation_time) const;
+
   // Owned by profile.
   base::WeakPtr<predictors::ResourcePrefetchPredictor> predictor_;