New issue
Advanced search Search tips

Issue 901419 link

Starred by 1 user
Status: Fixed
Owner:
mmenke@chromium.org
Closed: Nov 16
Cc:
ericorth@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification



Sign in to add a comment

Remove net/interfaces, moving files into services/network or services/proxy_resolver

Project Member Reported by mmenke@chromium.org, Nov 2 
Currently it uses proxy_resolver::mojom::ProxyResolverRequestClient to make DNS requests, and net::interfaces::HostResolverRequestClient to receive the response.  We should remove net::interfaces::HostResolverRequestClient entirely, and just pass in a network::mojom::HostResolver when initialize the proxy resolver.

This will give us one fewer mojo interface to worry about, and is a step towards removing mojo classes from net (The remaining mojo files in net are all just POD classes related to network addresses, and can just be moved into services/network/mojom)

Comment 1 by mmenke@chromium.org, Nov 2

Cc: -mmenke@chromium.org
Owner: mmenke@chromium.org
Status: Assigned (was: Untriaged)
Oops, meant to assign this to me.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 6 

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

commit 9fb6183aef678fb2f2ba3b9e540d0d61555ce584
Author: Matt Menke <mmenke@chromium.org>
Date: Tue Nov 06 21:03:45 2018

Move legacy DNS-related net/ mojo interfaces to proxy resolver service.

Also moves the code that imlepments each side of the interfaces from
net/ over to the location of the single consumer of each of them, in
the proxy_resolver and network services.

This CL is strictly a move, with no code changes. I will do some cleanup
work in followup CLs.

Bug:  901419 
Change-Id: I31f6bb91dccc1d250d7c2fd03a07c4173030fc97
Reviewed-on: https://chromium-review.googlesource.com/c/1320209
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605820}
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/chrome/browser/BUILD.gn
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/chrome/browser/net/chrome_mojo_proxy_resolver_factory_browsertest.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/net/BUILD.gn
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/net/dns/BUILD.gn
[delete] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/net/dns/host_resolver_mojo.cc
[delete] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/net/dns/host_resolver_mojo.h
[delete] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/net/dns/host_resolver_mojo_unittest.cc
[delete] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/net/dns/mojo_host_resolver_impl.h
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/net/interfaces/BUILD.gn
[delete] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/net/interfaces/host_resolver.typemap
[delete] https://crrev.com/69f4102d098fa48f4f7f97688fa38c5a6796f48e/net/interfaces/host_resolver_service.mojom
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/net/interfaces/typemaps.gni
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/network/BUILD.gn
[rename] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/network/mojo_host_resolver_impl.cc
[add] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/network/mojo_host_resolver_impl.h
[rename] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/network/mojo_host_resolver_impl_unittest.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/network/proxy_resolver_factory_mojo.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/network/proxy_resolver_factory_mojo_unittest.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/BUILD.gn
[add] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/host_resolver_mojo.cc
[add] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/host_resolver_mojo.h
[add] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/host_resolver_mojo_unittest.cc
[rename] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h
[rename] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings_unittest.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/proxy_resolver_factory_impl.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/proxy_resolver_factory_impl_unittest.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/proxy_resolver_impl.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/proxy_resolver_impl_unittest.cc
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/public/cpp/OWNERS
[rename] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/public/cpp/mojo_host_struct_traits.cc
[rename] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/public/cpp/mojo_host_struct_traits.h
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/public/cpp/proxy_resolver.typemap
[modify] https://crrev.com/9fb6183aef678fb2f2ba3b9e540d0d61555ce584/services/proxy_resolver/public/mojom/proxy_resolver.mojom
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 6 

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

commit c0a6a49accb13de6e951c46d0cbfbf817a6b6135
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Nov 06 21:29:02 2018

Revert "Move legacy DNS-related net/ mojo interfaces to proxy resolver service."

This reverts commit 9fb6183aef678fb2f2ba3b9e540d0d61555ce584.

Reason for revert:

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

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/linux-rel/9695

Sample Failed Step: compile

Original change's description:
> Move legacy DNS-related net/ mojo interfaces to proxy resolver service.
> 
> Also moves the code that imlepments each side of the interfaces from
> net/ over to the location of the single consumer of each of them, in
> the proxy_resolver and network services.
> 
> This CL is strictly a move, with no code changes. I will do some cleanup
> work in followup CLs.
> 
> Bug:  901419 
> Change-Id: I31f6bb91dccc1d250d7c2fd03a07c4173030fc97
> Reviewed-on: https://chromium-review.googlesource.com/c/1320209
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Eric Roman <eroman@chromium.org>
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605820}

Change-Id: Ib6e8a9b7501437d72a0a59df5881cdfc5dd12c0f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  901419 
Reviewed-on: https://chromium-review.googlesource.com/c/1320594
Cr-Commit-Position: refs/heads/master@{#605824}
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/chrome/browser/BUILD.gn
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/chrome/browser/net/chrome_mojo_proxy_resolver_factory_browsertest.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/BUILD.gn
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/BUILD.gn
[add] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/host_resolver_mojo.cc
[add] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/host_resolver_mojo.h
[add] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/host_resolver_mojo_unittest.cc
[rename] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/mojo_host_resolver_impl.cc
[add] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/mojo_host_resolver_impl.h
[rename] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/mojo_host_resolver_impl_unittest.cc
[rename] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/mojo_host_struct_traits.cc
[rename] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/dns/mojo_host_struct_traits.h
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/interfaces/BUILD.gn
[add] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/interfaces/host_resolver.typemap
[add] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/interfaces/host_resolver_service.mojom
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/interfaces/typemaps.gni
[rename] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/proxy_resolution/mojo_proxy_resolver_v8_tracing_bindings.h
[rename] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/net/proxy_resolution/mojo_proxy_resolver_v8_tracing_bindings_unittest.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/network/BUILD.gn
[delete] https://crrev.com/b318a844cbcf2f0d949eaa197c94d3115d1322ab/services/network/mojo_host_resolver_impl.h
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/network/proxy_resolver_factory_mojo.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/network/proxy_resolver_factory_mojo_unittest.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/BUILD.gn
[delete] https://crrev.com/b318a844cbcf2f0d949eaa197c94d3115d1322ab/services/proxy_resolver/host_resolver_mojo.cc
[delete] https://crrev.com/b318a844cbcf2f0d949eaa197c94d3115d1322ab/services/proxy_resolver/host_resolver_mojo.h
[delete] https://crrev.com/b318a844cbcf2f0d949eaa197c94d3115d1322ab/services/proxy_resolver/host_resolver_mojo_unittest.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/proxy_resolver_factory_impl.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/proxy_resolver_factory_impl_unittest.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/proxy_resolver_impl.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/proxy_resolver_impl_unittest.cc
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/public/cpp/OWNERS
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/public/cpp/proxy_resolver.typemap
[modify] https://crrev.com/c0a6a49accb13de6e951c46d0cbfbf817a6b6135/services/proxy_resolver/public/mojom/proxy_resolver.mojom
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7 

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

commit d60f16ec748ffec87b277d387068355bd1b65734
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Nov 07 20:18:26 2018

Move legacy DNS-related net/ mojo interfaces to proxy resolver service.

This is a reland of https://chromium-review.googlesource.com/c/1320209,
which was reverted for breaking the build in
https://chromium-review.googlesource.com/c/1320594.

The original CL removed a mojom file, but one file still tryied to
include the removed file's mojom.h header. It managed to pass the CQ
presumably because the left over mojom.h file wasn't being deleted
between builds.  This CL fixes the issue.

Original change's description:
> Move legacy DNS-related net/ mojo interfaces to proxy resolver service.
>
> Also moves the code that imlepments each side of the interfaces from
> net/ over to the location of the single consumer of each of them, in
> the proxy_resolver and network services.
>
> This CL is strictly a move, with no code changes. I will do some cleanup
> work in followup CLs.
>
> Bug:  901419 
> Change-Id: I31f6bb91dccc1d250d7c2fd03a07c4173030fc97
> Reviewed-on: https://chromium-review.googlesource.com/c/1320209
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: Eric Roman <eroman@chromium.org>
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605820}

