Hook up OnPACScriptError when the network service is enabled. |
||||||||
Issue descriptionThe ChromeNetworkDelegate calls into the extensions system when there's a problem with a PAC script. We currently have no equivalent when the NetworkService is enabled - we presumably want to add a callback either to NetworkServiceClient or possibly to ProxyConfigPollerClient, renaming the interface, if we want to avoid bloating up NetworkServiceClient and adding another content API to some global object. Marking this as a Canary blocker, mostly because I assume it's useful in debugging proxy issues - something we probably don't want to break for our Canary users, if we want people to actually use Canary.
,
Jun 15 2018
Extensions triage - marking this as available for someone to grab.
,
Jun 16 2018
,
Jun 19 2018
,
Aug 14
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e1b1e21d199581797f57ea4f06d99cadefffb3cd commit e1b1e21d199581797f57ea4f06d99cadefffb3cd Author: Eric Roman <eroman@chromium.org> Date: Tue Aug 14 22:14:23 2018 Associate a bug with network service failing test ProxySettingsApiTest.ProxyEventsParseError. Bug: 851609 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I2f261550d21f80e4b2143c0253a3867f11091885 Reviewed-on: https://chromium-review.googlesource.com/1175054 Commit-Queue: Eric Roman <eroman@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#583054} [modify] https://crrev.com/e1b1e21d199581797f57ea4f06d99cadefffb3cd/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Aug 21
Matt mind brainstorming with me? If we exposed a method on NetworkServiceClient, we would then need a way to map back to the Profile (NetworkContext) that the error related to, since the extension pac script error listener has an affinity to the Profile. How would we do that mapping? In this regard re-purposing ProxyConfigPollerClient may offer a more direct translation, since that already has an affinity to the NetworkContext. Or yet another approach could be to have a new method on NetworkContext for setting the PAC error observer. Mainly just different ergonomics compared to passing it in the NetworkContextParams, however could introduce some raciness. WDYT?
,
Aug 21
So web-initiated requests have URLLoaders that know the process ID and request ID (Or, for main frames, 0 (or -1?) and some other ID that NetworkServiceClient already can use to fine the right tab). For those requests, can just use the pre-existing logic. For other requests...Erm...I don't think we have any way to get the right profile, currently. So I think we'll need to go with one of the other two options, since I'd rather not add yet more information about browser-side objects to the network service. I'd prefer adding a field to NetworkContextParams over ProxyConfigPollerClient. I'd love to be able to remove ProxyConfigPollerClient, and would rather not introduce to the proxy classes to the concept of extensions. That having been said, I could live with either approach.
,
Aug 21
Oh, and ProxyConfigPollerClient is passed as a NetworkContextParam, so I think neither option is racy.
,
Aug 21
Thanks for the advice Matt! I will try adding something to NetworkContextParams. Separately, I can remove ProxyConfigPollerClient as a follow-up, since there are other ways we can implement that.
,
Aug 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d576f34af2134e4205788845247d5d52063fee1f commit d576f34af2134e4205788845247d5d52063fee1f Author: Eric Roman <eroman@chromium.org> Date: Thu Aug 30 22:31:19 2018 Make chrome.proxy.onProxyError extension API work under the Network Service. This adds an optional "ProxyErrorClient" client to the NetworkContext which receives notifications of errors in the PAC script, as well as notifications of failed URL loads that may have been proxy related. Bug: 876568 , 851609 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I645ef91875135fcc81a7bd113442a0b65c661e65 Reviewed-on: https://chromium-review.googlesource.com/1192123 Reviewed-by: Matt Menke <mmenke@chromium.org> Reviewed-by: Will Harris <wfh@chromium.org> Commit-Queue: Eric Roman <eroman@chromium.org> Cr-Commit-Position: refs/heads/master@{#587812} [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/chrome/browser/net/chrome_extensions_network_delegate.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/chrome/browser/net/chrome_extensions_network_delegate.h [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/chrome/browser/net/chrome_network_delegate.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/chrome/browser/net/chrome_network_delegate.h [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/chrome/browser/net/proxy_config_monitor.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/chrome/browser/net/proxy_config_monitor.h [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/net/base/layered_network_delegate.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/net/base/layered_network_delegate.h [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/net/base/layered_network_delegate_unittest.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/services/network/network_context.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/services/network/network_context_unittest.cc [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/services/network/public/mojom/network_context.mojom [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/services/network/public/mojom/proxy_config_with_annotation.mojom [modify] https://crrev.com/d576f34af2134e4205788845247d5d52063fee1f/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Aug 30
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8ae9cb3ceebc17db40280a83c5599a2a27bc739 commit b8ae9cb3ceebc17db40280a83c5599a2a27bc739 Author: Jan Wilken Dörrie <jdoerrie@chromium.org> Date: Fri Aug 31 08:10:59 2018 Revert "Make chrome.proxy.onProxyError extension API work under the Network Service." This reverts commit d576f34af2134e4205788845247d5d52063fee1f. Reason for revert: Likely culprit of https://crbug.com/879491 . Original change's description: > Make chrome.proxy.onProxyError extension API work under the Network Service. > > This adds an optional "ProxyErrorClient" client to the NetworkContext which > receives notifications of errors in the PAC script, as well as notifications > of failed URL loads that may have been proxy related. > > Bug: 876568 , 851609 > Cq-Include-Trybots: luci.chromium.try:linux_mojo > Change-Id: I645ef91875135fcc81a7bd113442a0b65c661e65 > Reviewed-on: https://chromium-review.googlesource.com/1192123 > Reviewed-by: Matt Menke <mmenke@chromium.org> > Reviewed-by: Will Harris <wfh@chromium.org> > Commit-Queue: Eric Roman <eroman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#587812} TBR=eroman@chromium.org,mmenke@chromium.org,wfh@chromium.org Change-Id: I286c7f768b04b6e47f1836abc459a33c7b394498 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 876568 , 851609 Cq-Include-Trybots: luci.chromium.try:linux_mojo Reviewed-on: https://chromium-review.googlesource.com/1199043 Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org> Cr-Commit-Position: refs/heads/master@{#587966} [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/chrome/browser/net/chrome_extensions_network_delegate.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/chrome/browser/net/chrome_extensions_network_delegate.h [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/chrome/browser/net/chrome_network_delegate.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/chrome/browser/net/chrome_network_delegate.h [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/chrome/browser/net/proxy_config_monitor.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/chrome/browser/net/proxy_config_monitor.h [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/net/base/layered_network_delegate.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/net/base/layered_network_delegate.h [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/net/base/layered_network_delegate_unittest.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/services/network/network_context.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/services/network/network_context_unittest.cc [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/services/network/public/mojom/network_context.mojom [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/services/network/public/mojom/proxy_config_with_annotation.mojom [modify] https://crrev.com/b8ae9cb3ceebc17db40280a83c5599a2a27bc739/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Aug 31
,
Sep 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9fb9f49403a9a47597db318cd3646284f71b9d2 commit b9fb9f49403a9a47597db318cd3646284f71b9d2 Author: Eric Roman <eroman@chromium.org> Date: Sat Sep 01 03:37:46 2018 Make chrome.proxy.onProxyError extension API work under the Network Service. This adds an optional "ProxyErrorClient" client to the NetworkContext which receives notifications of errors in the PAC script, as well as notifications of failed URL loads that may have been proxy related. (Reland of d576f34af2134e4205788845247d5d52063fee1f). Bug: 876568 , 851609 , 879491 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I08987d658879913d94c9b13b6fdc1de2a32a6d54 Reviewed-on: https://chromium-review.googlesource.com/1200076 Commit-Queue: Eric Roman <eroman@chromium.org> Reviewed-by: Will Harris <wfh@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Cr-Commit-Position: refs/heads/master@{#588259} [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/chrome/browser/net/chrome_extensions_network_delegate.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/chrome/browser/net/chrome_extensions_network_delegate.h [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/chrome/browser/net/chrome_network_delegate.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/chrome/browser/net/chrome_network_delegate.h [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/chrome/browser/net/proxy_config_monitor.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/chrome/browser/net/proxy_config_monitor.h [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/net/base/layered_network_delegate.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/net/base/layered_network_delegate.h [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/net/base/layered_network_delegate_unittest.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/services/network/network_context.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/services/network/network_context_unittest.cc [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/services/network/public/mojom/network_context.mojom [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/services/network/public/mojom/proxy_config_with_annotation.mojom [modify] https://crrev.com/b9fb9f49403a9a47597db318cd3646284f71b9d2/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
Sep 4
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by eroman@chromium.org
, Jun 12 2018