1904892 - Add support for "initiatorType" and "destination" to network events

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

There is the request from Puppeteer folks to add a resourceType to the network events. It's just a proposal at the moment and the specification needs to be done. See https://github.com/w3c/webdriver-bidi/issues/704 for details.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Downgrading as optional for M12 because it's not clear when a spec for this field will be ready.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is a blocker for Cypress to make use of WebDriver BiDi instead of CDP. We probably have to get it added to our M13 milestone given that it causes breakage for generic stuff they do (see bug 1604723 comment 23).

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

https://github.com/w3c/webdriver-bidi/pull/809 has the actual additions that we will be able to merge soon. The two properties are now initiatorType and destination. But those are not supported for the network.beforeRequestSent event where we would need the initiator type which is to be followed in a different PR.

As such lets use this bug for the implementation of the initiatorType and destination properties.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

The WebDriver BiDi PR got merged. So this bug can be implemented now.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

When implementing this feature we should as well deprecate the Initiator.type field as proposed in this PR:
https://github.com/w3c/webdriver-bidi/pull/813/files

Comment 6

[Julian Descottes [:jdescottes]]

I gave this a quick shot today, got help from necko peers. Some observations:

• the destination can be computed from the loadInfo.internalContentPolicyType but to convert it to an actual fetch destination string, we need to reuse the mapping found at https://searchfox.org/mozilla-central/rev/b765e2890b0eb85b24f54bc7ff04491fd0704e30/dom/fetch/InternalRequest.cpp#261
• the initiatorType needs to be exposed to scripts, but after that it's usually set to empty string in the parent process. In content processes I get better results, but if possible it would be great to directly have the information in the parent. Otherwise we need to setup another listener mechanism to wait for information from content before emitting network events, which might introduce more delays to emit the events

Kershaw: I asked on the channel, but just to avoid forgetting about it: do you think we can propagate the initiatorType information from the content channel to the parent channel before "http-on-before-connect" is emitted?

Comment 7

[Valentin Gosu [:valentin]]

Yes, should be easy to do.

We need to HttpChannelOpenArgs, set the arg in
mozilla::net::HttpChannelChild::ContinueAsyncOpen, extract it in mozilla::net::HttpChannelParent::Init then in mozilla::net::HttpChannelParent::DoAsyncOpen we need to call
httpChannel->SetInitiatorType(aInitiatorType)

Comment 8

[Julian Descottes [:jdescottes]]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #7)

Yes, should be easy to do.

We need to HttpChannelOpenArgs, set the arg in
mozilla::net::HttpChannelChild::ContinueAsyncOpen, extract it in mozilla::net::HttpChannelParent::Init then in mozilla::net::HttpChannelParent::DoAsyncOpen we need to call
httpChannel->SetInitiatorType(aInitiatorType)

Thanks I will give it a shot!

Edit: works perfectly, thanks for the hints.

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1904892 - Set the initiatorType in HttpChannelParent and expose to scripts

This changeset synchronizes the initiatorType from the child channel to the parent channel in asyncOpen.
It also removes noscript from initiatorType in nsITimedChannel

Comment 10

[Julian Descottes [:jdescottes]]

Attachment: Bug 1904892 - [bidi] Add destination and initiatorType to network events RequestData

Add the new fields to request data for all event types

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: Bug 1904892 - [wdspec] Add wdspec tests for requestData initiatorType and destination

Depends on D229381

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1904892 - [wdspec] Update wdspec assert helpers to support initiatorType and destination

Depends on D229381

Update the basic assert helpers to check the new fields and change initiator to be optional
for beforeRequestSent

Comment 13

[Julian Descottes [:jdescottes]]

Attachment: Bug 1904892 - Expose fetch destination on nsILoadInfo

Depends on D229382

Expose a getter on nsILoadInfo to easily retrieve the fetch destination as a string
This avoids duplicating the mapping logic in JS for webdriver bidi

Comment 14

[Phabricator Automation]

Comment on attachment 9438368 [details]
Bug 1904892 - Expose fetch destination on nsILoadInfo

Revision D229384 was moved to bug 1932162. Setting attachment 9438368 [details] to obsolete.

Comment 15

[Phabricator Automation]

Comment on attachment 9438361 [details]
Bug 1904892 - Set the initiatorType in HttpChannelParent and expose to scripts

Revision D229378 was moved to bug 1932162. Setting attachment 9438361 [details] to obsolete.

Comment 16

[Julian Descottes [:jdescottes]]

Downgrading to 2p since part of the patches moved to Bug 1932162.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/117435d3eba6
[bidi] Add destination and initiatorType to network events RequestData r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/7a28805ee7e5
[wdspec] Update wdspec assert helpers to support initiatorType and destination r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/d4f7c1a6155f
[wdspec] Add wdspec tests for requestData initiatorType and destination r=webdriver-reviewers,whimboo

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49338 for changes under testing/web-platform/tests

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/117435d3eba6
https://hg.mozilla.org/mozilla-central/rev/7a28805ee7e5
https://hg.mozilla.org/mozilla-central/rev/d4f7c1a6155f

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #5)

When implementing this feature we should as well deprecate the Initiator.type field as proposed in this PR:
https://github.com/w3c/webdriver-bidi/pull/813/files

Julian it looks like we missed to to that. So we probably should get a new bug filed.

Comment 22

[Julian Descottes [:jdescottes]]

Right I need to file a new bug, but that was intentional, I wanted to do it separately.