Bug:  901419 
Change-Id: If03854352a32b182657b761459dee371d928e5cd
Reviewed-on: https://chromium-review.googlesource.com/c/1321090
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606144}
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/chrome/browser/BUILD.gn
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/chrome/browser/net/chrome_mojo_proxy_resolver_factory_browsertest.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/net/BUILD.gn
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/net/dns/BUILD.gn
[delete] https://crrev.com/afc8b5307d59f96be9668baa6937ac32db091aaf/net/dns/host_resolver_mojo.cc
[delete] https://crrev.com/afc8b5307d59f96be9668baa6937ac32db091aaf/net/dns/host_resolver_mojo.h
[delete] https://crrev.com/afc8b5307d59f96be9668baa6937ac32db091aaf/net/dns/host_resolver_mojo_unittest.cc
[delete] https://crrev.com/afc8b5307d59f96be9668baa6937ac32db091aaf/net/dns/mojo_host_resolver_impl.h
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/net/interfaces/BUILD.gn
[delete] https://crrev.com/afc8b5307d59f96be9668baa6937ac32db091aaf/net/interfaces/host_resolver.typemap
[delete] https://crrev.com/afc8b5307d59f96be9668baa6937ac32db091aaf/net/interfaces/host_resolver_service.mojom
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/net/interfaces/typemaps.gni
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/network/BUILD.gn
[rename] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/network/mojo_host_resolver_impl.cc
[add] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/network/mojo_host_resolver_impl.h
[rename] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/network/mojo_host_resolver_impl_unittest.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/network/proxy_resolver_factory_mojo.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/network/proxy_resolver_factory_mojo_unittest.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/BUILD.gn
[add] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/host_resolver_mojo.cc
[add] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/host_resolver_mojo.h
[add] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/host_resolver_mojo_unittest.cc
[rename] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h
[rename] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings_unittest.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/proxy_resolver_factory_impl.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/proxy_resolver_factory_impl_unittest.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/proxy_resolver_impl.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/proxy_resolver_impl_unittest.cc
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/public/cpp/OWNERS
[rename] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/public/cpp/mojo_host_struct_traits.cc
[rename] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/public/cpp/mojo_host_struct_traits.h
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/public/cpp/proxy_resolver.typemap
[modify] https://crrev.com/d60f16ec748ffec87b277d387068355bd1b65734/services/proxy_resolver/public/mojom/proxy_resolver.mojom

Comment 5 by mmenke@chromium.org, Nov 7

Summary: Renove net/interfaces, moving files into services/network or services/proxy_resolver (was: Convert services/proxy_resolver/ to use network::mojom::HostResolver)
Repurposing bug, per discussion with eroman.

Comment 6 by dxie@google.com, Nov 13

Labels: Hotlist-KnownIssue

Comment 7 by mmenke@chromium.org, Nov 13

Summary: Remove net/interfaces, moving files into services/network or services/proxy_resolverm (was: Renove net/interfaces, moving files into services/network or services/proxy_resolver)

Comment 8 by mmenke@chromium.org, Nov 13

Summary: Remove net/interfaces, moving files into services/network or services/proxy_resolver (was: Remove net/interfaces, moving files into services/network or services/proxy_resolverm)

Comment 9 by mmenke@chromium.org, Nov 16

Status: Fixed (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 17 

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

commit d2c25d295548d130666b7dabac0c056ed1d82847
Author: Matt Menke <mmenke@chromium.org>
Date: Sat Nov 17 01:24:34 2018

Remove net/interfaces.

All it has is an OWNERS file, left over from
https://chromium-review.googlesource.com/c/chromium/src/+/1323791

Bug:  901419 
Change-Id: Ifc6989523ffb102f4aa9f98d2d7c0c58c5e4971d
Reviewed-on: https://chromium-review.googlesource.com/c/1340524
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609063}
[delete] https://crrev.com/f4add91cbce4d83efe3f03544f706c1576388017/net/interfaces/OWNERS
►

Sign in to add a comment