Overhaul document unloading/destruction/aborting #9907
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kalenikaliaksandr
added a commit
to kalenikaliaksandr/serenity
that referenced
this pull request
Nov 3, 2023
See whatwg/html#9907 As far as I understand these changes do not fix bugs in our implementation but allows us to remove some hacks and be aligned with the spec.
kalenikaliaksandr
approved these changes
Nov 3, 2023
kalenikaliaksandr
added a commit
to kalenikaliaksandr/serenity
that referenced
this pull request
Nov 3, 2023
See whatwg/html#9907 As far as I understand these changes do not fix bugs in our implementation but allows us to remove some hacks and be aligned with the spec.
domenic
force-pushed
the
recursive-unload-destroy-etc
branch
from
465ce88 to
2e397f0
Compare
As noted in #9148, the way in which destroy calls abort, and unload calls destroy, is not sound when most of these algorithms act on the entire tree. Instead, we need to split these algorithms into single-document base variants, which call each other, and whole-tree "document and its descendant" variants, which proceed carefully. The whole-tree variants in particular need to take care with how they queue tasks. We cannot fire events (like unload for unloading, or readystatechange for aborting) inside of inactive documents, so we need to delay making a document inactive until its children have been processed. (The related issue #9869 around reloading is not yet solved, however, since that is a special case where the same session history entry is reused and thus making documents inactive is harder to avoid.) Closes #9148.
domenic
force-pushed
the
recursive-unload-destroy-etc
branch
from
2e397f0 to
a49f99e
Compare
domfarolino
reviewed
Dec 8, 2023
|
Edit: Fixed the build. |
domfarolino
approved these changes
Jan 10, 2024
|
Yeah, so the hiding of "in parallel" in the algorithm header can indeed be confusing. I'll open another issue to discuss that, since it's a potential general problem. |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As noted in #9148, the way in which destroy calls abort, and unload calls destroy, is not sound when most of these algorithms act on the entire tree. Instead, we need to split these algorithms into single-document base variants, which call each other, and whole-tree "document and its descendant" variants, which proceed carefully.
The whole-tree variants in particular need to take care with how they queue tasks. We cannot fire events (like unload for unloading, or readystatechange for aborting) inside of inactive documents, so we need to delay making a document inactive until its children have been processed. (The related issue #9869 around reloading is not yet solved, however, since that is a special case where the same session history entry is reused and thus making documents inactive is harder to avoid.)
Closes #9148. Closes #9904 while we're in the area.
@kalenikaliaksandr could you take a look? Also CC @ADKaster.
/browsing-the-web.html ( diff )
/document-lifecycle.html ( diff )
/document-sequences.html ( diff )