Added chrome-side support for link rel=preconnect

Added chrome-side support for link rel=preconnect

This plumbs the blink preconnect support through to the existing
preconnect implementation exposed by the predictor. There was an
existing IPC for most of the plumbing that wasn't being used.

There was a bit of structural movement:

- Moved the IPC definition to the new network_hints component
- Renamed the IPC message to fall into a namespace used by network_hints
- Renamed the DNS prefetch IPC into the same namespace
- Added a connection count to the preconnect IPC

And the actual plumbing itself which mirrors the behavior of the DNS
prefetch plumbing where it queues the requests and posts a task back to
itself for submitting the actual requests at a later point.

It also counts preconnect requests for the same URL and uses that as an
indicator that more connections are needed (as many connections as
requests up to a limit of 6). This will only work if the URLs are
discovered after each other and before the scheduled task gets run which
will easily be the case for link tags that are grouped together but may
not be the case if they are scattered through the head (an acceptable
tradeoff).  The net stack will de-dupe preconnect requests and just
leverage existing or pending connections which is why we need to
accumulate and count before requesting.

BUG=450682
Committed: https://crrev.com/ec1939d34edef32fbda9e2a6d3722749e343f5a8
Cr-Commit-Position: refs/heads/master@{#315782}

[Pat Meenan, 2015-02-06 17:53:38]

pmeenan@chromium.org changed reviewers:
+ mkwst@chromium.org, mmenke@chromium.org, thakis@chromium.org,
ttuttle@chromium.org

[Pat Meenan, 2015-02-06 17:53:38]

pmeenan@chromium.org changed required reviewers:
+ mkwst@chromium.org, mmenke@chromium.org, thakis@chromium.org,
ttuttle@chromium.org

[Pat Meenan, 2015-02-06 17:53:39]

mmenke, ttuttle: PTAL at the network_hints component plumbing into the
predictor.

mkwst: PTAL at the moving of the IPC message from render_messages into the
component (and the rename from DNSPrefetchMsg* to NetworkHintsMsg*)

thakis: PTAL at changes to the IPC message processing in
chrome_message_render_filter and the additional debug string in url_info.cc

[Mike West, 2015-02-07 04:40:50]

IPC rename LGTM.

[Pat Meenan, 2015-02-07 14:15:18]

New patchsets have been uploaded after l-g-t-m from mkwst@chromium.org

[Deprecated (see juliatuttle), 2015-02-10 17:48:42]

LGTM. Predictor plumbing looks reasonable, but I'm not intimately familiar with
the predictor myself.

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_render_message_filter.cc (right):

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_render_message_filter.cc:117: count = 1;
I'd discard the message (and maybe log a warning?) if count < 1. That shouldn't
happen, and it seems weird to silently fix it up.

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_render_message_filter.cc:118: else if (count
> 6)
Make this a constant, or pull it from somewhere else (if it's e.g. some
connection limit set elsewhere)?

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.cc (right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
2015?

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.h (right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.h:1: // Copyright (c) 2011
The Chromium Authors. All rights reserved.
2015?

[Nico, 2015-02-10 17:52:49]

my files lgtm (somewhat stampy)

[mmenke, 2015-02-10 17:57:25]

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_render_message_filter.cc (right):

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_render_message_filter.cc:118: else if (count
> 6)
On 2015/02/10 17:48:42, ttuttle wrote:
> Make this a constant, or pull it from somewhere else (if it's e.g. some
> connection limit set elsewhere)?

Can we just preconnect however many are requested?  If we don't allow that many
preconnects, so what?  Doesn't seem like this should be the place we check.  The
socket pools already do this, anyways. 
https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/client_...

[mmenke, 2015-02-10 18:10:14]

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/prescient_networking_dispatcher.h
(right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/prescient_networking_dispatcher.h:27:
network_hints::RendererPreconnect  preconnect_;
Remove extra space

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.cc (right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:9: #include <ctype.h>
What is this needed for?

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:31: if (!url.is_valid()
|| !url.has_host() || !url.has_scheme())
GURL can't have relative URLs, I believe.  So the first should imply the next
two.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:44: FROM_HERE,
base::Bind(&RendererPreconnect::SubmitPreconnect,
include base/location.h for FROM_HERE

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:55: it !=
url_request_count_map_.end(); ++it) {
Use a C++x11 range loop?  const auto&...  (If I weren't suggesting you change
the loop, I'd suggest this use const_iterator)

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:62: }  // namespcae
network_hints
typo:  namespace

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.h (right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.h:1: // Copyright (c) 2011
The Chromium Authors. All rights reserved.
On 2015/02/10 17:48:42, ttuttle wrote:
> 2015?

Also, newer files shouldn't use the (c)

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.h:21: #include <string>
I don't think this is used

[Pat Meenan, 2015-02-10 18:44:15]

New patchsets have been uploaded after l-g-t-m from
ttuttle@chromium.org,thakis@chromium.org

[Pat Meenan, 2015-02-10 18:44:50]

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_render_message_filter.cc (right):

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_render_message_filter.cc:117: count = 1;
On 2015/02/10 17:48:42, ttuttle wrote:
> I'd discard the message (and maybe log a warning?) if count < 1. That
shouldn't
> happen, and it seems weird to silently fix it up.

Done.

https://codereview.chromium.org/899883004/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_render_message_filter.cc:118: else if (count
> 6)
On 2015/02/10 17:57:25, mmenke wrote:
> On 2015/02/10 17:48:42, ttuttle wrote:
> > Make this a constant, or pull it from somewhere else (if it's e.g. some
> > connection limit set elsewhere)?
> 
> Can we just preconnect however many are requested?  If we don't allow that
many
> preconnects, so what?  Doesn't seem like this should be the place we check. 
The
> socket pools already do this, anyways. 
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/socket/client_...

Done.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/prescient_networking_dispatcher.h
(right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/prescient_networking_dispatcher.h:27:
network_hints::RendererPreconnect  preconnect_;
On 2015/02/10 18:10:14, mmenke wrote:
> Remove extra space

Done.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.cc (right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2015/02/10 17:48:42, ttuttle wrote:
> 2015?

Done.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:9: #include <ctype.h>
On 2015/02/10 18:10:14, mmenke wrote:
> What is this needed for?

Removed. Leftover from copying the DNS code to use as a basis.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:31: if (!url.is_valid()
|| !url.has_host() || !url.has_scheme())
On 2015/02/10 18:10:14, mmenke wrote:
> GURL can't have relative URLs, I believe.  So the first should imply the next
> two.

Done.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:44: FROM_HERE,
base::Bind(&RendererPreconnect::SubmitPreconnect,
On 2015/02/10 18:10:14, mmenke wrote:
> include base/location.h for FROM_HERE

Done. The DNS prefetch code was doing the same thing so I added base/location.h
there as well.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:55: it !=
url_request_count_map_.end(); ++it) {
On 2015/02/10 18:10:14, mmenke wrote:
> Use a C++x11 range loop?  const auto&...  (If I weren't suggesting you change
> the loop, I'd suggest this use const_iterator)

Done.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:62: }  // namespcae
network_hints
On 2015/02/10 18:10:14, mmenke wrote:
> typo:  namespace

Done. Typo was copied from the DNS prefetch code so I fixed it over there as
well.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.h (right):

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.h:1: // Copyright (c) 2011
The Chromium Authors. All rights reserved.
On 2015/02/10 17:48:42, ttuttle wrote:
> 2015?

Done.

https://codereview.chromium.org/899883004/diff/20001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.h:21: #include <string>
On 2015/02/10 18:10:14, mmenke wrote:
> I don't think this is used

Removed. Leftover when I used strings instead of GURLs for the list of URLs.

[mmenke, 2015-02-10 18:49:59]

LGTM, modulo nit

https://codereview.chromium.org/899883004/diff/40001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.cc (right):

https://codereview.chromium.org/899883004/diff/40001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:53: for (const auto
&entry: url_request_count_map_) {
nit:  for (const auto& entry : url_request_count_map_) {

(& next to auto rather than entry, space before colon)

[Pat Meenan, 2015-02-10 19:05:05]

New patchsets have been uploaded after l-g-t-m from mmenke@chromium.org

[Pat Meenan, 2015-02-10 19:05:37]

https://codereview.chromium.org/899883004/diff/40001/components/network_hints...
File components/network_hints/renderer/renderer_preconnect.cc (right):

https://codereview.chromium.org/899883004/diff/40001/components/network_hints...
components/network_hints/renderer/renderer_preconnect.cc:53: for (const auto
&entry: url_request_count_map_) {
On 2015/02/10 18:49:59, mmenke wrote:
> nit:  for (const auto& entry : url_request_count_map_) {
> 
> (& next to auto rather than entry, space before colon)

Done.

[Pat Meenan, 2015-02-10 20:23:08]

The CQ bit was checked by pmeenan@chromium.org

[commit-bot: I haz the power, 2015-02-10 20:23:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/899883004/60001

[commit-bot: I haz the power, 2015-02-10 20:31:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-10 20:31:33]

Try jobs failed on following builders:
  ios_rel_device_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)

[Pat Meenan, 2015-02-10 21:07:46]

pmeenan@chromium.org changed reviewers:
+ jam@chromium.org

[Pat Meenan, 2015-02-10 21:07:46]

pmeenan@chromium.org changed required reviewers:
+ jam@chromium.org

[Pat Meenan, 2015-02-10 21:07:47]

jam@ could you PTAL at the new DEPS file for the network_hints component (ipc/
and content/public in particular) which were added for the preconnect IPC
message handling - https://codereview.chromium.org/899883004/patch/60001/70006

[jam, 2015-02-11 17:02:22]

On 2015/02/10 21:07:47, Pat Meenan wrote:
> jam@ could you PTAL at the new DEPS file for the network_hints component (ipc/
> and content/public in particular) which were added for the preconnect IPC
> message handling - https://codereview.chromium.org/899883004/patch/60001/70006

lgtm

[jam, 2015-02-11 17:02:28]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2015-02-11 17:03:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/899883004/60001

[commit-bot: I haz the power, 2015-02-11 17:08:18]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-11 17:08:59]

Patchset 4 (id:??) landed as
https://crrev.com/ec1939d34edef32fbda9e2a6d3722749e343f5a8
Cr-Commit-Position: refs/heads/master@{#315782}