STR:

• Install https://addons.mozilla.org/en-US/firefox/addon/ublock-origin/
• Visit https://www.cnn.com/

ER: See resources being blocked by "uBlock Origin".

@Raymond: how uBlock is blocking requests? Can you please point me to the right place in the code base?
It would be great if the Network panel marks requests that are blocked.

Honza

Can you please point me to the right place in the code base?

Well in uBO it's a couple of places in the same one file:

• At onBeforeRequest() time:
  • https://github.com/gorhill/uBlock/blob/master/src/js/traffic.js#L122; or
  • https://github.com/gorhill/uBlock/blob/master/src/js/traffic.js#L233; or
  • https://github.com/gorhill/uBlock/blob/master/src/js/traffic.js#L322; or
  • https://github.com/gorhill/uBlock/blob/master/src/js/traffic.js#L408
• At onHeadersReceived() time:
  • https://github.com/gorhill/uBlock/blob/master/src/js/traffic.js#L927

Network requests can also be blocked at onBeforeSendHeaders() time but uBO does not use this capability.

Just a guess, but in Firefox's extension framework side, I think this is the place which receives the result of whether an extension wants a network request blocked or not:

• toolkit/components/extensions/webrequest/WebRequest.jsm#779

And this would be where it would be effectively cancelled:

• /toolkit/components/extensions/webrequest/WebRequest.jsm#828

But as said, this is just a guess from searching in Firefox's code.

Thanks Raymond!

@Luca, is the above comment right about the place where a request is cancelled (in WebExtensions code base)?

Honza

@Valentin, it would be great if the Network panel in DevTools could indicate cancelled requests.

I am thinking about the following API the panel could utilize:

• New observer notification when a request is cancelled: http-on-cancel-request. The arguments passed into the callback could be something like as follows: onRequestCancelled(channel, topic, cancelCode) (similarly to the existing HTTP notifications).

• New requestCancelReason property. This could be similar to what we've done in bug 1493599. The bug introduces requestBlockingReason prop in nsILoadInfo [1] (used e.g. to explain CSP blocking reason). Not sure in which interface we could have that prop, but the Network panel could use it to display the reason to the user.

• Not sure about this one, but could we get name of the extension responsible for cancelling a request automatically?
  I am thinking about this place: https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#211

[1] https://searchfox.org/mozilla-central/rev/7556a400affa9eb99e522d2d17c40689fa23a729/netwerk/base/nsILoadInfo.idl#1113

WDYT?

Honza

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #4)

@Valentin, it would be great if the Network panel in DevTools could indicate cancelled requests.

• Not sure about this one, but could we get name of the extension responsible for cancelling a request automatically?
  I am thinking about this place: https://searchfox.org/mozilla-central/source/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#182

Couldn't this code send the onRequestCancelled notification? As opposed to sending it for any type of cancellation?

Then it could pass the extension name in the notification, or we could add requestCancelReason as a member to nsIChannel which would be available later, when we display the info in the netmonitor.

Thoughts?

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #4)

@Valentin, it would be great if the Network panel in DevTools could indicate cancelled requests.

I am thinking about the following API the panel could utilize:

• New observer notification when a request is cancelled: http-on-cancel-request. The arguments passed into the callback could be something like as follows: onRequestCancelled(channel, topic, cancelCode) (similarly to the existing HTTP notifications).

Nit: the cancelCode can be read from channel.status.

• New requestCancelReason property. This could be similar to what we've done in bug 1493599. The bug introduces requestBlockingReason prop in nsILoadInfo [1] (used e.g. to explain CSP blocking reason). Not sure in which interface we could have that prop, but the Network panel could use it to display the reason to the user.

I think this would make sense on nsIChannel, but callers of channel.cancel(...) would ALSO have to set the reason BEFORE whatever notification is made (kinda fragile)

maybe having a new method nsIChannel.cancelWithReason(code, reason) - what the reason would be, a string key or a number we list somewhere? number codes where fine for the blocking reason, as we have it under our control. but here I'm not sure what all the reason could be.

then, how should we behave when the channel is cancelled more than once?

• Not sure about this one, but could we get name of the extension responsible for cancelling a request automatically?
  I am thinking about this place: https://searchfox.org/mozilla-central/source/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#182

if we want this ONLY for webext then yes, that sounds like most reasonable to me.

(In reply to Honza Bambas (:mayhemer) from comment #6)

@mayhemer: thanks for the input!

• New observer notification when a request is cancelled: http-on-cancel-request. The arguments passed into the callback could be something like as follows: onRequestCancelled(channel, topic, cancelCode) (similarly to the existing HTTP notifications).

Nit: the cancelCode can be read from channel.status.

http-on-cancel-request should be useful for bug 1556451 too so, this sounds like good API to add

maybe having a new method nsIChannel.cancelWithReason(code, reason) - what the reason would be, a string key or a number we list somewhere? number codes where fine for the blocking reason, as we have it under our control. but here I'm not sure what all the reason could be.

then, how should we behave when the channel is cancelled more than once?

cancelWithReason sound good too, but just to note that the most important thing we need is the extension name that cancelled the request. It would be good enough for DevTools/Network panel if we have that name.

• Not sure about this one, but could we get name of the extension responsible for cancelling a request automatically?
  I am thinking about this place: https://searchfox.org/mozilla-central/source/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#182

if we want this ONLY for webext then yes, that sounds like most reasonable to me.

Yes, it would be for WebExtensions only.

Work on this bug should be synced with bug bug 1556451

Honza

Ah, I totally forgot about this bug. Note that the cancelWithReason would only be useful for the webext privileged back end. We could also not add any new API on nsIChannel and only set the blocking reason on nsILoadInfo (which is API we already have).

Tho, it would mean to change nsILoadInfo.requestBlockingReason to something more generic (a string) and have some internal set of strings C++ callers in gecko/necko can set.

But the question what that string would be still stands - should it be a localization tag or something semi-cryptic and finial to show in the UI directly?

(In reply to Honza Bambas (:mayhemer) from comment #8)

But the question what that string would be still stands - should it be a localization tag or something semi-cryptic and finial to show in the UI directly?

I discussed this bug with our product and the main thing we need is WebExtension name that cancelled the request (not so much a reason). So, instead of providing some cryptic codes/strings, let's provide only the name for now. The name is obviously a string.

The question is, does it make sense to implement cancelWithReason (or better name) and use it only to provide the name for now? The argument passed in would be a string (automatically calculated extension name).

WDYT?

Honza

OK. Would the webext back end fill this? I'm thinking if we could rather use the nsIWritablePropertyBag interface that our http channel already implements to carry this info, if that's enough for now.

(In reply to Honza Bambas (:mayhemer) from comment #10)

OK. Would the webext back end fill this?

Luca, could name of the extension (responsible for canceling an HTTP Request) be calculated within WebExtension framework?
Somewhere around this place:
https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/toolkit/components/extensions/webrequest/WebRequest.jsm#830
And consequently pass the name through nsIWritablePropertyBag interface on that channel?
(see also the previous comment)

Honza

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #3)

Thanks Raymond!

@Luca, is the above comment right about the place where a request is cancelled (in WebExtensions code base)?

Honza

HttpObserverManager's applyChanges looks like a good place, as it is where the request is actually cancelled based on the value returned from a blocking webextension's webRequest listener, and handlerResults result contains which was the extension that cancelled the request in opts.extension.

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #11)

Luca, could name of the extension (responsible for canceling an HTTP Request) be calculated within WebExtension framework?
Somewhere around this place:
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#182

And consequently pass the name through nsIWritablePropertyBag interface on that channel?
(see also the previous comment)

From what I recall the ChannelWrapper instance is shared between the extensions that have webRequest listeners that are intercepting the same request, and so I think that we may have to provide it as a parameter of the ChannelWrapper's Cancel method.

I'm adding a needinfo assigned to Shane, as he worked and knows more details about the webRequest API internals.

I think the most we could do is return the extension id that canceled the request (ie. via some new cancelWithReason). We could extend the webRequest API to allow an extension to provide a reason string as well, but for best compatibility we couldn't rely on that being available.

I would suggest something like:

channel.cancelWithReason(<string>source, <string>reason)

Source in our case would be the extension id. Reason can be a future addition that is some arbitrary string provided by the extension.

Oh, and no, do not expect the extension name to be passed around. You can retrieve that given the extension id.

While we're at it, should redirects also get tagged?

If you wanted to extend the api to include the reason you would do it in webrequest.jsm[1]

Regardless of that, you'll have to pass the extension id in webrequest[1] and then change channelwrapper[2], in addition to any lower level changes necessary.

[1] https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/toolkit/components/extensions/webrequest/WebRequest.jsm#830

[2] https://searchfox.org/mozilla-central/rev/8a990595ce6d5ed07ace2d4d5d86cc69aec90bde/toolkit/components/extensions/webrequest/ChannelWrapper.cpp#185

After Honza-to-Honza talk we decided that the best approach is to let the web-ext backend (specifically ChannelWrapper) provide the necessary API and communicate directly to DevTools with the provided reason and addon id. There are no platform (Necko) changes needed for this.

I can work on this as the last resort, if no one else can do it.

After reading a separate writeup about this, I think that the simplest solution might be adding a notifyObservers call when we cancel the request. Devtools can just listen for that and get the extension id and request id from the data.

https://searchfox.org/mozilla-central/rev/8a63fc190b39ed6951abb4aef4a56487a43962bc/toolkit/components/extensions/webrequest/WebRequest.jsm#969

Since Bug 1604618 is fixed we can also make progress with this report.

Instructions for anyone interested in implementing this new feature.

1. The backend needs to use the new API to see whether a request has been blocked by an extension.
   This should be done in _onComplete method within NetworkResponseListener
   https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/server/actors/network-monitor/network-response-listener.js#505

You might also see the a test file from the original bug
https://phabricator.services.mozilla.com/D57537#change-1UJmrASHGTxx

    let id;
    let reason;

    try {
      let properties = this.request.QueryInterface(Ci.nsIPropertyBag);
      id = properties.getProperty("cancelledByExtension");
      reason = this.request.loadInfo.requestBlockingReason;
    } catch (err) {
      // "cancelledByExtension" doesn't have to be available.
    }

We need to pass the data to addResponseContent

    this.httpActivity.owner.addResponseContent(response, {
      discardResponseBody: this.httpActivity.discardResponseBody,
      truncated: this.truncated,
      blockedReason: reason,
      blockingExtension: id,
    });

2. The addResponseContent needs to be updated to adopt the two new arguments
   https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/server/actors/network-monitor/network-event.js#504

Both arguments need to be emitted with all the other arguments.

3. Both argument types needs to be added to packet type definition
   https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/shared/specs/network-event.js#143-152

reason == number
extension id == string

4. The client side needs to read both arguments here
   https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/client/netmonitor/src/connector/firefox-data-provider.js#418-424

      case "responseContent":
        this.pushRequestToQueue(actor, {
          contentSize: networkInfo.response.bodySize,
          transferredSize: networkInfo.response.transferredSize,
          mimeType: networkInfo.response.content.mimeType,
          blockingExtension: packet.blockingExtension,
          blockedReason: packet.blockedReason,
        });

5. Rendering should happen in RequestListColumnTransferredSize
   https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/client/netmonitor/src/components/request-list/RequestListColumnTransferredSize.js#47

The two new props should be in this.props

const {
  blockedReason,
  blockingExtension,
} = this.props.item;

And the rendering logic needs to be updated to show the Extension ID
The column should render Blocked by <ext-id>

6. The UPDATED_TRANSFERRED_PROPS array needs to also have responseContent so, the column is rerendered when we send the packet from the backend.

7. Finally, UPDATE_PROPS array needs to also contain both new arguments "blockedReason" and "blockingExtension"
   https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/client/netmonitor/src/constants.js#159

8. This also needs a test. You can take an inspiration from:
   https://phabricator.services.mozilla.com/D57537#change-1UJmrASHGTxx

Honza

Harald, showing the blocking reason in Transferred column might result in unreadable label if the column is thin, see the attached a mockup indicating what could happen. There might be a tooltip, but not sure if it's enough.

I think that the Headers side panel could display a (red) notification message at the top, similarly how we indicate that a response has been truncated. WDYT?

Honza

The alternative would be "Blocked by Extension" and showing the full name in title and in the details page. Keeps the UI simpler as well.

hi can I work on this

Assigned to you, thanks for the help!
Honza

hi honza, I think _onComplete is not being called for blocked requests, am I missing something , i test by logging all requests.name in _onComplete and then searching the blocked request by ublock I cannot find the ones it blocks

@Pranav: sorry for the delay. Can you please provide your patch so, I can try it on my machine?

Honza

@Honza , I don't have the complete patch with me, but you can defiantly reproduce the issue I am facing
Steps
0) make sure you have installed https://addons.mozilla.org/en-US/firefox/addon/ublock-origin/ in browser

1. add following line here (https://searchfox.org/mozilla-central/rev/9e45d74b956be046e5021a746b0c8912f1c27318/devtools/server/actors/network-monitor/network-response-listener.js#504)

    let reason;

    try {
      reason = this.request.loadInfo.requestBlockingReason;
      console.log(reason)
    } catch (err) {
      // "cancelledByExtension" doesn't have to be available.
      console.log('error',err)
    }

2)open network tab in browser, then open browser console ,
3) go to https://edition.cnn.com/
4) you can only see 0 in browser console (that means non blocked request)
5) log the requests URLs by adding console.log(this.request.name) after console.log(reason)
6) open the ublock logger that is the 3rd option just below big blue power button
7) go to https://edition.cnn.com/ again
8)you can see all the blocked urls in the the ublock logger (they are highlighted by red color) try to search them in the browser console(we are logging requests urls in step 5) you cannot find any one

Side note ->
add following line here(https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#322)

if (subject.canceled) {
      console.log(subject.loadInfo.requestBlockingReason)
    }

you can see 6000 (which is code for blocked by extension) in browser console you can also log the request url by console.log(subject.name) and can match that requests from uBlock logger (they are blocked ones)..
.
.
.
.
so I think what is happening is network-response-listner only initiated when their is response is generated by a request but when an extension cancels a request no response is generated so network-response-listner never initialed for that request, ( https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#236) but because request is stopped _httpStopRequest is called https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#246

.
.
I hope I am able to describe the issue I am facing , If you find it difficult to follow please let me know
pranav

Thanks for the analysis!

Yes, you are right _onComplete is not the right place, sorry about that.

Look at the existing test for this API:
https://searchfox.org/mozilla-central/rev/b2ccce862ef38d0d150fcac2b597f7f20091a0c7/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_cancelWithReason.js#47-51

... it should happen in onStopRequest.

So, in our code, we should do it here:
https://searchfox.org/mozilla-central/rev/b2ccce862ef38d0d150fcac2b597f7f20091a0c7/devtools/server/actors/network-monitor/network-response-listener.js#365-373

Or, as you suggested _httpStopRequest should work too.

Honza

Hi jan,

after much further analysis I have few points that you should read

1. observeActivity callback of nsIHttpActivityObserver is never called for blocked resources , the reason can be that the request never leaves the browser, because of which
   a) NetworkResponseListener is never initialized for that request so I cannot use any method for that (i.e. _onComplete or onStopRequest)
   b) only solution is to utilize _httpStopRequest callback in network observer.
   c) network actor is also not initialised for that request (https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#764) so the owner of the httpActivity is still null for that request (https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#845) so I cannot use any methods accessible by NetworkEventActor i.e. addResponseContent

the only solution I can think of is to emit network-event-update:response-content from _httpStopRequest if request is blocked (utilizing requestBlockingReason)

does the solution looks good to you

(In reply to Pranav pandey from comment #26)

Hi jan,

after much further analysis I have few points that you should read

1. observeActivity callback of nsIHttpActivityObserver is never called for blocked resources , the reason can be that the request never leaves the browser, because of which
   a) NetworkResponseListener is never initialized for that request so I cannot use any method for that (i.e. _onComplete or onStopRequest)
   b) only solution is to utilize _httpStopRequest callback in network observer.
   c) network actor is also not initialised for that request (https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#764) so the owner of the httpActivity is still null for that request (https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#845) so I cannot use any methods accessible by NetworkEventActor i.e. addResponseContent

the only solution I can think of is to emit network-event-update:response-content from _httpStopRequest if request is blocked (utilizing requestBlockingReason)

does the solution looks good to you

I see, let's try _httpStopRequest

Also, don't hesitate to attach a patch (might be just work in progress) so, I can test on my machine and give you feedback.

Thanks!
Honza

Hi honza I am attaching a patch look at it, Its one way of doing it, It works presently, I still don't know the whole code but I made this work,

other way of doing it could be making a new event for network-monitor and emitting that client can listen to that can can take actions accordingly ..

let me know what you think
ps
pranav

Pranav, please look at the attached patch. It isn't finished, it needs cleanup and some polish but, should show you how to progress further.

Does that patch work for you?

Honza

Hi honza,

Thanks for the patch but it doesn't work for me. blockedReason in create net event !!! not getting this in log.

I am on macOS by the way
pranav

(In reply to Pranav pandey from comment #33)

Hi honza,
Thanks for the patch but it doesn't work for me. blockedReason in create net event !!! not getting this in log.

Do you see anything in the Transferred column for blocked resources?

Honza

I cannot see any blocked resources in my network panel

Pranav

@Harald, can you please try my patch (with uBlock) and check the Transferred column?
Perhaps my patch does work only on my machine from some reason?

Honza

@Bomsy tried the patch and it works for him

But it looks like it works for images only. In any case, try to use uBlock settings to block some images (see my screenshot, e.g. bigger than 100kb) and test again.

Thanks!
Honza

Luca, I am attaching a patch that should render name of an web-extension blocking requests in the Network panel (in the Transferred column). I am using API introduced in bug 1604618

The patch works nicely for images and I can see logs (see my patch) coming from

• onStopRequest
• _httpStopRequest
• _onComplete

However, for all the other resources, I see only log coming from onStopRequest.
Is that expected behavior? Why images are treated differently?

Honza

But it looks like it works for images only. In any case, try to use uBlock settings to block some images (see my screenshot, e.g. bigger than 100kb) and test again.

In case it matters, I just want to note that the feature seen in the screenshot causes an image to be blocked after the response headers were received for that image, so essentially there is still a connection made to the remote server. A filter like *$image however will block the network request before a connection to the remote server is made.

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #38)

Luca, I am attaching a patch that should render name of an web-extension blocking requests in the Network panel (in the Transferred column). I am using API introduced in bug 1604618

The patch works nicely for images and I can see logs (see my patch) coming from

• onStopRequest
• _httpStopRequest
• _onComplete

However, for all the other resources, I see only log coming from onStopRequest.
Is that expected behavior? Why images are treated differently?

I think that it is due to what Raymond mentioned in Comment 39.

I applied the patch locally and I configured it as shown in comment 37, and I noticed the same behavior you described in comment 38,
but then I added the custom filter suggested by Raymond in Comment 39 and with that custom filter all the blocked images were not listed in the DevTools Network panel anymore (and the messages logged by the patch were only coming from onStopRequest, as it was already happening for the other blocked requests).

Pranav, please read the previous comment, can you confirm. Does it work the same for you?

Honza

Yes I can reproduce that your patch is working when you block images as mentioned in comment 37, but when used filters like *$image your patch doesn't work,

I don't get any logged messages from onStopRequest(in network-response-listener) as mentioned in comment #40

Pranav

Just had a quick meeting with Luca and here are some comments:

1. As stated in comment #39 A filter like *$image however will block the network request before a connection to the remote server is made.
   This means that NetworResponseListener is never instantiated and registered for such request
   https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/devtools/server/actors/network-monitor/network-observer.js#923

And it explains why onStopRequest (callback on NetworResponseListener) is never called
https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/devtools/server/actors/network-monitor/network-response-listener.js#365

2. So, we only have _httpStopRequest callback that handles http-on-stop-request observer notificaiton
   https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/devtools/server/actors/network-monitor/network-observer.js#314

This notification is fired for all blocked requests (no matter when uBlock does it's blocking)

3. What we need to do is firing an event from _httpStopRequest informing the client (the Network panel) that there is blocked request.

An example of how this can be done is e.g. addResponseContent
https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/devtools/server/actors/network-monitor/network-response-listener.js#505

The method is implemented here:
https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/devtools/server/actors/network-monitor/network-event.js#504

It emits new event network-event-update:response-content

See also my comment here the === Implementation === part that explains how to send data from the backend to the client
https://bugzilla.mozilla.org/show_bug.cgi?id=1358038#c18

Pranav, let me know if this is enough information to make a progress.

Honza

Hi Honza,

I do understand what you are excepting,

but I don't think that we can use network-event-update:response-content its spec says its of type networkEventUpdate with updateType responseContent if I emit that then in client side https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/connector/firefox-data-provider.js#397 should be actorID of network-event actor which doesn't exits if we follow along the code then as far as I can understand it is not going to work, (here also actorID is not defined https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/connector/firefox-data-provider.js#528, also that request is not not in request array that we maintain in our redux store)

best way I can see is to create a new network for that with networkEvent(that is also right event as clent is concerned as it will see that request for the first time)

That is what I have done in my patch can you please look at it again an give me suggestions for that,

If I am wrong above sorry for that,

Pranav

Yes, you are right, we need "networkEvent".

See my attached patch, it calls _createNetworkEvent in _httpStopRequest, but the event doesn't make it to the client from some reason.

It's weird, when it doesn't filter out other requests it works well. Comment out the marked line in the patch (look for my TODO comment there).

You can use the Browser Toolbox to debug where the event is lost.

Add BP in the client here:
https://searchfox.org/mozilla-central/rev/070a000dd49aac4a26147e137efcd91a728d13b8/devtools/client/netmonitor/src/connector/firefox-data-provider.js#89

And hit it by executing normal BP, see callstack and create another BP deep in the code. Then execute blocked request and hit your second BP. It would be great to find the culprit!

Honza

hi honzz,

what do you mean by not make it to the client from server I can see those blocked requests perfectly in the network panel when I use your patch..

see that api.segment.io/t shows up in network pannel in attached screenshot

pranav

(In reply to Pranav pandey from comment #46)

what do you mean by not make it to the client from server I can see those blocked requests perfectly in the network panel when I use your patch..

For me the this._createNetworkEvent(subject, { blockedReason: reason }) doesn't work properly. I don't see the blocked request in the Network panel.

Follow these STR:

0. Install uBlock from: https://addons.mozilla.org/en-US/android/addon/ublock-origin/
1. uBlock configuration: Create a filter *.php in My Filters in uBlock settings
2. Load http://janodvarko.cz/firebug/tests/601/Issue601.htm
3. Press the button on the page. It generates process.php request that should be blocked but, it doesn't appear in the Network panel.

Can you see the issue on your machine (in the Network panel)?

Honza

Hi honza
I can see the blocked resources in my network panel, as you can see in the attached screen-shot

I don't use any specific configuration (the are default) I just applied your patch

pranav

Hubert, could you please install my patch from comment #45 and try my STR from comment #47. Can you see the blocked requests in the Network panel?

Honza

Ok will test in a bit.

Honza,
I don't see the request in the Network panel.

Hi, Honza
I don't think the event get lost,
STR

1. Load http://janodvarko.cz/firebug/tests/601/Issue601.htm and cofigure your uBlock to block php requests
2. open browser toolbox > network panel
3. open network panel in browser
4. hit post button you can see the blocked request in browser toolBox > network panel but not in browser > network-panel
   .
   .
   .
   Ok so this is happening because matchRequest(channel, this.filters, true) return false for every request blocked or not, it is sending true for browser toolbox > network-pannel here (https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor/network-observer.js#68)

you can also test this. close any browser toolbox and browser console and see your terminal for logs.(make sure only browser is open)

1. hit post
2. you can only see _httpStopRequest BAIL OUT getting logged

can you reproduce this ,
in conclusion event is not getting lost but the this._createNetworkEvent(subject, { blockedReason: reason }) you can see fired is in browser toolbox itself

pranav

Luca, we are facing one issue here. When we handle the http-on-stop-request event for blocked request - the request doesn't seem to have nsILoadContext associated.

Here is how we get the context:
https://searchfox.org/mozilla-central/rev/7d0c94a0e9a9fe1f83553f49b10128567d21709d/devtools/shared/webconsole/network-helper.js#248

We need the context to get the top frame (or parent window where the request is executed):
https://searchfox.org/mozilla-central/rev/7d0c94a0e9a9fe1f83553f49b10128567d21709d/devtools/shared/webconsole/network-helper.js#217

The top frame/parent window is used to figure out whether the request belongs to the debugged tab (where the Toolbox is opened) and whether we should display it in the Network panel.

here is the matchRequest function implemented:
https://searchfox.org/mozilla-central/rev/7d0c94a0e9a9fe1f83553f49b10128567d21709d/devtools/server/actors/network-monitor/network-observer.js#102

Do you see a way how to solve this or does this need a platform change?

Honza

Hi Jan,
I took a look and replied to the needinfo in Comment 54.

This screenshot shows what I see in the network panel after I applied the small change described in comment 54.

Thanks Luca for looking into this!

The problem is that the check if (!matchRequest(channel, this.filters)) { is there to filter out requests from different tabs. So, we need to keep it there.

Alex, I am out of ideas here. Would you have some tips how to fix it?

You can read

• comment #47 with STRs
• comment #53 with more info

Honza

I've been chatting about this with Shane, and we were wondering if the information that the ChannelWrapper could already provide would be enough to properly filter out what we need before creating the devtools NetworkEvent.

here is what ChannelWrapper provides that seems related to this:

• browserElement: https://searchfox.org/mozilla-central/rev/278046367dab878316f60f0bd7f740cf73f3c447/dom/chrome-webidl/ChannelWrapper.webidl#326-335
• frameAncestors: https://searchfox.org/mozilla-central/rev/278046367dab878316f60f0bd7f740cf73f3c447/dom/chrome-webidl/ChannelWrapper.webidl#337-348

I finally got some time to look at this again.

@Luca, I am not sure how the props (browserElement, frameAncestors) Could help us. Can you provide more info?

It still feels like the problem is that at the time when http-on-stop-request is fired we can't get the nsILoadContext from request.notificationCallbacks and thus we can't get the topFrameElement;

Querying request.notificationCallbacks for nsILoadContext throws NS_NOINTERFACE

So, how the ChannelWrapper can help here?

Honza

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #58)

I finally got some time to look at this again.

@Luca, I am not sure how the props (browserElement, frameAncestors) Could help us. Can you provide more info?

From what I was recalling about this, we are currently bailing out on the blocked request because matchRequest(channel, filters) is returning false, because NetworkHelper.getTopFrameForRequest(channel) is not returning a topFrame for the request and so we are unable to compare topFrame.outerWindowID with filters.outerWindowID.

It still feels like the problem is that at the time when http-on-stop-request is fired we can't get the nsILoadContext from request.notificationCallbacks and thus we can't get the topFrameElement;

Querying request.notificationCallbacks for nsILoadContext throws NS_NOINTERFACE

I may be wrong, but I think that querying request.notificationCallbacks for nsILoadContext is throwing in this case because (as I was describing in comment 54) request.notificationCallbacks isn't yet set when we are checking that request.

So, how the ChannelWrapper can help here?

While discussing about this with Shane, he did asked me if we could be using ChannelWrapper's frameAncestors to look for the topFrame outerWindowID related to that request.

But I took another look to this (because I was worried that I my have forget some relevant detail, on missed to notice something useful the last time I looked into it)...

... and I noticed that channel.loadInfo does actually have a channel.loadInfo.topOuterWindowID property which seems to also provide the topFrame outerWindowID (see also its description from the .idl file here) that we need to decide if this request is relevant for the current tab or not.

I tried locally to apply to matchRequest(channel, filters) the following small change (on top of the rest of the changes applied by attachment 9132033 [details] [diff] [review]) and it seems to be also enough to make the blocked top level request to be listed in the network panel:

diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network-observer.js
--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -110,6 +110,13 @@ function matchRequest(channel, filters) 
         // outerWindowID getter from browser.js (non-remote <xul:browser>) may
         // throw when closing a tab while resources are still loading.
       }
+    } else if (
+      channel.loadInfo &&
+      channel.loadInfo.topOuterWindowID == filters.outerWindowID
+    ) {
+      // If we couldn't get the top frame outerWindowID from the loadContext,
+      // look to the channel.loadInfo.topOuterWindowID instead.
+      return true;
     }
   }

Honza, does this check looks reasonable to you? is there any reason why we cannot use channel.loadInfo.topOuterWindowID for that check?
(at least as a fallback if we are unable to retrieve it from the loadContext as we usually do in the current implementation)

Woot, this is great help Luca, thanks!

Honza, does this check looks reasonable to you?
Yes, it works for me and I don't see any reason why not to use it.

Pranav, I think you can try to improve my patch...

Some comments:

• You can use my patch as the starting point
• All logs (for testing) should be removed from the patch
• We need to use the code from Luca (in the previous comment)
• The blockingExtension needs to be also passed to the client from _httpStopRequest
• Code clean up is needed in the patch.

Does that make sense?

Honza

So, I am facing one more bug related to duplicity entries in the Network panel.

STR:

1. Apply the attached patch https://phabricator.services.mozilla.com/D65452 (diff 4)
2. Open the Network panel and load http://janodvarko.cz/firebug/tests/601/Issue601.htm
3. Use uBlock to Block media elements larger than 5 KB (see the Settings tab)
4. Reload the page. You should see favicon.ico entry twice => BUG, see also my screenshot

This is because "http-on-stop-request" is sent after ACTIVITY_SUBTYPE_TRANSACTION_CLOSE.

We remove the favicon.ico request from the openRequests map when ACTIVITY_SUBTYPE_TRANSACTION_CLOSE is handled [1] and re-add it into the map when "http-on-stop-request" is fired [2]. The patch is also calling createNetworkEvent from within the "http-on-stop-request" to sent NETWORK_EVENT which creates the duplicity in the Network panel UI.

Note that we have to call createNetworkEvent from the _httpStopRequest function since for requests blocked by uBLock My Filters e.g. *.php - we only get "http-on-stop-request" event. You can use my test page http://janodvarko.cz/firebug/tests/601/Issue601.htm to test this scenario too.

[1] https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/devtools/server/actors/network-monitor/network-observer.js#1059
[2] https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/devtools/server/actors/network-monitor/network-observer.js#324

You can also read my comment here https://bugzilla.mozilla.org/show_bug.cgi?id=1628162#c6

Luca does this make sense? Any tips?
Perhaps we need to ignore duplicated NETWORK_EVENTS on the client side?

Honza

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #61)

Created attachment 9139742 [details]
image.png

So, I am facing one more bug related to duplicity entries in the Network panel.

STR:

1. Apply the attached patch https://phabricator.services.mozilla.com/D65452 (diff 4)
2. Open the Network panel and load http://janodvarko.cz/firebug/tests/601/Issue601.htm
3. Use uBlock to Block media elements larger than 5 KB (see the Settings tab)
4. Reload the page. You should see favicon.ico entry twice => BUG, see also my screenshot

This is because "http-on-stop-request" is sent after ACTIVITY_SUBTYPE_TRANSACTION_CLOSE.

We remove the favicon.ico request from the openRequests map when ACTIVITY_SUBTYPE_TRANSACTION_CLOSE is handled [1] and re-add it into the map when "http-on-stop-request" is fired [2]. The patch is also calling createNetworkEvent from within the "http-on-stop-request" to sent NETWORK_EVENT which creates the duplicity in the Network panel UI.

Note that we have to call createNetworkEvent from the _httpStopRequest function since for requests blocked by uBLock My Filters e.g. *.php - we only get "http-on-stop-request" event. You can use my test page http://janodvarko.cz/firebug/tests/601/Issue601.htm to test this scenario too.

[1] https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/devtools/server/actors/network-monitor/network-observer.js#1059
[2] https://searchfox.org/mozilla-central/rev/9120151ddb35f2d4c37bfe613a54a4f10a9a3dc5/devtools/server/actors/network-monitor/network-observer.js#324

You can also read my comment here https://bugzilla.mozilla.org/show_bug.cgi?id=1628162#c6

Luca does this make sense?

Sorry Jan, I really wanted to get back to this sooner, but after taking an actual look (which took me more than I wanted ;-)).

Your analysis about what is happening sounds right to me too, but I'd like to add some details that may be worth a mention:

It looks (based on both the screenshot you attached on comment 61 and a quick look I took on the property that we are actually received by two calls to _createNetworkEvent) that:

• when we do receive ACTIVITY_SUBTYPE_TRANSACTION_CLOSE we do not have yet all the details related to which extension has actually blocked the request (blockedReason and blockingExtension are both undefined in that call)
• but then we do have those additional details when we receive http-on-stop-request.
• The channelId is the same one in both cases and so (as you also mention in your comment) I do expect that the channel is the same one and we get two network events instead of just one because when we receive ACTIVITY_SUBTYPE_TRANSACTION_CLOSE we do remove the channel from the openRequests map, and so _findActivityObject does return null and we create a new network event.

I see that you opened Bug 1632748 and that it is blocking multiple issues related to blocked requests that are not being properly displayed in the network panel, and I don't know enough about the underlying components to really know if ACTIVITY_SUBTYPE_TRANSACTION_CLOSE is expected to be the last notification and if that would solve all those issues properly, I'll keep an eye on it to see what Nhi thinks about Bug 1632748.

In the meantime I thought it was worth a mention the fact that when ACTIVITY_SUBTYPE_TRANSACTION_CLOSE is currently fired, WebRequest internals didn't have the chance to set on the channel the properties that allow us to know which extension has actually blocked the HTTP request, so that we can keep it mind.

Any tips?
Perhaps we need to ignore duplicated NETWORK_EVENTS on the client side?

An idea that comes to my mind (but I'm not sure if it would be able to also properly fix the other issues currently blocked by Bug 1632748, I just checked that it would fix this issue though, see the screenshot attached) is to track the closed transactions in a WeakMap, and then tweak _findActivityObject to also check if the request is in the closedTransactions WeakMap when it isn't in the openRequests Map.

This is a (rough ;-) patch to implement this strategy:

diff --git a/devtools/server/actors/network-monitor/network-observer.js b/devtools/server/actors/network-monitor/network-observer.js
--- a/devtools/server/actors/network-monitor/network-observer.js
+++ b/devtools/server/actors/network-monitor/network-observer.js
@@ -153,6 +153,7 @@ function NetworkObserver(filters, owner)
 
   this.openRequests = new Map();
   this.openResponses = new Map();
+  this.closedTransactions = new WeakMap();
 
   this.blockedURLs = [];
 
@@ -223,6 +224,13 @@ NetworkObserver.prototype = {
   openRequests: null,
 
   /**
+   * WeakMap for the transaction closed (TODO: put a better comment if
+   * this could be a reasonable workaround/solution for the underlying
+   * issue)
+   */
+  closedTransactions: null,
+
+  /**
    * Object that holds response headers coming from this._httpResponseExaminer.
    */
   openResponses: null,
@@ -694,6 +702,7 @@ NetworkObserver.prototype = {
       ? new Date(Math.round(timestamp / 1000))
       : new Date()
     ).toISOString();
+
     event.fromCache = fromCache;
     event.fromServiceWorker = fromServiceWorker;
     // Only consider channels classified as level-1 to be trackers if our preferences
@@ -849,7 +858,16 @@ NetworkObserver.prototype = {
    *        The HTTP activity object, or null if it is not found.
    */
   _findActivityObject: function(channel) {
-    return this.openRequests.get(channel) || null;
+    const openRequest = this.openRequests.get(channel);
+    if (openRequest) {
+      return openRequest;
+    }
+    // Look if there is a channel in the recent closed transactions.
+    const closedRequest = this.closedTransactions.get(channel);
+    if (closedRequest) {
+      return closedRequest;
+    }
+    return null;
   },
 
   /**
@@ -1100,6 +1118,7 @@ NetworkObserver.prototype = {
       );
     }
     this.openRequests.delete(httpActivity.channel);
+    this.closedTransactions.set(httpActivity.channel, httpActivity);
   },
 
   _getBlockedTiming: function(timings) {

Let me know what you think about this solution based on the additional closedTransactions WeakMap.

This is great help, Luca!

I agree, let's see what Nhi/Networking team think about Bug 1632748 and if ACTIVITY_SUBTYPE_TRANSACTION_CLOSE can't be the last fired event we can use & build on top of your patch.

Thanks for your analysis, I appreciate that!

Honza

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #62)

Let me know what you think about this solution based on the additional closedTransactions WeakMap.

Luca, what if we turned the existing this.openRequests map to WeakMap and didn't remove requests from it in _onTransactionClose. This way we wouldn't need the additional this.closedTransactions map...

Honza

(In reply to Jan Honza Odvarko [:Honza] (always need-info? me) from comment #64)

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #62)

Let me know what you think about this solution based on the additional closedTransactions WeakMap.

Luca, what if we turned the existing this.openRequests map to WeakMap and didn't remove requests from it in _onTransactionClose. This way we wouldn't need the additional this.closedTransactions map...

Honza

Yeah, I haven't tried but that sounds pretty reasonable to me:
based on a quick look on searchfox it does not seems that we ever iterate over the this.openRequests Map and so it looks that we may just replace it with a WeakMap and just leave it in the WeakMap (where it should then go away automatically once the request is completely gone).

sgtm.

Taking this bug per offline chat with Pranav. This is not good first bug and the amount of related reports is rather growing at this point.
I am also increasing priority to P2

Honza

New patch should solve few related issues:

• Showing related add-on for blocked resources (this bug)
• Showing requests rejected by CORS (Bug 1445637)
• Solving unreliable ACTIVITY_SUBTYPE_TRANSACTION_CLOSE (not being fired as the last event), by using WeakMap/WeakSet. This could solve Bug 1632748, perhaps even Bug 1556451
• Set of bugs related to multiple selected requests like Bug 1635516 and Bug 1628162 (edit: doesn't seem to be fixed by the patch)

Try is green:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a52f9ef58a1f40479e5900b4e085e99f3ef4ef93

Honza

https://hg.mozilla.org/mozilla-central/rev/a6237c79c2da

Is this expected that the network panel shows blocked ressources with uBlock Origin but doesn't show any blocked ressources with Adblock Plus? Maybe Adblock Plus does something different to block ressources. I would really like to understand the difference. Thanks! :)

(In reply to Sören Hentzschel from comment #72)

Is this expected that the network panel shows blocked ressources with uBlock Origin but doesn't show any blocked ressources with Adblock Plus? Maybe Adblock Plus does something different to block ressources. I would really like to understand the difference. Thanks! :)

Filed a bug 1638476.

Verified with Adblock, AddBlock Ultimate and uBlock Origin; on Windows 10, macOS 10.15.5, Ubuntu 18.04 using Firefox 78.0b8.

Updated the Network request columns section of the Request List article, including:

• an updated screenshot of the columns menu
• removal of the "Cause" column description
• addition of the "blocked" icon for the "Status" column
• addition of blocked requests for the "Transferred" column
• misc copy-edits