Add cross origin to Blink-driven preconnect

chrome/browser/net/preconnect.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/preconnect.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/base/load_flags.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_stream_factory.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/log/net_log.h"
 #include "net/ssl/ssl_config_service.h"
 #include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 
 using content::BrowserThread;
 
 namespace chrome_browser_net {
 
 void PreconnectOnUIThread(
     const GURL& url,
     const GURL& first_party_for_cookies,
     UrlInfo::ResolutionMotivation motivation,
     int count,
     net::URLRequestContextGetter* getter) {
   // Prewarm connection to Search URL.
   BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
+      BrowserThread::IO, FROM_HERE,
       base::Bind(&PreconnectOnIOThread, url, first_party_for_cookies,
-                 motivation, count, make_scoped_refptr(getter)));
+                 motivation, count, make_scoped_refptr(getter), false));
   return;
 }
 
-
-void PreconnectOnIOThread(
-    const GURL& url,
-    const GURL& first_party_for_cookies,
-    UrlInfo::ResolutionMotivation motivation,
-    int count,
-    net::URLRequestContextGetter* getter) {
+void PreconnectOnIOThread(const GURL& url,
+                          const GURL& first_party_for_cookies,
+                          UrlInfo::ResolutionMotivation motivation,
+                          int count,
+                          net::URLRequestContextGetter* getter,
+                          bool allow_credentials) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
     LOG(DFATAL) << "This must be run only on the IO thread.";
     return;
   }
   if (!getter)
     return;
   // We are now commited to doing the async preconnection call.
   UMA_HISTOGRAM_ENUMERATION("Net.PreconnectMotivation", motivation,
                             UrlInfo::MAX_MOTIVATED);
 
   net::URLRequestContext* context = getter->GetURLRequestContext();
   net::HttpTransactionFactory* factory = context->http_transaction_factory();
   net::HttpNetworkSession* session = factory->GetSession();
 
   std::string user_agent;
   if (context->http_user_agent_settings())
     user_agent = context->http_user_agent_settings()->GetUserAgent();
   net::HttpRequestInfo request_info;
   request_info.url = url;
   request_info.method = "GET";
   request_info.extra_headers.SetHeader(net::HttpRequestHeaders::kUserAgent,
                                        user_agent);
 
   net::NetworkDelegate* delegate = context->network_delegate();
   if (delegate->CanEnablePrivacyMode(url, first_party_for_cookies))
     request_info.privacy_mode = net::PRIVACY_MODE_ENABLED;

[mmenke, 2015/07/09 15:55:56]

Wait...Privacy mode is completely separate from allow_credentials?  I thought
that was the whole point of this CL.

[Yoav Weiss, 2015/07/09 16:19:39]

Ryan suggested that it's better to separate them here, even if they are doing
the same thing (connecting a socket from the pm pool) further down the stack.

[mmenke, 2015/07/09 16:31:30]

I'm not really following that.  So if we have preconnects that are cross domain,
but don't have the crossorigin flag, they can still have allow_credentials to be
true?

So I guess PRIVACY_MODE_ENABLED does not implicitly attach those flags already?

[Ryan Sleevi, 2015/07/10 09:34:22]

It does, but it's a layering thing. See comment below (or above, depending on
how codereview does this

Right, just to refresh everyone on the "state of things"

See this summary of the state here -
https://groups.google.com/a/chromium.org/d/msg/net-dev/t1o3rGqDQMw/4Hwb0xc8__YJ

The early comments on this CL were prior to working through this issue with the
WebPerf WG and discussing with Ilya & co the goals for this, with the end
conclusion being that we agree there needs to be separate pools for first/third
party contexts (on the basis of chrome-privacy's goals for non-linkability), and
as such, privacy mode is (unfortunately?) here to stay ( see
https://codereview.chromium.org/1131293004/#msg43 )

The result of this was the comment here -
https://codereview.chromium.org/1131293004/diff/40001/chrome/browser/net/prec...

In which we're talking about the layering of things. Because the preconnect
logic doesn't shunt through a WebURLRequest, we end up "duplicating" the logic
here that is expressed in
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/web_...

On 2015/07/09 16:31:30, mmenke wrote:
That's what line 73-76 is representing - the same transformation done to
WebURLRequests when anonymous is set.

 
+  if (!allow_credentials)
+    request_info.load_flags = net::LOAD_DO_NOT_SEND_COOKIES |
+                              net::LOAD_DO_NOT_SAVE_COOKIES |
+                              net::LOAD_DO_NOT_SEND_AUTH_DATA;

[mmenke, 2015/07/09 15:55:56]

nit:  Use braces when the body of an if takes up more than one line.

[Yoav Weiss, 2015/07/09 16:19:39]

added

+
   // It almost doesn't matter whether we use net::LOWEST or net::HIGHEST
   // priority here, as we won't make a request, and will surrender the created
   // socket to the pool as soon as we can.  However, we would like to mark the
   // speculative socket as such, and IF we use a net::LOWEST priority, and if
   // a navigation asked for a socket (after us) then it would get our socket,
   // and we'd get its later-arriving socket, which might make us record that
   // the speculation didn't help :-/.  By using net::HIGHEST, we ensure that
   // a socket is given to us if "we asked first" and this allows us to mark it
   // as speculative, and better detect stats (if it gets used).
   // TODO(jar): histogram to see how often we accidentally use a previously-
@@ skipping 27 matching lines @@
 
   // All preconnects should perform EV certificate verification.
   ssl_config.verify_ev_cert = true;
 
   net::HttpStreamFactory* http_stream_factory = session->http_stream_factory();
   http_stream_factory->PreconnectStreams(count, request_info, priority,
                                          ssl_config, ssl_config);
 }
 
 }  // namespace chrome_browser_net