Issue metadata
Sign in to add a comment
|
"proxy_resolver_factory" member of network.mojom.NetworkContextParams is disregarded on iOS |
||||||||||||||||||||||||
Issue descriptionIt should be possible to provide a |proxy_resolver_factory| mojo interface when creating a NetworkContext on iOS. Currently that doesn't work as "URLRequestContextBuilderMojo::CreateProxyResolutionService" #ifdefs it out on iOS and instead just uses the system proxy resolver: https://chromium.googlesource.com/chromium/src/+/5e6e9bf248b3e26c0ea9e4f0511c8fcbefdb2656/services/network/url_request_context_builder_mojo.cc#46 This short-circuit was probably intended to avoid wiring up a V8-backed proxy resolver factory implementation for the production path. Not connecting a V8-backed proxy resolver implementation is correct, however the layering of this check is currently in the wrong place as it prevents passing *any* proxy resolver implementation in, including a mock one. This is affecting the test NetworkContextTest.ProxyErrorClientNotifiedOfPacError, which is unable to mock proxy resolution when run on iOS.
,
Sep 6
Ken, do you know who would be a good person to fix this issue?
,
Sep 6
Adding some network service folks
,
Sep 7
This stems from this iOS exclusion in services/network/BUILD.gn [1] # TODO(sdefresne): This depends on net's enable_net_mojo getting turned on for # iOS, which depends on net_with_v8 as well. http://crbug.com/803149 if (!is_ios) { sources += [ "proxy_resolver_factory_mojo.cc", "proxy_resolver_factory_mojo.h", "proxy_service_mojo.cc", "proxy_service_mojo.h", ] deps += [ "//net/dns:mojo_service" ] } I don't think that dependency graph quite makes sense. We can probably compile those files on iOS and just exclude connecting to the actual proxy_resolver service (whose implementation does depend on V8). I can give this a try, but may punt back if it turns into a bigger task :) [1] https://cs.chromium.org/chromium/src/services/network/BUILD.gn?q=services/network/BUILD.gn&sq=package:chromium&g=0&l=212
,
Sep 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4102bdc56cf0febf57a275dff5391f6d8c9783b3 commit 4102bdc56cf0febf57a275dff5391f6d8c9783b3 Author: Eric Roman <eroman@chromium.org> Date: Wed Sep 12 19:14:28 2018 Allow specifying a proxy resolver factory (mojo pipe) on iOS. In production, iOS Chrome continues to use an in-process approach to proxy resolving using the system's proxy resolver. However similarly to other platforms, the NetworkContext now makes use of the |proxy_resolver_factory| parameter. This is exercised by a unit-test which uses it to configure a mock proxy resolving implementation. Bug: 881124 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I6b1f9f33e6452f64e193e57e5ce99526afc3f998 Reviewed-on: https://chromium-review.googlesource.com/1213935 Commit-Queue: Eric Roman <eroman@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/heads/master@{#590776} [modify] https://crrev.com/4102bdc56cf0febf57a275dff5391f6d8c9783b3/net/dns/BUILD.gn [modify] https://crrev.com/4102bdc56cf0febf57a275dff5391f6d8c9783b3/net/features.gni [modify] https://crrev.com/4102bdc56cf0febf57a275dff5391f6d8c9783b3/net/test/run_all_unittests.cc [modify] https://crrev.com/4102bdc56cf0febf57a275dff5391f6d8c9783b3/services/network/BUILD.gn [modify] https://crrev.com/4102bdc56cf0febf57a275dff5391f6d8c9783b3/services/network/network_context_unittest.cc [modify] https://crrev.com/4102bdc56cf0febf57a275dff5391f6d8c9783b3/services/network/url_request_context_builder_mojo.cc
,
Sep 13
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by kkhorimoto@chromium.org
, Sep 6Owner: eugene...@chromium.org
Status: Assigned (was: Untriaged)