Ensure documents drop when a pipeline exits

[gterzian]

minimalist companion/alternative to #24047

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #___ (GitHub issue number if applicable)

• There are tests for these changes OR
• These changes do not require tests because ___

---

This change is 

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/dom/globalscope.rs, components/script/dom/document.rs, components/script/dom/window.rs
• @KiChjang: components/script/dom/globalscope.rs, components/script/dom/document.rs, components/script/dom/window.rs

[highfive]

Warning

• These commits modify script code, but no tests are modified. Please consider adding a test!

[gterzian]

@bors-servo try=wpt (expected result: CRASH [expected OK] /html/cross-origin-embedder-policy/blob.https.html)

[bors-servo]

⌛ Trying commit d6b0aab with merge f32ed9d...

[bors-servo]

💔 Test failed - linux-rel-wpt

[gterzian]

{
    "status": "CRASH", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": null, 
    "test": "/html/cross-origin-embedder-policy/blob.https.html", 
    "line": 197679, 
    "action": "test_result", 
    "expected": "OK"
}

Same as with #24047

[gterzian]

@asajeffrey @jdm Ok so this is the most minimal change required to fix the leaking of window/global/document, and to be honest I have no idea why it has to do with Performance.

This does leave in place the risk that we unknowingly start leaking those again due to a future change to another MutNullableDom, or something else.

On the plus side it's a smaller change than the other PR, and I haven't found a way to make the other PR fit better into spec concepts.

[asajeffrey]

OK, this looks much much nicer than all the other approaches! It could be that if we didn't clear the performance list, then it ended up getting entries added to it but never removed, so if you quickly navigated away from the page it just sat there being a memory leak?

@bors-servo r+

[bors-servo]

📌 Commit 344684a has been approved by asajeffrey

[jdm]

It's cleaner, but I'm not thrilled about not actually understanding why it works.

[bors-servo]

⌛ Testing commit 344684a with merge 09d2159...

[gterzian]

@asajeffrey theory of performance entries being added "late" in the lifecycle of the pipeline, seems plausable, actually when debugging the other PR I did run into panics where performance.queue_entry seemed to be called after the pipeline was closed, and some of my changes(not the end results and I don't remember exactly the state of the code) would then result in a panic in such a situation.

I remember being surprised this could happen, since the message handler in the script-thread does a self.documents.find_window, which would seem to imply the queuing of a performance entry can only happen if the pipeline hasn't been closed yet.

@jdm If this could be reproduced, it could be a way to verify the "leak via performance entry that are never run" theory.

[gterzian]

Another possiblity, since performance tasks are potentially throttled by the task-queue, is that we get the following sequence:

1. Performance task is throttled,
2. Pipeline is closed.
3. Performance task is kept forever by the task-queue(it doesn't clear tasks when a pipeline is closed, in fact it sees those as tasks related to "not fully-active" documents, which is a leak in the case of a document that is not just "not fully-active" but actually related to a closed pipeline).

Although I looked into this in the other PR, and set it aside because it didn't actually seem to happen while we were still leaking windows.

[bors-servo]

💔 Test failed - status-taskcluster

[CYBAI]

@bors-servo retry

• #23290

[bors-servo]

💣 Failed to start rebuilding: Unknown error

[bors-servo]

⌛ Testing commit 344684a with merge 3ade7b6...

[asajeffrey]

The theory of the Performance takes being kept in the task queue seems plausible. Are there other instances of tasks being kept alive even though their document has been discarded?

[bors-servo]

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: asajeffrey
Pushing 3ade7b6 to master...

[kanaka]

Is this at a point where it would be worth me building and testing it?

[asajeffrey]

It's in master, if you can take a look that would be great!

[kanaka]

I hit a build regression on Linux I had to track down first: #24099 But it's building now and hopefully I'll have results shortly.

[kanaka]

I posted the results over in #23905. I assume that's still the right place until the fd growth issue is fully resolved?