Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Look into moving the layout DOM wrappers back into the script crate #29691

Closed
mrobinson opened this issue May 1, 2023 · 1 comment · Fixed by #29716
Closed

Look into moving the layout DOM wrappers back into the script crate #29691

mrobinson opened this issue May 1, 2023 · 1 comment · Fixed by #29716
Assignees
@mrobinson

Comments

@mrobinson
Copy link
Member

The layout / style DOM wrappers are in the layout_thread crate because they currently need to know how to initialize and clear the LayoutData which is stored on the DOM nodes themselves. This has a few issues:

  1. Details of the script thread leak into layout whereas the script_layout_interface could provide a safe interface between them.
  2. The wrappers must be duplicated in both Layout 2020 and Layout 2013.
  3. Because of the tricky design, there is a proliferation of traits on these wrappers which makes the code difficult to understand.

An idea here is to make all of the wrappers generics on LayoutData.
@mrobinson
Copy link
Member Author

Context:

  1. The wrappers were in layout.
  2. The wrappers were moved into the script crate in Move ServoLayoutNode and related structs to script. #11754.
  3. The wrappers were moved into layout in script: Move the layout_wrapper outside of script. #17744 and subsequently copied into layout_2020.

This would be moving them once again to script.

mrobinson added a commit to mrobinson/servo that referenced this issue May 5, 2023
@mrobinson
Eliminate duplicate Layout DOM wrappers
f68549b 
There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and
one for Layout 2020. As part of cleaning up and simplifying the
wrappers, this change parameterizes them on the specific layout data
they contain. This allows them to be shared again. In addition, various
small cleanups are included.

Fixes servo#29691.
@mrobinson mrobinson self-assigned this May 5, 2023
@mrobinson mrobinson mentioned this issue May 5, 2023
Merged
4 tasks
bors-servo added a commit that referenced this issue May 5, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=<try>
6f09225 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue May 5, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=<try>
232d1aa 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue May 5, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=<try>
73474a0 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue May 7, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=jdm
470fc2f 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue May 7, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=jdm
29a2cae 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue May 8, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=jdm
733f90e 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue May 8, 2023
@bors-servo
Auto merge of #29716 - mrobinson:generic-dom-wrapper, r=jdm
7cfdf87 
Eliminate duplicate Layout DOM wrappers

There are duplicate sets of Layout DOM wrappers: one for Layout 2013 and one for Layout 2020. As part of cleaning up and simplifying the wrappers, this change parameterizes them on the specific layout data they contain. This allows them to be shared again. In addition, various small cleanups are included.

Fixes #29691.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29691
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mrobinson mrobinson

Labels
None yet
Projects
Code Health
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Eliminate duplicate Layout DOM wrappers mrobinson/servo
1 participant
@mrobinson