Implement WebPrescientNetworking to trigger preconnect from Blink

chrome/browser/net/predictor.cc

Index: chrome/browser/net/predictor.cc
diff --git a/chrome/browser/net/predictor.cc b/chrome/browser/net/predictor.cc
index 6e82188244e7451743d8fe9c2103217ce72ea212..5ec2e457c076e798b7ba17f7582ab134c5954336 100644
--- a/chrome/browser/net/predictor.cc
+++ b/chrome/browser/net/predictor.cc
@@ -273,19 +273,29 @@ void Predictor::AnticipateOmniboxUrl(const GURL& url, bool preconnectable) {
                  CanonicalizeUrl(url), motivation));
 }
 
-void Predictor::PreconnectUrlAndSubresources(const GURL& url) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+void Predictor::PreconnectUrlAndSubresources(
+    const GURL& url,
+    UrlInfo::ResolutionMotivation motivation,
+    uint32_t motivationVariationId) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
+         BrowserThread::CurrentlyOn(BrowserThread::IO));

[jar (doing other things), 2013/05/04 00:32:05]

I need to check... but the old assertion suggests this MUST be on the UI thread.
 I think that the class members can only be accessed by that thread.  I need to
understand how you have changed that so as to be safe.

After reading the rest of the CL, I *think* you should not be calling this
directly, and should instead be doing a post task to get on the UI thread (when
handling the IPC message from the renderer).

   if (!predictor_enabled_)

[jar (doing other things), 2013/05/04 00:32:05]

This is vaguely thread safe (it is "only" a bool :-/ ), but should trigger
thread sanitizer issues if we ever had a test that toggles the network
prediction for this profile (I think).  I would expect this to be toggled on the
UI thread, but now <sadly> tested on the IO thread (sometimes?).

[kouhei (in TOK), 2013/05/07 09:16:12]

Actually, some methods on this class is already called from both UI and IO
thread. For this specific method, I prefer to have this run on IO thread, as on
the UI thread, the PreconnectOnUIThread will just bounce to IO thread.
cf.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...

The variable predictor_enabled_ and url_request_context_getter_ is used on both
UI and IO thread from what I see on other methods.
There seems no problem in using url_request_context_getter_, as it is
initialized once in InitNetworkPredictor, and never touched again.
predictor_enabled_ is actually explicitly modified on IO thread, so referencing
on the IO thread is fine.
However, the original version of this method (and many other methods in this
class) does READ access to this boolean flag from UI thread.

     return;
   if (!url.is_valid() || !url.has_host())
     return;
   if (preconnect_enabled()) {
     std::string host = url.HostNoBrackets();
-    UrlInfo::ResolutionMotivation motivation(UrlInfo::EARLY_LOAD_MOTIVATED);
     const int kConnectionsNeeded = 1;
-    PreconnectOnUIThread(CanonicalizeUrl(url), motivation,
-                         kConnectionsNeeded,
-                         url_request_context_getter_);
+    // TODO(kouhei): pass motivationVariationId
+    if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+      PreconnectOnUIThread(CanonicalizeUrl(url), motivation,
+                           kConnectionsNeeded,
+                           url_request_context_getter_);

[jar (doing other things), 2013/05/04 00:32:05]

I have to read the code more to see that your access to this member is also
thread safe.

+    } else {
+      PreconnectOnIOThread(CanonicalizeUrl(url), motivation,
+                           kConnectionsNeeded,
+                           url_request_context_getter_);
+    }
     PredictFrameSubresources(url.GetWithEmptyPath());
   }
 }