How can NetworkDelegate error out requests after OnBeforeURLRequest()?

[wdzierz...@opera.com]

Hi,

I'm looking for a way to enable a NetworkDelegate to error out certain URLRequests. My use case requires it to happen after OnBeforeURLRequest().

Encouraged by the docs, which say "See OnBeforeURLRequest for return value description.", I'm trying to simply return an error from OnBeforeStartTransaction(). This doesn't seem to work very well. Depending on the type of request, I'm ending up in some bad places.

For example, an SdchDictionaryFetcher notices we're trying to start a nested fetch loop:
[1068:5812:0902/095833:245125484:FATAL:sdch_dictionary_fetcher.cc(162)] Check failed: !in_loop_.
Backtrace:
base::debug::StackTrace::StackTrace
logging::LogMessage::~LogMessage() Line 533
net::SdchDictionaryFetcher::OnResponseStarted() Line 167
net::URLRequest::NotifyResponseStarted() Line 825
net::URLRequestJob::NotifyStartError() Line 626
net::URLRequestHttpJob::MaybeStartTransactionInternal() Line 581
net::URLRequestHttpJob::StartTransaction() Line 555
net::URLRequestHttpJob::DoStartTransaction() Line 820
net::URLRequestHttpJob::AddCookieHeaderAndStart() Line 804
net::URLRequestHttpJob::Start() Line 400
net::URLRequest::StartJob() Line 665
net::URLRequest::BeforeRequestComplete() Line 610
net::URLRequest::Start() Line 533
net::SdchDictionaryFetcher::DoSendRequest() Line 293
net::SdchDictionaryFetcher::DoLoop() Line 241
net::SdchDictionaryFetcher::ScheduleInternal() Line 221
net::SdchDictionaryFetcher::ScheduleReload() Line 136
net::SdchOwner::SchedulePersistedDictionaryLoads() Line 732
net::SdchOwner::OnPrefStorageInitializationComplete() Line 632
chrome_browser_net::SdchOwnerPrefStorage::OnInitializationCompleted() Line 110
JsonPrefStore::FinalizeFileRead() Line 440
JsonPrefStore::OnFileRead() Line 391

In another example, a content::ResourceLoader effectively self-destructs while executing StartRequestInternal(). The call stack below illustrates what happens right before attempting the delegate_->DidStartRequest(this) call in StartRequestInternal().
content::ResourceDispatcherHostImpl::RemovePendingLoader() Line 2027
content::ResourceDispatcherHostImpl::RemovePendingRequest() Line 2022
content::ResourceDispatcherHostImpl::DidFinishLoading() Line 1063
content::ResourceLoader::CallDidFinishLoading() Line 698
content::ResourceLoader::ResponseCompleted() Line 692
content::ResourceLoader::OnResponseStarted() Line 353
net::URLRequest::NotifyResponseStarted() Line 825
net::URLRequestJob::NotifyStartError() Line 626
net::URLRequestHttpJob::MaybeStartTransactionInternal() Line 603
net::URLRequestHttpJob::StartTransaction() Line 577
net::URLRequestHttpJob::DoStartTransaction() Line 845
net::URLRequestHttpJob::AddCookieHeaderAndStart() Line 829
net::URLRequestHttpJob::Start() Line 402
net::URLRequest::StartJob() Line 665
net::URLRequest::BeforeRequestComplete() Line 610
net::URLRequest::Start() Line 533
content::ResourceLoader::StartRequestInternal() Line 499
content::ResourceLoader::StartRequest() Line 189
content::ResourceDispatcherHostImpl::StartLoading() Line 2376
content::ResourceDispatcherHostImpl::BeginRequestInternal() Line 2362
content::ResourceDispatcherHostImpl::BeginRequest() Line 1568
content::ResourceDispatcherHostImpl::OnRequestResource() Line 1171

Is there something else I need to do in addition to returning an error from OnBeforeStartTransaction()? I'm on Chromium version 54.0.2832.2.

Thanks,
-- Wojtek

[Matt Menke]

This looks like a bug in URLRequestHttpJob - it calls URLRequestJob::NotifyStartError synchronously, which the URLRequest doesn't allow.  We could make URLRequestJob::NotifyStartError itself support being called synchronously, but then there's URLRequestJob::NotifyHeadersComplete, which has the same issue.  The naive solution would just be a PostTask, but not sure we want to delay every single request by an extra PostTask.

--
You received this message because you are subscribed to the Google Groups "net-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to net-dev+unsubscribe@chromium.org.
To post to this group, send email to net...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/01e51f99-a7cd-4824-83cb-7dfc337e37e6%40chromium.org.

[Randy Smith]

[+Helen]

Matt: Pointer to the relevant docs/comments on the non-sync restriction on NotifyStartError?  I a) can't find them, and b) have a vague memory that NotifyStartError was created to handle a corner case in the filter refactor that was using synchronous notification.

-- Randy

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/CAEK7mvrPZGij0VeP5HGhX4JwWPnHBLbv%3DTf680fLXAtWWefXNQ%40mail.gmail.com.

[Matt Menke]

Docs don't mention it.  NotifyStartError has been around for at least 4 years, and has never handled being called synchronously on Start, to the extent of my knowledge.  In fact, a lot of subclasses used to use NotifyDone on start failure, possibly because it mostly worked, and bypassed the sync restriction.  Or maybe just because they didn't know what they were supposed to be calling.

This is one of the many parts of the API that I've long wanted to clean up.

[Helen Li]

Matt is right. As a part of refactoring (CL), I had to change some of URLRequestJob subclasses to not invoke NotifyStartError synchronously. The restriction is not new. It's easy to use the API incorrectly, since there is no documentation. Not sure how extensive an refactor is needed to fix NotifyStartError and NotifyHeadersComplete.

[wdzierz...@opera.com]

Thanks for the analysis!

Unfortunately, I'm not familiar with the network code enough to fix this in a reasonable time frame myself. Filed https://bugs.chromium.org/p/chromium/issues/detail?id=644234.

-- Wojtek

> To unsubscribe from this group and stop receiving emails from it, send an email to net-dev+u...@chromium.org.

>
> To post to this group, send email to net...@chromium.org.
>
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/net-dev/01e51f99-a7cd-4824-83cb-7dfc337e37e6%40chromium.org.
>
>
>
>
>
>
>
> --
>
> You received this message because you are subscribed to the Google Groups "net-dev" group.
>

> To unsubscribe from this group and stop receiving emails from it, send an email to net-dev+u...@chromium.org.