Closed Bug 1637363 Opened 5 years ago Closed 5 years ago

Add subframe support when returning frameId in Network events

Categories

(Remote Protocol :: CDP, defect, P1)

defect

Tracking

(firefox78 fixed)

RESOLVED FIXED
Firefox 78
Tracking Status
firefox78 --- fixed

People

(Reporter: impossibus, Assigned: whimboo)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [puppeteer-beta-reserve])

Attachments

(1 file, 1 obsolete file)

Currently we only look at the top frame when computing the frameId

Not sure yet how important that actually is, but given that network events are used a lot I would rather see it fixed. It would also help me with bug 1637640 to actually have the correct frame ids reported. As such I'm taking that one.

I had a quick look into that and it seems to actually quite simple. The loadContext for HTTP requests and responses in content processes is actually a CanonicalBrowsingContext, and as such has an id property. That can perfectly be used here to filter out requests and responses not related to the page or sub frame itself, eg. favicons as requested by the browser.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [puppeteer-beta-reserve]
Blocks: 1637640

After further investigation I noticed that for all network requests related to resources (style sheets, images, etc) the loadContext is of type nsILoadContext and not CanonicalBrowsingContext. As such no browsing context id is available, which could be used as frameId.

Honza, is there an easy way to get the browsing context of the document request for a resource request?

Flags: needinfo?(honzab.moz)

As it looks like we can use wrappedChannel.windowId to setup a mapping to the frameId. Each document request has the windowId and frameId set, and for resource requests the windowId is equal to the document requests. It works fine for me for top-level and sub browsing contexts.

I'll leave needinfo for Honza in case there is something more elegant.

Blocks: 1638941
Blocks: 1638704
Blocks: 1638508

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

As it looks like we can use wrappedChannel.windowId to setup a mapping to the frameId. Each document request has the windowId and frameId set, and for resource requests the windowId is equal to the document requests. It works fine for me for top-level and sub browsing contexts.

I'll leave needinfo for Honza in case there is something more elegant.

Sorry for late answer, I'm not that much an expert to window ids, to be honest. On the child channel for http we collect toplevel content and outer window ids here: https://searchfox.org/mozilla-central/rev/ea7f70dac1c5fd18400f6d2a92679777d4b21492/netwerk/protocol/http/HttpChannelChild.cpp#2645-2661. Not sure if that is of any help.

But I believe that what you have found is what you need.

Flags: needinfo?(honzab.moz)

Thanks for your reply Honza. As it looks like I will indeed leave what I have right now. The code you pointed at is using the innerWindowId, which changes for each and every navigation request. But in our case we make use of the unique browsing context id.

Blocks: 1638196

On Ubuntu 18.04 tests are failing more often for debug builds
due to not received events. Bumping up the value here seems to
help.

Depends on D76835

No longer blocks: 1638196
Depends on: 1638196

Comment on attachment 9151976 [details]
Bug 1637363 - [remote] Bump default timeout for history.record() to 2s due to timeouts of debug builds.

Revision D76992 was moved to bug 1638196. Setting attachment 9151976 [details] to obsolete.

Attachment #9151976 - Attachment is obsolete: true
No longer blocks: 1638941
No longer blocks: 1638704
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bf7719c227fb [remote] Use frameId of the related browsing context for Network events. r=remote-protocol-reviewers,maja_zf
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 78
Component: CDP: Network → CDP
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: