New issue
Advanced search Search tips

Issue 717253 link

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

Blocked on:
issue 715697
Blocking:
issue 715695



Sign in to add a comment

Make it possible to use [Mojo] V8 ProxyResolver with URLRequestBuilder

Project Member Reported by mmenke@chromium.org, May 1 2017 
This is needed to let it create URLRequestContexts that use themselves for fetching PACs.

There are a couple complexities here:
* Cronet doesn't want or need V8, but uses URLRequestContextBuilder.
* URLRequestContextBuilder is in net/, but the v8 stuff is in net_with_v8 / net_browser_services.

The simplest solution I can think of is to subclass URLRequestContextBuilder, making a URLRequestContextBuilderWithV8 that uses in-process v8 or out-of-process Mojo as needed, but leaves all other work to URLRequestContextBuilder.  This will necessitate yet another net/ target, sadly.  On the plus side, it will simplify Chrome a bit, since it can just depend on the new target and net, instead of deciding between net_browser_services or new_with_v8.  It also means we can use the same code to set up IOThread's and ProfileIOData's ProxyServices (Though we may need to be able to have ProxyConfigService use a Mojo side channel, eventually).

Certainly happy to entertain other ideas.
Project Member

Comment 1 by bugdroid1@chromium.org, May 17 2017 

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

commit f4fc1f04fe1a0d835a508720fdd90841a8d1583b
Author: mmenke <mmenke@chromium.org>
Date: Wed May 17 13:30:27 2017

Allow use of Mojo/V8 ProxyResolvers with URLRequestContextBuilder.

This adds URLRequestContextBuilderV8, which can construct a
URLRequestContext using either type of ProxyResolver. When using such
a resolver, the ProxyService will fetch requests using the
URLRequestContext that was created with the builder.

Added a subclass instead of modifying URLRequestContextBuilder so it
can continue to be used without depending on V8.

BUG= 717253 

Review-Url: https://codereview.chromium.org/2881613002
Cr-Commit-Position: refs/heads/master@{#472439}

[modify] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/BUILD.gn
[modify] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/proxy/proxy_script_fetcher_impl_unittest.cc
[modify] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/proxy/proxy_service_mojo_unittest.cc
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/proxy/test_mojo_proxy_resolver_factory.cc
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/proxy/test_mojo_proxy_resolver_factory.h
[modify] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/test/embedded_test_server/http_request.cc
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/test/embedded_test_server/simple_connection_listener.cc
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/test/embedded_test_server/simple_connection_listener.h
[modify] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/url_request/url_request_context_builder.h
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/url_request/url_request_context_builder_v8.cc
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/url_request/url_request_context_builder_v8.h
[add] https://crrev.com/f4fc1f04fe1a0d835a508720fdd90841a8d1583b/net/url_request/url_request_context_builder_v8_unittest.cc

Comment 2 by mmenke@chromium.org, May 17 2017

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, May 19 2017 

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

commit b7060318b0cc8ccf7f4572104d08580919f1a733
Author: pasko <pasko@chromium.org>
Date: Fri May 19 14:41:11 2017

Revert of Allow use of Mojo/V8 ProxyResolvers with URLRequestContextBuilder. (patchset #10 id:280001 of https://codereview.chromium.org/2881613002/ )

Reason for revert:
 http://crbug.com/724471   V8InProcessShutdownWithHungRequest flaky on a cronet bot

Original issue's description:
> Allow use of Mojo/V8 ProxyResolvers with URLRequestContextBuilder.
>
> This adds URLRequestContextBuilderV8, which can construct a
> URLRequestContext using either type of ProxyResolver. When using such
> a resolver, the ProxyService will fetch requests using the
> URLRequestContext that was created with the builder.
>
> Added a subclass instead of modifying URLRequestContextBuilder so it
> can continue to be used without depending on V8.
>
> BUG= 717253 
>
> Review-Url: https://codereview.chromium.org/2881613002
> Cr-Commit-Position: refs/heads/master@{#472439}
> Committed: https://chromium.googlesource.com/chromium/src/+/f4fc1f04fe1a0d835a508720fdd90841a8d1583b

TBR=eroman@chromium.org,mmenke@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 717253 

Review-Url: https://codereview.chromium.org/2888043008
Cr-Commit-Position: refs/heads/master@{#473182}

[modify] https://crrev.com/b7060318b0cc8ccf7f4572104d08580919f1a733/net/BUILD.gn
[modify] https://crrev.com/b7060318b0cc8ccf7f4572104d08580919f1a733/net/proxy/proxy_script_fetcher_impl_unittest.cc
[modify] https://crrev.com/b7060318b0cc8ccf7f4572104d08580919f1a733/net/proxy/proxy_service_mojo_unittest.cc
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/proxy/test_mojo_proxy_resolver_factory.cc
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/proxy/test_mojo_proxy_resolver_factory.h
[modify] https://crrev.com/b7060318b0cc8ccf7f4572104d08580919f1a733/net/test/embedded_test_server/http_request.cc
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/test/embedded_test_server/simple_connection_listener.cc
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/test/embedded_test_server/simple_connection_listener.h
[modify] https://crrev.com/b7060318b0cc8ccf7f4572104d08580919f1a733/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/b7060318b0cc8ccf7f4572104d08580919f1a733/net/url_request/url_request_context_builder.h
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/url_request/url_request_context_builder_v8.cc
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/url_request/url_request_context_builder_v8.h
[delete] https://crrev.com/b68636d648c64b4312aa050486b74cccd6ceece4/net/url_request/url_request_context_builder_v8_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, May 23 2017 

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

commit 93be9ca690ac29ad965a7f7bf6f899cf05c0c582
Author: mmenke <mmenke@chromium.org>
Date: Tue May 23 16:29:13 2017

Allow use of Mojo/V8 ProxyResolvers with URLRequestContextBuilder.

This adds URLRequestContextBuilderV8, which can construct a
URLRequestContext using either type of ProxyResolver. When using such
a resolver, the ProxyService will fetch requests using the
URLRequestContext that was created with the builder.

Added a subclass instead of modifying URLRequestContextBuilder so it
can continue to be used without depending on V8.

This was originally landed in
https://codereview.chromium.org/2881613002/, and reverted in
https://codereview.chromium.org/2888043008/.  This version of the CL
fixes a test-only lifetime issue.

BUG= 717253 ,  724471 

Review-Url: https://codereview.chromium.org/2898073002
Cr-Commit-Position: refs/heads/master@{#473933}

[modify] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/BUILD.gn
[modify] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/proxy/proxy_script_fetcher_impl_unittest.cc
[modify] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/proxy/proxy_service_mojo_unittest.cc
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/proxy/test_mojo_proxy_resolver_factory.cc
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/proxy/test_mojo_proxy_resolver_factory.h
[modify] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/test/embedded_test_server/http_request.cc
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/test/embedded_test_server/simple_connection_listener.cc
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/test/embedded_test_server/simple_connection_listener.h
[modify] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/url_request/url_request_context_builder.cc
[modify] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/url_request/url_request_context_builder.h
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/url_request/url_request_context_builder_v8.cc
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/url_request/url_request_context_builder_v8.h
[add] https://crrev.com/93be9ca690ac29ad965a7f7bf6f899cf05c0c582/net/url_request/url_request_context_builder_v8_unittest.cc

Comment 5 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 6 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
►

Sign in to add a comment