Implement WebPrescientNetworking to trigger preconnect from Blink

chrome/browser/net/predictor.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/net/predictor.h"
 
 #include <algorithm>
 #include <cmath>
 #include <set>
 #include <sstream>
@@ skipping 255 matching lines @@
   last_omnibox_preresolve_ = now;
 
   // Perform at least DNS pre-resolution.
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(&Predictor::Resolve, base::Unretained(this),
                  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());
   }
 }
 
 UrlList Predictor::GetPredictedUrlListAtStartup(
     PrefService* user_prefs,
     PrefService* local_state) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   UrlList urls;
   // Recall list of URLs we learned about during last session.
@@ skipping 887 matching lines @@
     IOThread* io_thread,
     net::URLRequestContextGetter* getter) {
   // Empty function for unittests.
 }
 
 void SimplePredictor::ShutdownOnUIThread(PrefService* user_prefs) {
   SetShutdown(true);
 }
 
 }  // namespace chrome_browser_net