New issue
Advanced search Search tips

Issue 821021 link

Starred by 6 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug


Sign in to add a comment

Design a HostResolver interface for network service

Project Member Reported by xunji...@chromium.org, Mar 12 2018

Issue description

Add HostResolver Mojo interface and convert consumers. mmenke@ said this needs some design work, but can largely just keep the same API, and perhaps remove the method to clear the cache.


 
Blocking: 598073

Comment 2 by dxie@chromium.org, May 15 2018

Labels: -Pri-3 Proj-Servicification-Canary OS-All Pri-1
Owner: ericorth@chromium.org
Status: Assigned (was: Available)
Blocking: 844146

Comment 4 by dxie@chromium.org, May 18 2018

Labels: -OS-All OS-Windows OS-Linux OS-Mac OS-Chrome OS-Android
Blocking: 845648
Blockedon: 846423
Blockedon: 846454
Status: Started (was: Assigned)
Blocking: 850066
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb02821074ba14de3d6a503ef204b694b25017a4

commit cb02821074ba14de3d6a503ef204b694b25017a4
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Jun 19 15:40:30 2018

Annotate many of the browser_tests disabled under the NetworkService.

Also remove a couple that either no longer exist, have been disabled
generally due to flakiness, or are now passing.

BUG= 844950 ,  844951 ,  844952 ,  853251 ,  844928 ,
BUG= 843205 ,  844949 ,  844925 ,  844939 , 821021,
BUG=853798,  844973 ,  844927 ,  844926 ,  844950 

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I094a012fe2076c7badf86a094140c7d74db183be
Reviewed-on: https://chromium-review.googlesource.com/1104802
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568464}
[modify] https://crrev.com/cb02821074ba14de3d6a503ef204b694b25017a4/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Blocking: 853696
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fac52799b234b1d482a2d9cb4065af1fd910cd82

commit fac52799b234b1d482a2d9cb4065af1fd910cd82
Author: Eric Orth <ericorth@chromium.org>
Date: Fri Jul 13 21:11:16 2018

Add new ResolveHost API.

Primarily an interface-only change. Only change past the interface level
of the stack was to internally pass rawer information rather than
use RequestInfo as that struct is specific to the old Resolve API.

Only handles the basic case for now (HostPortPair input without any
significant options), but additional funtionality will be added in
subsequent changes.

Copy and convert all the unittests that can be implemented using the new
method with its current basic functionality. If we ever convert all
usage to the new method and delete the old, we'll be able to cleanly
delete all the old tests.

Design doc:
https://docs.google.com/document/d/1NmADJX00oRe9TxFCJTWl8ofdfyR22pbKT8-5Ad5lBsU

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I4fe454761e2930d6f235e8e505b689f8917fdeed
Reviewed-on: https://chromium-review.googlesource.com/1099427
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575065}
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/components/cronet/stale_host_resolver.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/components/cronet/stale_host_resolver.h
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/host_resolver.h
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/host_resolver_impl.h
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/host_resolver_mojo.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/host_resolver_mojo.h
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/mapped_host_resolver.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/mapped_host_resolver.h
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/dns/mock_host_resolver.h
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/proxy_resolution/proxy_resolver_v8_tracing_unittest.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/proxy_resolution/proxy_resolver_v8_tracing_wrapper_unittest.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/net/socket/socks_client_socket_unittest.cc
[modify] https://crrev.com/fac52799b234b1d482a2d9cb4065af1fd910cd82/services/network/proxy_resolver_factory_mojo_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e5a94e05aefc08fec228e375eccf64dc011d47c2

commit e5a94e05aefc08fec228e375eccf64dc011d47c2
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Jul 13 21:56:21 2018

Revert "Add new ResolveHost API."

This reverts commit fac52799b234b1d482a2d9cb4065af1fd910cd82.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 575065 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2ZhYzUyNzk5YjIzNGIxZDQ4MmEyZDljYjQwNjVhZjFmZDkxMGNkODIM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/15781

Sample Failed Step: net_unittests

Original change's description:
> Add new ResolveHost API.
> 
> Primarily an interface-only change. Only change past the interface level
> of the stack was to internally pass rawer information rather than
> use RequestInfo as that struct is specific to the old Resolve API.
> 
> Only handles the basic case for now (HostPortPair input without any
> significant options), but additional funtionality will be added in
> subsequent changes.
> 
> Copy and convert all the unittests that can be implemented using the new
> method with its current basic functionality. If we ever convert all
> usage to the new method and delete the old, we'll be able to cleanly
> delete all the old tests.
> 
> Design doc:
> https://docs.google.com/document/d/1NmADJX00oRe9TxFCJTWl8ofdfyR22pbKT8-5Ad5lBsU
> 
> Bug: 821021
> Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I4fe454761e2930d6f235e8e505b689f8917fdeed
> Reviewed-on: https://chromium-review.googlesource.com/1099427
> Commit-Queue: Eric Orth <ericorth@chromium.org>
> Reviewed-by: Helen Li <xunjieli@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575065}

Change-Id: Ifa50c76a409dde3580599275aade1686795d6c17
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1137111
Cr-Commit-Position: refs/heads/master@{#575084}
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/components/cronet/stale_host_resolver.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/components/cronet/stale_host_resolver.h
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/host_resolver.h
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/host_resolver_impl.h
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/host_resolver_mojo.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/host_resolver_mojo.h
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/mapped_host_resolver.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/mapped_host_resolver.h
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/dns/mock_host_resolver.h
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/proxy_resolution/proxy_resolver_v8_tracing_unittest.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/proxy_resolution/proxy_resolver_v8_tracing_wrapper_unittest.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/net/socket/socks_client_socket_unittest.cc
[modify] https://crrev.com/e5a94e05aefc08fec228e375eccf64dc011d47c2/services/network/proxy_resolver_factory_mojo_unittest.cc

That didn't last very long.  Looks like the issue was something specific to the "Cast Audio Linux" environment.  Investigating...
Blocking: -844146
Still digging into it, but this failure leading to the rollback is a weird one.  Culprit finder found a few different environments with the issue, some of which were tested (and passed) with the original CQ.  And so far, everything seems to be passing fine in my newly-created retry CL.

Within the failed test reports from culprit finder, the failures are all newly-added-in-the-CL _ResolveHost tests, so it does seem related to the CL.  But the failures themselves don't make much sense to me.  They're all cases testing the result of a host resolution.  In the memory dumps of expected vs actual IpEndpoint objects, the actuals looks reasonable (eg "00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-01 10-00 50-00" which parses out to ::1:80 or localhost) but the expecteds look like garbage data (eg "01-AE 9D-0C 4F-02 00-00 00-00 00-00 00-00 00-00 00-02 50-00").  Is something going wrong with the IP-from-string parsing used by the tests?
I'm running out of ideas.  Nothing jumps out as wrong with the relevant parsing code, and I can't find any recent changes to any of it.

Maybe I'll just try resubmitting the CL, and we'll see if it causes any issues again.  Maybe something else crazy was going on at the time and then culprit finder found my stuff because it added new tests (that then failed because of the crazy stuff going on).  It's a weak theory, but maybe worth a try...
Nevermind.  We found work in a DCHECK in the test that perfectly explains all the weirdness.  Fixing...
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70992982f3e286f340d427e2ad2f788db0221715

commit 70992982f3e286f340d427e2ad2f788db0221715
Author: Eric Orth <ericorth@chromium.org>
Date: Tue Jul 24 00:25:00 2018

Retry "Add new ResolveHost API."

This reverts commit e5a94e05aefc08fec228e375eccf64dc011d47c2.
Original commit fac52799b234b1d482a2d9cb4065af1fd910cd82.

Patchset 1 is a clean revert of the revert.  Fixes in subsequent patchsets.

Only change is fixing some work that was done inside a DCHECK in the tests.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ic1649dc90711c23c33255cc5f1ed9d902eb29f07
Reviewed-on: https://chromium-review.googlesource.com/1147140
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577362}
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/components/cronet/stale_host_resolver.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/components/cronet/stale_host_resolver.h
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/host_resolver.h
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/host_resolver_impl.h
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/host_resolver_mojo.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/host_resolver_mojo.h
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/mapped_host_resolver.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/mapped_host_resolver.h
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/dns/mock_host_resolver.h
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/proxy_resolution/proxy_resolver_v8_tracing_unittest.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/proxy_resolution/proxy_resolver_v8_tracing_wrapper_unittest.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/net/socket/socks_client_socket_unittest.cc
[modify] https://crrev.com/70992982f3e286f340d427e2ad2f788db0221715/services/network/proxy_resolver_factory_mojo_unittest.cc

eric, can you outline what else is needed here?
Lots of work remains here.  The biggest thing is to implement the service API itself (https://chromium-review.googlesource.com/c/chromium/src/+/1113665, still being finished up).  After that, we need to convert at least the code we care about for servicification to that new API (we may do that under a separate bug).  And while doing that, we're likely to find new cases that need to be handled by the API, leading to more work there.

I'm also investigating a quicker plan to unblock canary before we finish all the main DNS servicification work for this bug.  We would then be able to take more time to do all the actual DNS servicification work, as it's not an effort we are comfortable with rushing.
Project Member

Comment 22 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38d665f22ad1e10725afbdb30f8bc85003e3e9ee

commit 38d665f22ad1e10725afbdb30f8bc85003e3e9ee
Author: Eric Orth <ericorth@chromium.org>
Date: Thu Jul 26 19:19:06 2018

Implement CreateRequest API for MockHostResolver.

As MockHostResolver is essentially a simplified version of
HostResolverImpl, the implementation very much mirrors the non-mock
implementation.

Also allow cancellation in HangingHostResolver (by tracking number of
cancellations made since a cancelled request has no external
difference from a hung request).

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Id439203542d3206af63ba46d2906e3c6e94b6b39
Reviewed-on: https://chromium-review.googlesource.com/1125160
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578394}
[modify] https://crrev.com/38d665f22ad1e10725afbdb30f8bc85003e3e9ee/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/38d665f22ad1e10725afbdb30f8bc85003e3e9ee/net/dns/mock_host_resolver.h

Notes for the record on a backup "minimal" plan to unblock servicification canary without actually finishing up this bug and the rest of DNS servicification:
*Only root reason this is a canary blocker is the separate host caches and that, for privacy, they would all need to be cleared when we attempt to clear any.
*We could unblock canary by adding code to all the cache clearing locations (browser data removal, closing incognito) to, if network service is enabled, also directly clear the old "local" cache rather than just the service cache.
*Would have minor performance implications to have separate caches, but at least the important privacy implications would be handled.
*Better solution is still to continue with DNS servicification, so we'll only plan on implementing this if DNS becomes the long pull for canary.  Should be a fairly simple CL if we decide to do the minimal shortcut.
*Whether we do this "minimal" plan or just continue with strait DNS servicification, there is no plan to rush the actual DNS API work.  We will take our time with that to ensure it is done right, and it will likely take quite a bit of time.
-> without actually finishing up this bug and the rest of DNS servicification *first*.  We would still finish that at our leisure once unblocking canary.
Project Member

Comment 25 by bugdroid1@chromium.org, Jul 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25406959371e27414b86a5a8a233a15978e1b207

commit 25406959371e27414b86a5a8a233a15978e1b207
Author: Eric Orth <ericorth@chromium.org>
Date: Tue Jul 31 22:01:07 2018

Implement new ResolveHost API in MappedHostResolver.

