Adds simple browser-side implementation for DNS prefetching.

Adds simple browser-side implementation for DNS prefetching.

R=wzhong@chromium.org,jar@chromium.org,tsepez@chromium.org,creis@chromium.org
BUG=419909
Committed: https://crrev.com/00df169a990ea09d488f1c13ec0293fc11d7992e
Cr-Commit-Position: refs/heads/master@{#307137}

[gunsch, 2014-11-18 17:46:42]

Hi all:

@wzhong/jar: please review the implementation. wzhong@ wrote the original in the
Chromecast codebase that this is based off of.

@creis: review for new dependency on content/public/browser, from
components/dns_prefetch/browser/.

@tsepez: review for new dependency on ipc, from
components/dns_prefetch/browser/.

https://codereview.chromium.org/730833004/diff/1/components/dns_prefetch/brow...
File components/dns_prefetch/browser/DEPS (right):

https://codereview.chromium.org/730833004/diff/1/components/dns_prefetch/brow...
components/dns_prefetch/browser/DEPS:3: "+ipc",
@tsepez: this DEPS is already declared in components/dns_prefetch/common. Would
it be more appropriate to move this to components/dns_prefetch/DEPS, or have it
declared both here and there?

[Tom Sepez, 2014-11-18 17:56:07]

> @tsepez: this DEPS is already declared in components/dns_prefetch/common.
Would
> it be more appropriate to move this to components/dns_prefetch/DEPS, or have
it
> declared both here and there?
Either is acceptable, LGTM on the DEPS.

[Charlie Reis, 2014-11-18 19:20:27]

[+davidben for FYI]

I don't know enough about the DNS prefetch component to say whether it actually
needs to depend on content, but content/public/browser seems reasonable to me. 
Rubber stamp LGTM on the dependency.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
File components/dns_prefetch/browser/net_message_filter.cc (right):

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:50: 
nit: Remove extra line.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:83: 
nit: Remove extra line.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:86: VLOG(2) <<
__FUNCTION__;
Did you intend to include VLOG statements here?

[mmenke, 2014-11-18 19:28:08]

mmenke@chromium.org changed reviewers:
+ mmenke@chromium.org

[mmenke, 2014-11-18 19:28:09]

Drive by nits

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
File components/dns_prefetch/browser/net_message_filter.h (right):

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:8: #include
"components/dns_prefetch/common/prefetch_common.h"
Can just forward declare LookupRequest instead of including this.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:25: virtual bool
OnMessageReceived(const IPC::Message& message) override;
-virtual

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:28: virtual
~NetMessageFilter();
-virtual +override

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:34:
DISALLOW_COPY_AND_ASSIGN(NetMessageFilter);
include macros.h for DISALLOW_COPY_AND_ASSIGN

[gunsch, 2014-11-18 20:45:57]

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
File components/dns_prefetch/browser/net_message_filter.cc (right):

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:50: 
On 2014/11/18 19:20:26, Charlie Reis wrote:
> nit: Remove extra line.

Done.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:83: 
On 2014/11/18 19:20:27, Charlie Reis wrote:
> nit: Remove extra line.

Done.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:86: VLOG(2) <<
__FUNCTION__;
On 2014/11/18 19:20:27, Charlie Reis wrote:
> Did you intend to include VLOG statements here?

Removed.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
File components/dns_prefetch/browser/net_message_filter.h (right):

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:8: #include
"components/dns_prefetch/common/prefetch_common.h"
On 2014/11/18 19:28:09, mmenke wrote:
> Can just forward declare LookupRequest instead of including this.

Done.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:25: virtual bool
OnMessageReceived(const IPC::Message& message) override;
On 2014/11/18 19:28:09, mmenke wrote:
> -virtual

Done.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:28: virtual
~NetMessageFilter();
On 2014/11/18 19:28:09, mmenke wrote:
> -virtual +override

Done.

https://codereview.chromium.org/730833004/diff/20001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.h:34:
DISALLOW_COPY_AND_ASSIGN(NetMessageFilter);
On 2014/11/18 19:28:09, mmenke wrote:
> include macros.h for DISALLOW_COPY_AND_ASSIGN

Done.

[gunsch, 2014-11-19 17:21:35]

gunsch@chromium.org changed reviewers:
+ ttuttle@chromium.org
- jar@chromium.org

[gunsch, 2014-11-19 17:21:35]

gunsch@chromium.org changed required reviewers:
+ creis@chromium.org, tsepez@chromium.org, ttuttle@chromium.org

[gunsch, 2014-11-19 17:21:56]

@ttuttle: can you take a look at the DNS prefetching code itself?

[mmenke, 2014-11-19 17:27:56]

mmenke@chromium.org changed reviewers:
- mmenke@chromium.org

[gunsch, 2014-12-02 15:40:15]

@ttuttle: ping?

[Deprecated (see juliatuttle), 2014-12-05 18:23:48]

lgtm modulo nits:

https://codereview.chromium.org/730833004/diff/40001/components/dns_prefetch/...
File components/dns_prefetch/browser/net_message_filter.cc (right):

https://codereview.chromium.org/730833004/diff/40001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:53: VLOG(2) <<
__FUNCTION__ << ": " << hostname_ << ", result=" << result;
Do you need all of the VLOGs in this file?

https://codereview.chromium.org/730833004/diff/40001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:86: DnsLookupRequest*
request = new DnsLookupRequest(host_resolver_, hostname);
It took me a while to figure out that this is freed by the base::Owned in the
callback inside it. Perhaps add a comment to that extent?

[gunsch, 2014-12-05 23:54:00]

https://codereview.chromium.org/730833004/diff/40001/components/dns_prefetch/...
File components/dns_prefetch/browser/net_message_filter.cc (right):

https://codereview.chromium.org/730833004/diff/40001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:53: VLOG(2) <<
__FUNCTION__ << ": " << hostname_ << ", result=" << result;
Removed the others (they seem kind of excessive) but I do think this one's more
practical.

https://codereview.chromium.org/730833004/diff/40001/components/dns_prefetch/...
components/dns_prefetch/browser/net_message_filter.cc:86: DnsLookupRequest*
request = new DnsLookupRequest(host_resolver_, hostname);
On 2014/12/05 18:23:48, ttuttle wrote:
> It took me a while to figure out that this is freed by the base::Owned in the
> callback inside it. Perhaps add a comment to that extent?

Done.

[gunsch, 2014-12-06 00:25:45]

The CQ bit was checked by gunsch@chromium.org

[commit-bot: I haz the power, 2014-12-06 00:26:41]

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

[commit-bot: I haz the power, 2014-12-06 01:33:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-06 01:33:55]

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