Clean-up "new pipeline" flow in constellation - match with correct event-loop

[gterzian]

I was just looking at

servo/components/constellation/constellation.rs

Line 906 in 447ae1e

fn new_pipeline(

trying to follow the flow of how we go about loading a cross-site iframe.

Couple of things to note:

1. The first if load_data.url.as_str() != "about:blank" seems wrong, since it does get_event_loop which will look at the host to determine a match re event-loop. I think in the case of "about:blank" iframes or auxiliary you would want to run it in the event-loop of the creator, which isnt' running at "about:blank"
2. if let Some(parent), here we always get the event-loop of the parent. That means a cross-site iframe will run in the same event-loop as the parent. We want to run it in it's own event-loop.
3. else if let Some(creator) This is only set in the case of an "about:blank" iframe, and it takes the event-loop of the creator. The logic is right, but I think it would be preempted by 1.

So to fix this we want:

1. To handle the "about:blank" case better. That's newly created iframe or auxiliary. Both should then either have parent or opener be some. So we should really just either take the event-loop of the parent, or of the opener.
2. In all other cases, we should use self.get_event_loop, which will either select a matching event-loop in the browsing context group of the opener, or the top-level, or create a new one.

The idea is that an auxiliary should initially be "about:blank", and share an event-loop with it's opener, and an iframe should share the event-loop of it's parent.

Then, when either are loaded to a different site, they would still remain part of the same browsing context group, but instead of automatically sharing the event-loop of the parent or opener, they would either share an existing event-loop, or get a new one, based on their host.

[gterzian]

Ok so what I wrote above is completely wrong, since the first arm is if load_data.url.as_str() != "about:blank", so that means "if it's not an about:blank load, then use get_event_loop", so my "fix" is basically the current state of affairs.

Leaving this open just in case, I find it somewhat hairy.

One thing that is perhaps still wrong is that we don't seem to deal with the "about:blank" newly created auxiliary in the right way, since it will fall through to the (None, None) case, we should instead give it the event-loop of it's opener, just like a newly created iframe gets the event-loop of it's creator.

Also, the (None, None) fall-through is a bit weird, because that means we spawn a new event-loop, yet since host is None, we don't seem to apply self.set_event_loop to it, so it becomes basically unregistered with the constellation?

I think this function can use a few more pair of eyes from time to time...

[gterzian]

Ok, learning something new, although I should have known it already:

Both "about:blank" new iframes and auxiliaries do not get a new event-loop assigned via new_pipeline in the constellation, instead they get added directly to the script-thread of the page that created them via a sync call to ScriptThread::process_attach_layout.

The message sent to the constellation is just to tell it: "hey they're a new pipeline in this existing event-loop".

So by the time we get to new_pipeline, it's not the case of a newly created iframe or auxiliary anymore, it's the case of those being navigated.

Then the question remains, why does reloading a page that contains a cross-site iframe, make script crash when the initial nested BC is created?

The reason is because ScriptThread::process_attach_layout is called in an event-loop that doesn't have a layout thread yet. But why is that?