Generally just applies the mapping to the input HostPortPair and passes
on to the wrapped implementation. Some special handling for when the
input is mapped to an error.

Bug: 821021
Change-Id: I0d108976e363993ebb36b8081948ef410894fa47
Reviewed-on: https://chromium-review.googlesource.com/1156856
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579587}
[modify] https://crrev.com/25406959371e27414b86a5a8a233a15978e1b207/net/dns/mapped_host_resolver.cc
[modify] https://crrev.com/25406959371e27414b86a5a8a233a15978e1b207/net/dns/mapped_host_resolver.h
[modify] https://crrev.com/25406959371e27414b86a5a8a233a15978e1b207/net/dns/mapped_host_resolver_unittest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59fa7f808447e0712688d0666c09c31bddd9c812

commit 59fa7f808447e0712688d0666c09c31bddd9c812
Author: Eric Orth <ericorth@chromium.org>
Date: Thu Aug 02 21:22:15 2018

Add mojo version for new HostResolver API.

Implemented as a single ResolveHost method in NetworkContext for now with all
the real work in a separate ResolveHostRequest class that we'll be able to use
from multiple places if we copy or move the ResolveHost method to other
interfaces.

Per recent discussions, we've diverged from the design in the doc. Responses
are now always sent via a dedicated response client mojo pipe instead of using
response callbacks on the request pipe. This makes cancellation and callback
control much clearer and bug-proofed for clients because it's always clear
which pipe controls the callbacks and the callbacks are usually tightly bound
to the relevant mojo binding. The design doc will be updated after this CL is
landed.

Design doc:
https://docs.google.com/document/d/1NmADJX00oRe9TxFCJTWl8ofdfyR22pbKT8-5Ad5lBsU

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ib0ae407008fdbec089437083796cae725cd8d8d0
Reviewed-on: https://chromium-review.googlesource.com/1113665
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580325}
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/BUILD.gn
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/network_context.cc
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/network_context.h
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/network_context_unittest.cc
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/public/mojom/BUILD.gn
[add] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/public/mojom/host_resolver.mojom
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/public/mojom/network_context.mojom
[add] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/resolve_host_request.cc
[add] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/resolve_host_request.h
[modify] https://crrev.com/59fa7f808447e0712688d0666c09c31bddd9c812/services/network/test/test_network_context.h

Cc: cduvall@chromium.org
Project Member

Comment 28 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9cdaf5da9b467d136e611d8a19953ed1005a031

commit f9cdaf5da9b467d136e611d8a19953ed1005a031
Author: Eric Orth <ericorth@chromium.org>
Date: Wed Aug 15 20:00:30 2018

Create HostResolver mojo interface.

This allows a passable interface separate from NetworkContext that can
be passed to places where NetworkContext is not easily available. The
old NetworkContext::ResolveHost still exists as a shortcut as it is
trivially implemented using an internal HostResolver.

Implementation mostly consisted of moving the ResolveHost code from
NetworkContext to the new class and adding a bit of code in
NetworkContext to manage ownership of the HostResolver objects.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I2cbb675bc584783a4add17ad7d8a6701dbaf9098
Reviewed-on: https://chromium-review.googlesource.com/1158618
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583368}
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/BUILD.gn
[add] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/host_resolver.cc
[add] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/host_resolver.h
[add] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/host_resolver_unittest.cc
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/network_context.cc
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/network_context.h
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/network_context_unittest.cc
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/public/mojom/host_resolver.mojom
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/f9cdaf5da9b467d136e611d8a19953ed1005a031/services/network/test/test_network_context.h

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13

commit 00fe5a66e4344ea063050a2b12c5d2d8b26f6c13
Author: Eric Orth <ericorth@chromium.org>
Date: Wed Aug 15 22:20:00 2018

Add a struct of optional params to net/ host resolution API.

Everything in the struct should have a reasonable default value and the
struct param itself is optional using base::Optional.

