Skip to content

add CanGc as argument to Validatable.validity_state#40155

Merged
yezhizhen merged 1 commit intoservo:mainfrom
yerke:replace-remaining-CanGc-note-part-16
Oct 25, 2025
Merged

add CanGc as argument to Validatable.validity_state#40155
yezhizhen merged 1 commit intoservo:mainfrom
yerke:replace-remaining-CanGc-note-part-16

Conversation

@yerke
Copy link
Copy Markdown
Contributor

@yerke yerke commented Oct 25, 2025

add CanGc as argument to Validatable.validity_state

Testing: These changes do not require tests because they are a refactor.
Addresses part of #34573.

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 25, 2025
@yerke
Copy link
Copy Markdown
Contributor Author

yerke commented Oct 25, 2025

@jdm Here is one for Validatable.validity_state

jdm
jdm approved these changes Oct 25, 2025
View reviewed changes
components/script/dom/html/htmlselectelement.rs Outdated

self.validity_state()
self.validity_state(can_gc)
.perform_validation_and_update(ValidationFlags::VALUE_MISSING, CanGc::note());
Copy link
Copy Markdown
Member

@jdm jdm Oct 25, 2025

Choose a reason for hiding this comment

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

Suggested change
.perform_validation_and_update(ValidationFlags::VALUE_MISSING, CanGc::note());
.perform_validation_and_update(ValidationFlags::VALUE_MISSING, can_gc);

Copy link
Copy Markdown
Contributor Author

@yerke yerke Oct 25, 2025

Choose a reason for hiding this comment

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

Thanks for catching that. Updated.

components/script/dom/html/htmltextareaelement.rs Outdated

self.validity_state()
self.validity_state(can_gc)
.perform_validation_and_update(ValidationFlags::all(), CanGc::note());
Copy link
Copy Markdown
Member

@jdm jdm Oct 25, 2025

Choose a reason for hiding this comment

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

Suggested change
.perform_validation_and_update(ValidationFlags::all(), CanGc::note());
.perform_validation_and_update(ValidationFlags::all(), can_gc);

Copy link
Copy Markdown
Contributor Author

@yerke yerke Oct 25, 2025

Choose a reason for hiding this comment

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

Thanks for catching that. Updated.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Oct 25, 2025
@yerke yerke force-pushed the replace-remaining-CanGc-note-part-16 branch from e0488d2 to 9c6c2cb Compare October 25, 2025 05:10
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 25, 2025
@yezhizhen
Copy link
Copy Markdown
Member

yezhizhen commented Oct 25, 2025

So the idea is that, we pass can_gc as far as possible, so that GC won't be happen until all callee with can_gc finishes?
And CanGC::note is considered the root?

@jdm
Copy link
Copy Markdown
Member

jdm commented Oct 25, 2025

So the idea is that, we pass can_gc as far as possible, so that GC won't be happen until all callee with can_gc finishes? And CanGC::note is considered the root?

No, a CanGc argument does not have any runtime behaviour. It's simply a warning to the caller that a GC could happen before the called function returns.

@yezhizhen yezhizhen mentioned this pull request Oct 25, 2025
Merged
@yerke
Copy link
Copy Markdown
Contributor Author

yerke commented Oct 25, 2025

@jdm CI is happy. Do you mind adding it to the merge queue? Thanks.

@yezhizhen yezhizhen added this pull request to the merge queue Oct 25, 2025
@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 Oct 25, 2025
Merged via the queue into servo:main with commit 132bd24 Oct 25, 2025
33 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 Oct 25, 2025
@yerke yerke deleted the replace-remaining-CanGc-note-part-16 branch October 25, 2025 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdm jdm jdm approved these changes

@yezhizhen yezhizhen Awaiting requested review from yezhizhen yezhizhen is a code owner

@gterzian gterzian Awaiting requested review from gterzian gterzian is a code owner

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yerke @yezhizhen @jdm @servo-highfive