I will take this. I added that class. Happy to get rid of it.

The extensions API (e.g. chrome.webRequest listener callbacks) are not yet hooked up with Network Service. +rockot@  who is working on that.
Unassigning myself. This is blocked on that work.

I think this work is orthogonal from webRequest.

Not entirely orthogonal. The current browser tests (https://cs.chromium.org/chromium/src/chrome/test/data/extensions/extension_throttle/ and ExtensionRequestLimitingThrottleBrowserTest) are written in a way that depends on webRequest. We can change those tests, but not sure if it's worth doing so.

To move those browser tests off the whitelist, we will need webRequest. 

xunjieli@ Are you going to start on this, or would you like me to take it?

If you are available to take this, by all means. Thank you!

The throttle is partially implemented, but the tests use the extension WebRequest API, so blocked on  issue 721414 .

This WIP is uploaded to https://crrev.com/c/914589, but is still far from working. Blocked on webRequest work in 721414.

@Chris: webRequest is mostly working with network service. What parts specifically is the WIP blocked on?

@jam: not sure. I'll go back to this next and see if it's still blocked on anything. thx.

Update: Resuming work on this.

The extensions throttle tests rely on webRequest, specifically they set both onCompleted and onErrorOccurred event listeners - neither of which are being called when an XHR request is dispatched.

It may be possible to refactor these tests to use XMLHttpRequest.onreadystatechange, but it's probably best to fix webRequest.

So I took a look at these tests and I know why you aren't seeing WebRequest events for the XHRs. It's pretty not-great.

As a preface, here are some explicit design decisions that were made:

  1) We send URLLoaderFactory interfaces to a renderer every time
     it navigates. The renderer uses them for subresource loads (e.g. XHRs)
     indefinitely (or until they break for some reason)

  2) When we create those URLLoaderFactory interfaces, we decide to
     proxy them through the browser if and only if there's at least
     one WebRequest listener installed (there are probably other cases too
     but they're irrelevant here).

  3) The above means that we accept races between e.g. extension
     installation (where typically a background page installs WebRequest
     event listeners) and page navigation. So pages navigated during
     or very briefly after an extension installation may need to be
     reloaded before they trigger WebRequest events. Cool, whatever.

Now unfortunately all of these tests issue XHRs from their background page, which is obviously going to be navigated before any WebRequest event listeners are installed, so none of those requests will ever get proxied through WebRequest.

In the case of these tests, the WebRequest listeners are actually installed by a navigated page rather than by the background page, but that is kind of irrelevant. Even if the background page installed them, it would be too late because the background page is already navigated and already has its non-proxied URLLoaderFactory pipes.

Solving this will require either:

  a) continuing to accept the limitations outlined above and
     restructuring the tests to e.g. add event listeners in the
     background page, wait for them to be installed browser-side,
     and then perform XHRs from a different navigated page, or

  b) changing some combination of (1) and (2) above.

Apologies for the late reply. I was out last week.
For background on ExtensionRequestLimitingThrottle, there is an extensions-dev@ thread:
https://groups.google.com/a/chromium.org/d/msg/extensions-dev/V7E6_66rhAs/hXKZk-fFyCIJ

Extensions team wanted to keep this protection feature when I checked with them in 2015. 

If we adopt (a) mentioned in #16, won't that cause a behavior change? If a misbehaving extensions performs XHRs on a navigated background page, it can issue requests that are not throttled.

Option (c) is to revert r6a47d0e298a4c3050d95505f5ee18b122fdc213b. net::URLRequestThrottlerManager which lives inside of network service should do the same thing. I didn't succeed in removing URLRequestThrottlerManager because cloud print was still using it. 

Reverting 6a47d0e298a4c3050d95505f5ee18b122fdc213b doesn't really help, alone - we need to only throttle certain requests, and the code that knows which ones was in the ChromeNetworkDelegate...Which doesn't work with the network service.

We're also going to need to figure out what to do with cloud print, of course.

I suggested to cmumford@ off-thread that we just push a new URLLoaderFactoryBundle to renderers when necessary (e.g. if a WebRequest listener is installed where none existed before).

This will still be fundamentally "racy" in the sense that something like:

  chrome.webRequest.foo.addListener(...);
  sendSomeXhr(...);

will not work as it does in the non-network service case today. That might be acceptable, though it will require some additional synchronization steps to be added to some browser tests.

Otherwise what we can do is have the renderer request a new URLLoaderFactoryBundle when it knows it's going to need one. Renderer-side impl of the WebRequest API can trip that code path any time a listener is added. There's still a bit of trickiness in making sure that request isn't sent until after the corresponding listener is registered in the browser, but it should be manageable.

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

commit 5ec1b0dbb9a7fd465fc07379615ca44dcf685d4a
Author: Chris Mumford <cmumford@chromium.org>
Date: Mon Jun 25 18:12:51 2018

Removed unused network logging from extensions throttle.

The extensions network throttle (ExtensionThrottleManager) used
to use the net::NetLog to log throttling activity. Logging was
only done if ExtensionThrottleManager::set_net_log was called,
but that was removed nearly four years ago in refs/heads/master@{#298862}
(https://codereview.chromium.org/631203003).

This change removes the logging implementation as these classes
will soon be moving to the render process.

Bug:  784576 
Change-Id: I1025dd6f3134d438e549165f5b7045b3ecf2dc61
Reviewed-on: https://chromium-review.googlesource.com/1113899
Commit-Queue: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570096}
[modify] https://crrev.com/5ec1b0dbb9a7fd465fc07379615ca44dcf685d4a/extensions/browser/extension_throttle_entry.cc
[modify] https://crrev.com/5ec1b0dbb9a7fd465fc07379615ca44dcf685d4a/extensions/browser/extension_throttle_entry.h
[modify] https://crrev.com/5ec1b0dbb9a7fd465fc07379615ca44dcf685d4a/extensions/browser/extension_throttle_manager.cc
[modify] https://crrev.com/5ec1b0dbb9a7fd465fc07379615ca44dcf685d4a/extensions/browser/extension_throttle_manager.h

Currently up for review: http://crrev.com/c/914589

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

commit 3f0eda935338567fd3c770fd24b44a3af46a4b3b
Author: Chris Mumford <cmumford@chromium.org>
Date: Mon Jul 23 14:51:17 2018

Implement Extension request throttling for network service.

This change introduces the ExtensionURLLoaderThrottle which
exists in the renderer and replaces ExtensionRequestLimitingThrottle
when the network service is both enabled and disabled.

Also re-enabled two extension throttle tests to speculatively
fix issue 836188. Hopefully now that the throttling is done in
the renderer they will no longer be flaky.

Bug:  784576 ,836188
Cq-Include-Trybots: luci.chromium.try:linux_mojo;master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib0c18f7b3fb17f4fbe47d79bf2f5bd7f1ebd6e06
Reviewed-on: https://chromium-review.googlesource.com/914589
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577168}
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/android_webview/renderer/aw_url_loader_throttle_provider.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/android_webview/renderer/aw_url_loader_throttle_provider.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/browser/chrome_content_browser_client.cc
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/browser/extensions/extension_url_loader_throttle_browsertest.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/common/chrome_switches.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/common/chrome_switches.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/renderer/url_loader_throttle_provider_impl.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/renderer/url_loader_throttle_provider_impl.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/test/BUILD.gn
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/test/data/extensions/extension_throttle/background.js
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/chrome/test/data/extensions/extension_throttle/test_request_eventually_throttled.html
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/chrome/test/data/extensions/extension_throttle/test_request_eventually_throttled.js
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/chrome/test/data/extensions/extension_throttle/test_request_not_throttled.html
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/chrome/test/data/extensions/extension_throttle/test_request_not_throttled.js
[add] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/test/data/extensions/extension_throttle/test_request_throttle.html
[add] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/chrome/test/data/extensions/extension_throttle/test_request_throttle.js
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/chrome/test/data/extensions/extension_throttle/test_request_throttled_on_first_try.html
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/chrome/test/data/extensions/extension_throttle/test_request_throttled_on_first_try.js
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/content/public/renderer/url_loader_throttle_provider.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/browser/BUILD.gn
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/extensions/browser/extension_request_limiting_throttle.cc
[delete] https://crrev.com/42f02d90384e3fb427379cd86880282c4dbcae5d/extensions/browser/extension_request_limiting_throttle.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/common/switches.cc
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/common/switches.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/BUILD.gn
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/DEPS
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_entry.cc
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_entry.h
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_entry_interface.h
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_manager.cc
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_manager.h
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_simulation_unittest.cc
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_test_support.cc
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_test_support.h
[rename] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_throttle_unittest.cc
[add] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_url_loader_throttle.cc
[add] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/extensions/renderer/extension_url_loader_throttle.h
[modify] https://crrev.com/3f0eda935338567fd3c770fd24b44a3af46a4b3b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter