Is there an obvious template for delaying a response to requests using URLLoaderInterceptor?

With the embedded test server, ControllableHttpResponse lets you delay responses.  I don't believe it would be too hard to do with URLLoaderInterceptor, either.  I'd tend to suggest using the embedded test server over URLLoaderInterceptor when reasonably doable, just because it avoids dependency on how the network stack is hooked up.

+1 to always preferring ControllableHttpResponse over URLLoaderInterceptor. I'll add a comment to the latter that it should only be used as a last resort.

Thanks for the responses! In this case, the browsertests already have a URLLoaderInterceptor installed that does the work of handling the requests (both requests to read a file off disk and requests that should fail):

https://cs.chromium.org/chromium/src/chrome/browser/net/dns_probe_browsertest.cc?q=DNSProbeBr&sq=package:chromium&l=546

Naively, it seems to me that the most straightforward path would be to insert a conditional into that method that causes it to bail out when the request should be delayed and then execute the response later when directed. From my reading of URLLoaderInterceptor, this could be accomplished as follows:

- If the request should be delayed, InterceptURLLoaderRequest() inserts the params into a vector of delayed requests and respond true.
- When it is desired to respond to all delayed requests, a wrapper method could simply go through the vector and call InterceptURLLoaderRequest() again on each of them.
- The one subtlety here is that some of these requests are failed requests, but InterceptURLLoaderRequest() will already have responded true for them and hence won't get the default URLLoaderInterceptor handling of them. I would break out the code for handling them here: https://cs.chromium.org/chromium/src/content/public/test/url_loader_interceptor.cc?q=url_loader_interce&sq=package:chromium&l=61 into a public URLLoaderInterceptor helper method that could be called here.

Does that make sense, or does it still seem better to use ControllableHttpResponse in this case? Naively, the latter doesn't seem to fit naturally into the existing setup of the dns probe tests to me.

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

commit d207aa5a8f7ce716b908d2be778a3dbe4f43a072
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Feb 09 00:56:54 2018

Better categorize some DnsProbe browsertests Network Service failures

Discovered in the course of producing
https://chromium-review.googlesource.com/c/chromium/src/+/887065.

Bug:  810329 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ibb57d149d5e69c09cb0c795a404d957d79bcce9f
Reviewed-on: https://chromium-review.googlesource.com/906733
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535600}
[modify] https://crrev.com/d207aa5a8f7ce716b908d2be778a3dbe4f43a072/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

jam@, please help triage/reassign.

Dropping priority since this is a test-only issue

punting it off Canary list.

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

commit 8e0417b1f432b5f42c7c43ec938946fc62e49660
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Jan 04 21:44:07 2019

Convert remaining DnsProbeBrowserTests to work with network service.

These depended on delaying responses. Also simplify DNSErrorPageTest a bit.

Bug:  810329 
Change-Id: Id8192455ce0f2bdbce16509695808cf6229cbd87
Reviewed-on: https://chromium-review.googlesource.com/c/1395145
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Doug Turner <dougt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620072}
[modify] https://crrev.com/8e0417b1f432b5f42c7c43ec938946fc62e49660/chrome/browser/net/dns_probe_browsertest.cc
[modify] https://crrev.com/8e0417b1f432b5f42c7c43ec938946fc62e49660/chrome/browser/net/errorpage_browsertest.cc
[modify] https://crrev.com/8e0417b1f432b5f42c7c43ec938946fc62e49660/chrome/browser/net/url_request_mock_util.cc
[modify] https://crrev.com/8e0417b1f432b5f42c7c43ec938946fc62e49660/chrome/browser/net/url_request_mock_util.h
[modify] https://crrev.com/8e0417b1f432b5f42c7c43ec938946fc62e49660/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

BTW to follow-up on earlier comments, the DNS tests depended on sending net error codes, which can't be simulated when using the EmbeddedTestServer, so I used the URLLoaderInterceptor.