Include DNS qtype and initial priority support in the new parameter.
Lots of tests added in now that qtype and priority are supported by the
new API.

I'll add a similar struct of optional params to the mojo API in the next
CL.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I4b628364e53f1c3fdf34819071b6562eaedc9c14
Reviewed-on: https://chromium-review.googlesource.com/1171574
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583418}
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/components/cronet/stale_host_resolver.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/components/cronet/stale_host_resolver.h
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver.h
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver_impl.h
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver_mojo.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/host_resolver_mojo.h
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/mapped_host_resolver.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/mapped_host_resolver.h
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/mapped_host_resolver_unittest.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/dns/mock_host_resolver.h
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/proxy_resolution/proxy_resolver_v8_tracing_unittest.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/proxy_resolution/proxy_resolver_v8_tracing_wrapper_unittest.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/net/socket/socks_client_socket_unittest.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/services/network/proxy_resolver_factory_mojo_unittest.cc
[modify] https://crrev.com/00fe5a66e4344ea063050a2b12c5d2d8b26f6c13/services/network/resolve_host_request.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Aug 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5

commit e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Aug 15 22:26:22 2018

Convert PepperTCPSocketMessageFilter to use the mojo host resolver.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ic6422ecc5f2298c25a8720a7dfeac17114ede0ad
Reviewed-on: https://chromium-review.googlesource.com/1173492
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583419}
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/chrome/test/ppapi/ppapi_browsertest.cc
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/content/browser/renderer_host/pepper/pepper_tcp_socket_message_filter.cc
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/content/browser/renderer_host/pepper/pepper_tcp_socket_message_filter.h
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/ppapi/BUILD.gn
[add] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/ppapi/tests/test_tcp_socket_private_crash.cc
[add] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/ppapi/tests/test_tcp_socket_private_crash.h
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/services/network/host_resolver.cc
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/services/network/host_resolver.h
[modify] https://crrev.com/e0903d18987a24ef8003d758ac0c3ac3f6cf3ec5/services/network/public/mojom/network_service_test.mojom

Project Member

Comment 31 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/233489e49c60e6145eb8d1873a49e15a98236c25

commit 233489e49c60e6145eb8d1873a49e15a98236c25
Author: Eric Orth <ericorth@chromium.org>
Date: Thu Aug 16 03:04:41 2018

Add optional parameters struct to DNS Mojo API.

Everything in the struct should have a reasonable default value and the
struct param itself is optional.

Added DNS qtype and initial priority parameters. Qtype uses a new mojo
enum with mojo EnumTraits to convert to and from the net::HostResolver
enum. RequestPriority already has a mojoified version in the network
service.

Also moved the control handle parameter to the new struct as it is an
optional parameter.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I1b1b9e6d77d23216b210b56885b426bc2b710155
Reviewed-on: https://chromium-review.googlesource.com/1173337
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583518}
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/host_resolver.cc
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/host_resolver.h
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/host_resolver_unittest.cc
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/network_context.cc
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/network_context.h
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/network_context_unittest.cc
[add] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/public/cpp/host_resolver.typemap
[add] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/public/cpp/host_resolver_mojom_traits.cc
[add] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/public/cpp/host_resolver_mojom_traits.h
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/public/cpp/typemaps.gni
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/public/mojom/host_resolver.mojom
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/public/mojom/network_context.mojom
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/resolve_host_request.cc
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/resolve_host_request.h
[modify] https://crrev.com/233489e49c60e6145eb8d1873a49e15a98236c25/services/network/test/test_network_context.h

Project Member

Comment 32 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b30bc17b599d1110485be4e5c96b16a6bfa270fd

commit b30bc17b599d1110485be4e5c96b16a6bfa270fd
Author: Eric Orth <ericorth@chromium.org>
Date: Fri Aug 17 21:09:57 2018

Add is_speculative flag to DNS API.

Slight behavior change to never return any address results when the
flag is set (to prevent misuse). Affects even the old API usage, but
should be safe since a quick check showed that all current usage of the
flag never reads the resulting addresses.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I2579521d181022a8b0c425f959bcc0808f7ed7fa
Reviewed-on: https://chromium-review.googlesource.com/1177913
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584187}
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/net/dns/host_resolver.cc
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/net/dns/host_resolver.h
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/services/network/host_resolver.cc
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/services/network/host_resolver_unittest.cc
[modify] https://crrev.com/b30bc17b599d1110485be4e5c96b16a6bfa270fd/services/network/public/mojom/host_resolver.mojom

Project Member

Comment 33 by bugdroid1@chromium.org, Aug 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/322af3e45dd9b6ab6f6031b4c296860a93eabe9c

commit 322af3e45dd9b6ab6f6031b4c296860a93eabe9c
Author: Eric Orth <ericorth@chromium.org>
Date: Mon Aug 20 18:12:59 2018

Add some flags to mojo DNS API.

Adding include_canonical_name and loopback_only to the optional
parameters struct.  These will be equivalent to HOST_RESOLVER_CANONNAME
and HOST_RESOLVER_LOOPBACK_ONLY from the old API.  And at least for now,
they'll be implemented by converting to the old flags for simple
compatibility with the current implementation of HostCache::Key.

Added canonical name support to the mojo AddressList as it was missing.

Bug: 821021
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I7ff288f05d6a13e1c6c3c293d9f7a1064f011380
Reviewed-on: https://chromium-review.googlesource.com/1179996
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584500}
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/dns/host_resolver.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/dns/host_resolver.h
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/dns/host_resolver_impl.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/dns/host_resolver_impl_unittest.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/dns/mock_host_resolver.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/dns/mock_host_resolver.h
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/interfaces/address_list.mojom
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/interfaces/address_list_mojom_traits.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/net/interfaces/address_list_mojom_traits.h
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/services/network/host_resolver.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/services/network/host_resolver_unittest.cc
[modify] https://crrev.com/322af3e45dd9b6ab6f6031b4c296860a93eabe9c/services/network/public/mojom/host_resolver.mojom

Project Member

Comment 34 by bugdroid1@chromium.org, Aug 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9181af2a4360375579d951fbc0f9a9ac580216b

commit b9181af2a4360375579d951fbc0f9a9ac580216b
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Aug 21 16:33:36 2018

Convert PepperHostResolverMessageFilter to use the mojo host resolver.

Bug: 821021
Change-Id: I420e1c9493b4eea33632e4b8b20c18b0a3abcb01
Reviewed-on: https://chromium-review.googlesource.com/1182009
Reviewed-by: Doug Turner <dougt@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584780}
[modify] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/chrome/test/ppapi/ppapi_browsertest.cc
[modify] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/content/browser/BUILD.gn
[modify] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/content/browser/renderer_host/pepper/pepper_host_resolver_message_filter.cc
[modify] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/content/browser/renderer_host/pepper/pepper_host_resolver_message_filter.h
[delete] https://crrev.com/3cdb7d19ce78a109ca9e0f9cf64a0da69506d11d/content/browser/renderer_host/pepper/pepper_lookup_request.h
[modify] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/ppapi/BUILD.gn
[add] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/ppapi/tests/test_host_resolver_crash.cc
[add] https://crrev.com/b9181af2a4360375579d951fbc0f9a9ac580216b/ppapi/tests/test_host_resolver_crash.h

Blocking: 874662
Labels: -Proj-Servicification-Canary Proj-Servicification
eric, I believe every we need for Canary is done. The remaining we will take care of post canary.
Labels: Hotlist-KnownIssue
Cc: zstein@chromium.org
Labels: -Hotlist-KnownIssue
Labels: Proj-Servicification-Stable Hotlist-KnownIssue
Labels: -Proj-Servicification-Stable
Blocking: -874662

Sign in to add a comment