Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(77)

Issue 9667025: [net/dns] Serve requests from HOSTS file if possible. (Closed)

Created:
8 years, 9 months ago by szym
Modified:
8 years, 9 months ago
Reviewers:
cbentzel, mmenke
CC:
chromium-reviews, cbentzel+watch_chromium.org, eroman, darin-cc_chromium.org
Visibility:
Public.

Description

[net/dns] Serve requests from HOSTS file if possible. If the DnsConfig is available, we can serve the requests synchronously in Resolve. However, we might not receive the DnsConfig until the Job is placed in the queue. In that case, we attempt to serve the jobs once DnsConfig is available. BUG=114826 TEST=./net_unittests --gtest_filter=HostResolverImplTest.ServeFromHosts Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126559

Patch Set 1 #

Total comments: 2

Patch Set 2 : Added self health check. #

Patch Set 3 : Add DnsClient to allow injection of mock DnsTransactionFactory. #

Patch Set 4 : Added MockDnsClient. #

Total comments: 10

Patch Set 5 : Added tests, AF_UNSPEC support, responded to review. #

Patch Set 6 : Delinted. #

Patch Set 7 : For some bizzare reason, forward-declared DnsClient makes ./content/browser/renderer_host/render_me… #

Patch Set 8 : MSVS does not interchange struct <=> class. #

Patch Set 9 : Remov dependency on netinet/in.h. #

Patch Set 10 : Rebased to trunk. #

Total comments: 24

Patch Set 11 : Responded to review. #

Total comments: 6

Patch Set 12 : Added FirstAddressToString. #

Patch Set 13 : Updated ResolveFromCache comment #

Total comments: 2

Patch Set 14 : Added NumberOfAddresses checks. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+662 lines, -108 lines) Patch
M net/base/host_resolver.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -4 lines 0 comments Download
M net/base/host_resolver_impl.h View 1 2 3 4 5 6 7 8 9 10 7 chunks +22 lines, -7 lines 0 comments Download
M net/base/host_resolver_impl.cc View 1 2 3 4 5 6 7 8 9 10 16 chunks +107 lines, -24 lines 0 comments Download
M net/base/host_resolver_impl_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 22 chunks +237 lines, -72 lines 0 comments Download
M net/base/net_log_event_type_list.h View 1 chunk +3 lines, -0 lines 0 comments Download
A net/dns/dns_client.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +40 lines, -0 lines 0 comments Download
A net/dns/dns_client.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +58 lines, -0 lines 0 comments Download
M net/dns/dns_test_util.h View 1 2 3 4 2 chunks +21 lines, -0 lines 0 comments Download
A net/dns/dns_test_util.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +166 lines, -0 lines 0 comments Download
M net/dns/dns_transaction_unittest.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M net/net.gyp View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
szym
Needs tests. http://codereview.chromium.org/9667025/diff/1/net/base/host_resolver_impl.cc File net/base/host_resolver_impl.cc (right): http://codereview.chromium.org/9667025/diff/1/net/base/host_resolver_impl.cc#newcode1209 net/base/host_resolver_impl.cc:1209: void ServeFromHosts() { Add a bool return ...
8 years, 9 months ago (2012-03-10 06:48:48 UTC) #1
mmenke
Just some nits. Probably won't get to a (hopefully) final review of your other CL ...
8 years, 9 months ago (2012-03-12 16:23:31 UTC) #2
szym
http://codereview.chromium.org/9667025/diff/1007/net/base/host_resolver_impl.cc File net/base/host_resolver_impl.cc (right): http://codereview.chromium.org/9667025/diff/1007/net/base/host_resolver_impl.cc#newcode1842 net/base/host_resolver_impl.cc:1842: TryServingAllJobsFromHosts(); On 2012/03/12 16:23:31, Matt Menke wrote: > Some ...
8 years, 9 months ago (2012-03-12 16:34:11 UTC) #3
mmenke
http://codereview.chromium.org/9667025/diff/1007/net/base/host_resolver_impl.cc File net/base/host_resolver_impl.cc (right): http://codereview.chromium.org/9667025/diff/1007/net/base/host_resolver_impl.cc#newcode1842 net/base/host_resolver_impl.cc:1842: TryServingAllJobsFromHosts(); On 2012/03/12 16:34:12, szym wrote: > On 2012/03/12 ...
8 years, 9 months ago (2012-03-12 16:39:22 UTC) #4
szym
On 2012/03/12 16:39:22, Matt Menke wrote: > I think our time would be better spent ...
8 years, 9 months ago (2012-03-12 17:48:15 UTC) #5
szym
This should be land-able now.
8 years, 9 months ago (2012-03-12 20:14:53 UTC) #6
mmenke
http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl.cc File net/base/host_resolver_impl.cc (right): http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl.cc#newcode1593 net/base/host_resolver_impl.cc:1593: const BoundNetLog& source_net_log) { You should update the description ...
8 years, 9 months ago (2012-03-13 15:56:27 UTC) #7
szym
https://chromiumcodereview.appspot.com/9667025/diff/13017/net/base/host_resolver_impl.cc File net/base/host_resolver_impl.cc (right): https://chromiumcodereview.appspot.com/9667025/diff/13017/net/base/host_resolver_impl.cc#newcode1861 net/base/host_resolver_impl.cc:1861: TryServingAllJobsFromHosts(); On 2012/03/13 15:56:27, Matt Menke wrote: > If ...
8 years, 9 months ago (2012-03-13 18:14:29 UTC) #8
mmenke
http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl_unittest.cc File net/base/host_resolver_impl_unittest.cc (right): http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl_unittest.cc#newcode919 net/base/host_resolver_impl_unittest.cc:919: MessageLoop::current()->Run(); On 2012/03/13 18:14:29, szym wrote: > On 2012/03/13 ...
8 years, 9 months ago (2012-03-13 18:27:50 UTC) #9
szym
http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl_unittest.cc File net/base/host_resolver_impl_unittest.cc (right): http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl_unittest.cc#newcode1868 net/base/host_resolver_impl_unittest.cc:1868: hosts[DnsHostsKey("er_ipv6", ADDRESS_FAMILY_IPV6)] = local_ipv6; On 2012/03/13 18:27:50, Matt Menke ...
8 years, 9 months ago (2012-03-13 19:04:00 UTC) #10
mmenke
On 2012/03/13 19:04:00, szym wrote: > I don't want to test the IPv4 preference unless ...
8 years, 9 months ago (2012-03-13 19:15:46 UTC) #11
szym
Thanks for the review. http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl.h File net/base/host_resolver_impl.h (right): http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl.h#newcode155 net/base/host_resolver_impl.h:155: void set_dns_client_for_tests(scoped_ptr<DnsClient> client) { On ...
8 years, 9 months ago (2012-03-13 20:06:42 UTC) #12
mmenke
http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl.h File net/base/host_resolver_impl.h (right): http://codereview.chromium.org/9667025/diff/13017/net/base/host_resolver_impl.h#newcode155 net/base/host_resolver_impl.h:155: void set_dns_client_for_tests(scoped_ptr<DnsClient> client) { On 2012/03/13 20:06:42, szym wrote: ...
8 years, 9 months ago (2012-03-13 20:31:02 UTC) #13
szym
Updated. http://codereview.chromium.org/9667025/diff/12029/net/base/host_resolver_impl_unittest.cc File net/base/host_resolver_impl_unittest.cc (right): http://codereview.chromium.org/9667025/diff/12029/net/base/host_resolver_impl_unittest.cc#newcode944 net/base/host_resolver_impl_unittest.cc:944: unsigned awaited_num_completions_; On 2012/03/13 20:31:08, Matt Menke wrote: ...
8 years, 9 months ago (2012-03-13 21:17:19 UTC) #14
mmenke
LGTM! http://codereview.chromium.org/9667025/diff/12029/net/base/host_resolver_impl_unittest.cc File net/base/host_resolver_impl_unittest.cc (right): http://codereview.chromium.org/9667025/diff/12029/net/base/host_resolver_impl_unittest.cc#newcode1904 net/base/host_resolver_impl_unittest.cc:1904: ASSERT_EQ(OK, req2.result()); On 2012/03/13 21:17:19, szym wrote: > ...
8 years, 9 months ago (2012-03-13 21:23:39 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/szym@chromium.org/9667025/17001
8 years, 9 months ago (2012-03-13 23:32:39 UTC) #16
commit-bot: I haz the power
8 years, 9 months ago (2012-03-14 04:24:18 UTC) #17
Change committed as 126559

Powered by Google App Engine
This is Rietveld 408576698