Closed Bug 1805176 Opened 2 years ago Closed 4 months ago

Support network events using data URLs

Categories

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

task
Points:
5

Tracking

(firefox129 fixed)

RESOLVED FIXED
129 Branch
Tracking Status
firefox129 --- fixed

People

(Reporter: whimboo, Assigned: Sasha)

References

(Blocks 5 open bugs)

Details

(Whiteboard: [webdriver:m11][wptsync upstream][webdriver:relnote])

Attachments

(3 files)

Similar to DevTools NetMonitor (bug 1000540) but for our WebDriver BiDi implementation. When loading pages via data: or file:/// URLs we should support all the available network events.

Whiteboard: [webdriver:backlog]

As decided in one of our last meetings we are going to split off the file protocol case into its own bug.

Summary: Support network events for navigations to data and file URLs → Support network events for navigations to data URLs

Hi Valentin,

Do you know if requests to data: URIs are handled at all in Necko? Do we create a channel for those etc...
I think it's not the case but wanted to check.

Thanks!

Flags: needinfo?(moz.valentin)

Yes, we have nsDataHandler::NewChannel that creates data channels, and nsDataChannel::OpenContentStream that actually loads that channel into a stream.
We could have an approach similar to the one for file channels, where we send out a notification when one of these is loaded.

If we're only interested in navigations, we might also be able to use DocumentLoadListener::Open to send out the notification.

Flags: needinfo?(moz.valentin)

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

Yes, we have nsDataHandler::NewChannel that creates data channels, and nsDataChannel::OpenContentStream that actually loads that channel into a stream.
We could have an approach similar to the one for file channels, where we send out a notification when one of these is loaded.

If we're only interested in navigations, we might also be able to use DocumentLoadListener::Open to send out the notification.

Thanks! An observer notification would be great, and I confirm we want to monitor all data channels. I will file a bug for Necko.

Depends on: 1870293
Assignee: nobody → dotoole
Assignee: dotoole → nobody
Summary: Support network events for navigations to data URLs → Support network events using data URLs

There are 7 (maybe more) Puppeteer unit tests that are failing based on this missing feature. Lets re-evaluate.

Alex, would you consider this as a blocker?

Flags: needinfo?(alexrudenko)
Whiteboard: [webdriver:backlog] → [webdriver:backlog][webdriver:triage]

We would consider it for addition to the M11 milestone given that a lot of tests might use data URLs.

Points: --- → 5
Whiteboard: [webdriver:backlog][webdriver:triage] → [webdriver:m11]

If it's possible on your side to support these events, it would be great. In Chromium, we only support the events but not interception for data URLs. If it is too difficult to support it, we could exclude it from the scope and document the difference.

Flags: needinfo?(alexrudenko)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

Julian, would adding support for the network events also add support for interception, or would that need additional work (beside bug 1898158)? If that's the case we should get a bug filed.

I suspect interception will not work out of the box for data URIs. I can't imagine there's a strong case for supporting it either, but maybe that's something we should discuss on the spec.

Whiteboard: [webdriver:m11] → [webdriver:m11:blocked]
Depends on: 972821
Assignee: jdescottes → aborovova
Blocks: 972821
No longer depends on: 972821

Hi Valentin,
I've started working on this bug and notification works fine, but I'm not able to get much information from the channel, maybe some of it doesn't make much sense compare to other types, but we probably need at least headers, and it looks like visitRequestHeaders and visitOriginalResponseHeaders are not available. Do you know if there is maybe another way to get this info, or should/could we just hardcode it?

Similar case with getting the final information about the response (e.g. status code), for regular HTTP requests we listen to stream events but data channel doesn't seem to implement nsITraceableChannel interface, I guess that's why setNewListener method doesn't exist in this case. Again, I guess we could hardcode 200, but maybe there is another way to retrieve this information dynamically?

Thanks!

Flags: needinfo?(valentin.gosu)

FWIW, hardcoding some values seems fine spec-wise considering the way the response is built in scheme-fetch for data URI https://fetch.spec.whatwg.org/#concept-scheme-fetch

Return a new response whose status message is OK, header list is « (Content-Type, mimeType) », and body is dataURLStruct’s body as a body.

Seems like we can assume status 200 statusMessage OK and a single header Content-Type? Using a simple fetch await fetch('data:text/plain,42') I also seem to get a Content-Length header.

Agreed. Data channels don't have headers or status codes, so hardcoding is the way to go.

Flags: needinfo?(valentin.gosu)
Whiteboard: [webdriver:m11:blocked] → [webdriver:m11]

Hi Valentin,

I have another question. :)
I've noticed that in some case (e.g. data URL as src for images on the page, as background images or in fetch requests inside the page) the data-channel-opened notification is sent only in the content process, where at the moment we listen to all the network related notifications in the parent process. Is it maybe possible to bubble them up?

Thank you in advance

Flags: needinfo?(valentin.gosu)

Looks like Valentin is already off. Kershaw, could you maybe help here?

(In reply to Alexandra Borovova [:Sasha] from comment #13)

Hi Valentin,

I have another question. :)
I've noticed that in some case (e.g. data URL as src for images on the page, as background images or in fetch requests inside the page) the data-channel-opened notification is sent only in the content process, where at the moment we listen to all the network related notifications in the parent process. Is it maybe possible to bubble them up?

Thank you in advance

Flags: needinfo?(kershaw)
Blocks: 1903060

(In reply to Alexandra Borovova [:Sasha] from comment #13)

Hi Valentin,

I have another question. :)
I've noticed that in some case (e.g. data URL as src for images on the page, as background images or in fetch requests inside the page) the data-channel-opened notification is sent only in the content process, where at the moment we listen to all the network related notifications in the parent process. Is it maybe possible to bubble them up?

Thank you in advance

Yeah, I think that's possible, but it requires some amount of work.
I assume we need to forward the data URL to parent process and open it again.

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(kershaw)
Depends on: 1903212

(In reply to Kershaw Chang [:kershaw] from comment #18)

(In reply to Alexandra Borovova [:Sasha] from comment #13)

Hi Valentin,

I have another question. :)
I've noticed that in some case (e.g. data URL as src for images on the page, as background images or in fetch requests inside the page) the data-channel-opened notification is sent only in the content process, where at the moment we listen to all the network related notifications in the parent process. Is it maybe possible to bubble them up?

Thank you in advance

Yeah, I think that's possible, but it requires some amount of work.
I assume we need to forward the data URL to parent process and open it again.

Thanks! I've updated the bug 1903060, which I created for this issue. Let me know if you have any problems or concerns with it.

Blocks: 1903807
Pushed by aborovova@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3e5c06b95aa0 Add support for data URLs in the shared network classes. r=devtools-reviewers,jdescottes,bomsy https://hg.mozilla.org/integration/autoland/rev/10d9917879bf [bidi] Add support for network events using data URLs. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/ccef2524223b [wdspec] Add tests for network events with data URLs. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46861 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m11] → [webdriver:m11], [wptsync upstream]
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 129 Branch
Upstream PR merged by moz-wptsync-bot
Whiteboard: [webdriver:m11], [wptsync upstream] → [webdriver:m11][wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: