Skip to content

script: Switch GetClientRect and dirty_all_nodes to use unrooted iterators - #46144

Merged
TimvdLippe merged 4 commits into
servo:mainfrom
Narfinger:client-rect-and-similar
Jul 16, 2026
Merged

script: Switch GetClientRect and dirty_all_nodes to use unrooted iterators#46144
TimvdLippe merged 4 commits into
servo:mainfrom
Narfinger:client-rect-and-similar

Conversation

@Narfinger

@Narfinger Narfinger commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Both of these methods can be called a lot on some websites.
This uses the unrooted iterators variety for their loop.

Includes #46143

Testing: NoGC only needs compilation to be tested and DocumentFragment::ChildElementCount is hopefully tested by WPT.

@Narfinger
Narfinger force-pushed the client-rect-and-similar branch from 073ae17 to 2179225 Compare July 2, 2026 08:46
@Narfinger Narfinger changed the title script: Client rect use unrooted iterators script: Switch GetClientRect and dirty_all_nodes to use unrooted iterators Jul 2, 2026
@Narfinger Narfinger added the T-linux-wpt Do a try run of the WPT label Jul 2, 2026
@github-actions github-actions Bot removed the T-linux-wpt Do a try run of the WPT label Jul 2, 2026
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

🔨 Triggering try run (#28577875220) for Linux (WPT)

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

⚠️ Try run (#28577875220) failed!

@Narfinger
Narfinger force-pushed the client-rect-and-similar branch from 2179225 to 03be1ca Compare July 2, 2026 13:26
@Narfinger
Narfinger marked this pull request as ready for review July 7, 2026 06:07
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 7, 2026
@Narfinger
Narfinger force-pushed the client-rect-and-similar branch 2 times, most recently from 0246ab1 to 9912d72 Compare July 7, 2026 11:23
Comment thread components/script/dom/range/range.rs Outdated
Comment thread components/script/dom/range/range.rs
}

pub(crate) fn search(mut self) -> Option<DomRoot<Element>> {
pub(crate) fn search(mut self, no_gc: &NoGC) -> Option<DomRoot<Element>> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow-up we should investigate if search and maybe_search_in_containing_focus_navigation_scope could avoid returning a rooted Element.

@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 9, 2026
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
@Narfinger
Narfinger force-pushed the client-rect-and-similar branch from 18b2af2 to dc643a6 Compare July 10, 2026 09:34
Comment thread components/script/dom/document/documentfragment.rs Outdated
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
@Narfinger
Narfinger requested a review from jdm July 14, 2026 09:31
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jul 14, 2026
@jdm
jdm added this pull request to the merge queue Jul 14, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 14, 2026
@yezhizhen
yezhizhen removed this pull request from the merge queue due to a manual request Jul 14, 2026
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 14, 2026
@yezhizhen
yezhizhen added this pull request to the merge queue Jul 14, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 14, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 14, 2026
@jdm
jdm added this pull request to the merge queue Jul 14, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 14, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 14, 2026
Comment thread Cargo.lock Outdated
Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 15, 2026
@Narfinger

Copy link
Copy Markdown
Contributor Author

Done.

@jdm
jdm enabled auto-merge July 15, 2026 07:19
@jdm
jdm added this pull request to the merge queue Jul 15, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 15, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 15, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 15, 2026
@Narfinger

Copy link
Copy Markdown
Contributor Author

Windows CI just flaky?

@TimvdLippe
TimvdLippe added this pull request to the merge queue Jul 16, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 16, 2026
Merged via the queue into servo:main with commit e5e8d36 Jul 16, 2026
40 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 16, 2026
@Narfinger
Narfinger deleted the client-rect-and-similar branch July 16, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants