Open Bug 1756595 Opened 11 months ago Updated 2 months ago

Implement "browsingContext.navigationStarted" event

Categories

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

task
Points:
3

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [webdriver:backlog])

Attachments

(2 files, 1 obsolete file)

We should be able to fully implement the event. But if it turns out that the navigation id is causing us problems we could move it to a follow-up bug.

Note that when using the WebProgress listener we have a reference to the nsIChannel via the request argument. This could be used to retrieve the channel id for the network request.

See Also: → 1756610
Depends on: 1730642
Points: --- → 8
Priority: -- → P3
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

Won't be working on this for now.

Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Whiteboard: [bidi-m3-mvp] → [webdriver:backlog]
Priority: P3 → P2
Points: 8 → 3

Hi Olli,

The browsingContext.navigationStarted event is supposed to be triggered at step 12 of https://html.spec.whatwg.org/#beginning-navigation
Do you know what corresponds to this in our implementation and if this is observable from chrome JS? For now I'm using a WebProgressListener and STATE_START, but not sure if that's a good match? If it is do you know if I should rather monitor this in the content process or in the parent process?

thanks!

Flags: needinfo?(smaug)

do you know if I should rather monitor this in the content process or in the parent process?

For the record I'm prototyping both, but I have issues with both...
For the parent process approach, I observe browsing-context-attached, and add a webprogress listener for each new browsing context. This seems to work fine in most cases, but there are some edge cases where I miss the STATE_START, for instance with nested iframes using data: uri, such as:

data:text/html,<meta charset=utf8>%0A%20%20%20%20%3Ch1%3ETop%20level%20header%3C%2Fh1%3E%0A%20%20%20%20%3Cp%3EThis%20is%20a%20paragraph.%3C%2Fp%3E%0A%20%20%20%20%3Ciframe%20src%3D%22data%3Atext%2Fhtml%2C%3Cmeta%20charset%3Dutf8%3E%0A%20%20%20%20%20%20%20%20%3Ch2%3EIframe%20header%3C%2Fh2%3E%0A%20%20%20%20%20%20%20%20%3Ciframe%20src%3D'data%3Atext%2Fhtml%2C%3Cmeta%20charset%3Dutf8%3E%3Ch2%3ENested%20Iframe%20header%3C%2Fh2%3E%0A%20%20%20%20%20%20%20%20'%3E%3C%2Fiframe%3E%0A%20%20%20%20%22%3E%3C%2Fiframe%3E

I can never observe STATE_START for the iframes.

If I switch to the content process, and create webprogress listeners there, I can observe all of them. The simplified approach is to have a JSWindowActor instantiated for each and every window, and start a webprogress listener as soon as possible. But in that case I get 2 STATE_START for each navigation. One very early, before the new JSWindowActor is instantiated, and another one much later. Both for the same target URI, but a key difference is that the request for the second one is linked to a valid channel (I can get its channel id etc...).

It feels like monitoring things from the content process is more reliable (at least I haven't found a scenario where I miss navigations yet), but I'm not sure how to deal with those duplicate notifications. I was suspecting that maybe calling addProgressListener once the JSWindowActor spawns forced the webprogress to emit a fake STATE_START? So I was updated my patch to only add a listener once for a given WebProgress, but I still have 2 different notifications for the same navigation. And beyond checking the request's channel, I could not find a good way to differentiate those 2 notifications.

Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [webdriver:backlog] → [webdriver:m5]

Julian, do you have some output with MOZ_LOG="BCWebProgress:5" that you could attach? I might help here.

hmm, isn't STATE_START a bit too late. Aren't beforeunload listener called already at that point. And in the spec 16.1 does beforeunload dispatching.

Flags: needinfo?(smaug)
Attachment #9276889 - Attachment is obsolete: true

(In reply to Olli Pettay [:smaug][bugs@pettay.fi] from comment #6)

hmm, isn't STATE_START a bit too late. Aren't beforeunload listener called already at that point. And in the spec 16.1 does beforeunload dispatching.

We discussed this a bit more on Elements, and indeed beforeunload is dispatched before we get a STATE_START notification.
There is no earlier webprogress event we could use, so we are missing notifications to really monitor the early stages of a navigation.
We can still implement some downgraded version of the event if we want, but it will not be emitted at the right time and will not work for pages using a beforeunload listener.

For the record, also thought about actually listening to beforeunload as a temporary workaround, but that doesn't seem like a good fit, because we need to emit the URL targeted by the navigation and this information is not accessible from the beforeunload event.

From here, we need to list all BiDi events which require a navigation id, and check with DOM peers what we need to change on the platform side to observe those.

We still need to figure out if this would be better observed in the parent process or in the content process, but we can come back to that question later.

Unassigning and moving back to backlog for now, this is more complex than expected. I added my current patches for reference (tests + parent process approach)

Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Priority: P1 → P2
Whiteboard: [webdriver:m5] → [webdriver:backlog]
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.