1794763 - Use CancelWithReason() in ChannelWrapper

Status: NEW

(WebExtensions :: Request Handling, task, P3)

Description

[Kershaw Chang [:kershaw]]

See https://phabricator.services.mozilla.com/D156070#5120821
It'd be good to log the reason when a channel is cancelled by an extension.

Comment 1

[Rob Wu [:robwu]]

From the comment on the Phabricator revision:

IMO it would be really nice to have CancelWithReason take both a constant and a string so it could also handle the loadinfo blocking reason like this: https://searchfox.org/mozilla-central/rev/1d861901b6682ef2e191b9f7683eb267bdd131e2/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#220-222

That code is:

      loadInfo->SetRequestBlockingReason(aReason);
    }
    rv = chan->Cancel(nsresult(aResult));

What are you hoping to achieve by replacing Cancel + SetRequestBlockingReason with CancelWithReason?

Would that goal be unique to ChannelWrapper, or would it also apply to other requestBlockingReason providers? From a quick search, it seems that other uses of requestBlockingReason (e.g. NS_SetRequestBlockingReason) do not immediately cancel the request. So if cancelWithReason's signature were to be modified to take a constant in addition to the existing reason string, then the internal usage of that would be limited.

The primary consumer of ChannelWrapper, WebRequest.jsm sets BLOCKING_REASON_EXTENSION_WEBREQUEST. The only internal use of that reason constant is https://searchfox.org/mozilla-central/rev/aa1bb4f0ca8bfda4b117d1befca47b72d5dd6d5d/browser/base/content/nsContextMenu.js#1815. If we were to cancel with a fixed reason string rather than a constant, then this code could be adopted by using a string comparison instead of an integer comparison. Is that the goal that you had in mind?

Along with setting BLOCKING_REASON_EXTENSION_WEBREQUEST, cancelledByExtension is set, which is parsed and used in a bunch of places:

• notifying of extension-blocked downloads - nsContextMenu.js source.
  • FYI: This is also the only non-test usage of BLOCKING_REASON_EXTENSION_WEBREQUEST.
• bug 1555057: showing extension in network tab of devtools (source 1, source 2).
• logging in http channel (nsHttpChannel source).

Unrelated to cancel, but the code pattern used to associate an extension with a request is also used for redirects, with redirectedByExtension.

In theory, we could replace the requestBlockingReason = BLOCKING_REASON_EXTENSION_WEBREQUEST and extensionId usage by a concatenation of everything in a reason string, and parsing that string in the few places that uses it. That's not an improvement IMO.