Add CanGc as arguments in methods in devtools.rs, CharacterData, CSSStyleRule, CSSStyleSheet (#36375)

Add CanGc as arguments in methods in devtools.rs, CharacterData,
CSSStyleRule, CSSStyleSheet

Testing: These changes do not require tests because they are a refactor.
Addressed part of https://github.com/servo/servo/issues/34573.

---------

Signed-off-by: Yerkebulan Tulibergenov <yerkebulan@gmail.com>
This commit is contained in:
Yerkebulan Tulibergenov 2025-04-06 16:44:58 -07:00 committed by GitHub
parent 1f558a0d49
commit 33b00dbe40
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 42 additions and 30 deletions

View file

@ -102,10 +102,11 @@ pub(crate) fn handle_get_root_node(
documents: &DocumentCollection, documents: &DocumentCollection,
pipeline: PipelineId, pipeline: PipelineId,
reply: IpcSender<Option<NodeInfo>>, reply: IpcSender<Option<NodeInfo>>,
can_gc: CanGc,
) { ) {
let info = documents let info = documents
.find_document(pipeline) .find_document(pipeline)
.map(|document| document.upcast::<Node>().summarize(CanGc::note())); .map(|document| document.upcast::<Node>().summarize(can_gc));
reply.send(info).unwrap(); reply.send(info).unwrap();
} }
@ -113,11 +114,12 @@ pub(crate) fn handle_get_document_element(
documents: &DocumentCollection, documents: &DocumentCollection,
pipeline: PipelineId, pipeline: PipelineId,
reply: IpcSender<Option<NodeInfo>>, reply: IpcSender<Option<NodeInfo>>,
can_gc: CanGc,
) { ) {
let info = documents let info = documents
.find_document(pipeline) .find_document(pipeline)
.and_then(|document| document.GetDocumentElement()) .and_then(|document| document.GetDocumentElement())
.map(|element| element.upcast::<Node>().summarize(CanGc::note())); .map(|element| element.upcast::<Node>().summarize(can_gc));
reply.send(info).unwrap(); reply.send(info).unwrap();
} }
@ -139,6 +141,7 @@ pub(crate) fn handle_get_children(
pipeline: PipelineId, pipeline: PipelineId,
node_id: String, node_id: String,
reply: IpcSender<Option<Vec<NodeInfo>>>, reply: IpcSender<Option<Vec<NodeInfo>>>,
can_gc: CanGc,
) { ) {
match find_node_by_unique_id(documents, pipeline, &node_id) { match find_node_by_unique_id(documents, pipeline, &node_id) {
None => reply.send(None).unwrap(), None => reply.send(None).unwrap(),
@ -166,7 +169,7 @@ pub(crate) fn handle_get_children(
if !shadow_root.is_user_agent_widget() || if !shadow_root.is_user_agent_widget() ||
pref!(inspector_show_servo_internal_shadow_roots) pref!(inspector_show_servo_internal_shadow_roots)
{ {
children.push(shadow_root.upcast::<Node>().summarize(CanGc::note())); children.push(shadow_root.upcast::<Node>().summarize(can_gc));
} }
} }
let children_iter = parent.children().enumerate().filter_map(|(i, child)| { let children_iter = parent.children().enumerate().filter_map(|(i, child)| {
@ -175,7 +178,7 @@ pub(crate) fn handle_get_children(
let prev_inline = i > 0 && inline[i - 1]; let prev_inline = i > 0 && inline[i - 1];
let next_inline = i < inline.len() - 1 && inline[i + 1]; let next_inline = i < inline.len() - 1 && inline[i + 1];
let info = child.summarize(CanGc::note()); let info = child.summarize(can_gc);
if !is_whitespace(&info) { if !is_whitespace(&info) {
return Some(info); return Some(info);
} }
@ -240,7 +243,7 @@ pub(crate) fn handle_get_stylesheet_style(
let owner = node.stylesheet_list_owner(); let owner = node.stylesheet_list_owner();
let stylesheet = owner.stylesheet_at(stylesheet)?; let stylesheet = owner.stylesheet_at(stylesheet)?;
let list = stylesheet.GetCssRules().ok()?; let list = stylesheet.GetCssRules(can_gc).ok()?;
let styles = (0..list.Length()) let styles = (0..list.Length())
.filter_map(move |i| { .filter_map(move |i| {
@ -249,7 +252,7 @@ pub(crate) fn handle_get_stylesheet_style(
if *selector != *style.SelectorText() { if *selector != *style.SelectorText() {
return None; return None;
}; };
Some(style.Style()) Some(style.Style(can_gc))
}) })
.flat_map(|style| { .flat_map(|style| {
(0..style.Length()).map(move |i| { (0..style.Length()).map(move |i| {
@ -287,7 +290,7 @@ pub(crate) fn handle_get_selectors(
let rules = (0..owner.stylesheet_count()) let rules = (0..owner.stylesheet_count())
.filter_map(|i| { .filter_map(|i| {
let stylesheet = owner.stylesheet_at(i)?; let stylesheet = owner.stylesheet_at(i)?;
let list = stylesheet.GetCssRules().ok()?; let list = stylesheet.GetCssRules(can_gc).ok()?;
let elem = node.downcast::<Element>()?; let elem = node.downcast::<Element>()?;
Some((0..list.Length()).filter_map(move |j| { Some((0..list.Length()).filter_map(move |j| {

View file

@ -260,9 +260,9 @@ impl CharacterDataMethods<crate::DomTypeHolder> for CharacterData {
} }
// https://dom.spec.whatwg.org/#dom-childnode-remove // https://dom.spec.whatwg.org/#dom-childnode-remove
fn Remove(&self) { fn Remove(&self, can_gc: CanGc) {
let node = self.upcast::<Node>(); let node = self.upcast::<Node>();
node.remove_self(CanGc::note()); node.remove_self(can_gc);
} }
// https://dom.spec.whatwg.org/#dom-nondocumenttypechildnode-previouselementsibling // https://dom.spec.whatwg.org/#dom-nondocumenttypechildnode-previouselementsibling

View file

@ -87,7 +87,7 @@ impl SpecificCSSRule for CSSStyleRule {
impl CSSStyleRuleMethods<crate::DomTypeHolder> for CSSStyleRule { impl CSSStyleRuleMethods<crate::DomTypeHolder> for CSSStyleRule {
// https://drafts.csswg.org/cssom/#dom-cssstylerule-style // https://drafts.csswg.org/cssom/#dom-cssstylerule-style
fn Style(&self) -> DomRoot<CSSStyleDeclaration> { fn Style(&self, can_gc: CanGc) -> DomRoot<CSSStyleDeclaration> {
self.style_decl.or_init(|| { self.style_decl.or_init(|| {
let guard = self.cssgroupingrule.shared_lock().read(); let guard = self.cssgroupingrule.shared_lock().read();
CSSStyleDeclaration::new( CSSStyleDeclaration::new(
@ -98,7 +98,7 @@ impl CSSStyleRuleMethods<crate::DomTypeHolder> for CSSStyleRule {
), ),
None, None,
CSSModificationAccess::ReadWrite, CSSModificationAccess::ReadWrite,
CanGc::note(), can_gc,
) )
}) })
} }

View file

@ -70,14 +70,14 @@ impl CSSStyleSheet {
) )
} }
fn rulelist(&self) -> DomRoot<CSSRuleList> { fn rulelist(&self, can_gc: CanGc) -> DomRoot<CSSRuleList> {
self.rulelist.or_init(|| { self.rulelist.or_init(|| {
let rules = self.style_stylesheet.contents.rules.clone(); let rules = self.style_stylesheet.contents.rules.clone();
CSSRuleList::new( CSSRuleList::new(
self.global().as_window(), self.global().as_window(),
self, self,
RulesSource::Rules(rules), RulesSource::Rules(rules),
CanGc::note(), can_gc,
) )
}) })
} }
@ -127,38 +127,38 @@ impl CSSStyleSheet {
impl CSSStyleSheetMethods<crate::DomTypeHolder> for CSSStyleSheet { impl CSSStyleSheetMethods<crate::DomTypeHolder> for CSSStyleSheet {
/// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-cssrules> /// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-cssrules>
fn GetCssRules(&self) -> Fallible<DomRoot<CSSRuleList>> { fn GetCssRules(&self, can_gc: CanGc) -> Fallible<DomRoot<CSSRuleList>> {
if !self.origin_clean.get() { if !self.origin_clean.get() {
return Err(Error::Security); return Err(Error::Security);
} }
Ok(self.rulelist()) Ok(self.rulelist(can_gc))
} }
/// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-insertrule> /// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-insertrule>
fn InsertRule(&self, rule: DOMString, index: u32) -> Fallible<u32> { fn InsertRule(&self, rule: DOMString, index: u32, can_gc: CanGc) -> Fallible<u32> {
if !self.origin_clean.get() { if !self.origin_clean.get() {
return Err(Error::Security); return Err(Error::Security);
} }
self.rulelist() self.rulelist(can_gc)
.insert_rule(&rule, index, CssRuleTypes::default(), None, CanGc::note()) .insert_rule(&rule, index, CssRuleTypes::default(), None, can_gc)
} }
/// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-deleterule> /// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-deleterule>
fn DeleteRule(&self, index: u32) -> ErrorResult { fn DeleteRule(&self, index: u32, can_gc: CanGc) -> ErrorResult {
if !self.origin_clean.get() { if !self.origin_clean.get() {
return Err(Error::Security); return Err(Error::Security);
} }
self.rulelist().remove_rule(index) self.rulelist(can_gc).remove_rule(index)
} }
/// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-rules> /// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-rules>
fn GetRules(&self) -> Fallible<DomRoot<CSSRuleList>> { fn GetRules(&self, can_gc: CanGc) -> Fallible<DomRoot<CSSRuleList>> {
self.GetCssRules() self.GetCssRules(can_gc)
} }
/// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-removerule> /// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-removerule>
fn RemoveRule(&self, index: u32) -> ErrorResult { fn RemoveRule(&self, index: u32, can_gc: CanGc) -> ErrorResult {
self.DeleteRule(index) self.DeleteRule(index, can_gc)
} }
/// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-addrule> /// <https://drafts.csswg.org/cssom/#dom-cssstylesheet-addrule>
@ -167,6 +167,7 @@ impl CSSStyleSheetMethods<crate::DomTypeHolder> for CSSStyleSheet {
selector: DOMString, selector: DOMString,
block: DOMString, block: DOMString,
optional_index: Option<u32>, optional_index: Option<u32>,
can_gc: CanGc,
) -> Fallible<i32> { ) -> Fallible<i32> {
// > 1. Let *rule* be an empty string. // > 1. Let *rule* be an empty string.
// > 2. Append *selector* to *rule*. // > 2. Append *selector* to *rule*.
@ -184,10 +185,10 @@ impl CSSStyleSheetMethods<crate::DomTypeHolder> for CSSStyleSheet {
}; };
// > 6. Let *index* be *optionalIndex* if provided, or the number of CSS rules in the stylesheet otherwise. // > 6. Let *index* be *optionalIndex* if provided, or the number of CSS rules in the stylesheet otherwise.
let index = optional_index.unwrap_or_else(|| self.rulelist().Length()); let index = optional_index.unwrap_or_else(|| self.rulelist(can_gc).Length());
// > 7. Call `insertRule()`, with *rule* and *index* as arguments. // > 7. Call `insertRule()`, with *rule* and *index* as arguments.
self.InsertRule(rule, index)?; self.InsertRule(rule, index, can_gc)?;
// > 8. Return -1. // > 8. Return -1.
Ok(-1) Ok(-1)

View file

@ -2006,13 +2006,13 @@ impl ScriptThread {
None => warn!("Message sent to closed pipeline {}.", id), None => warn!("Message sent to closed pipeline {}.", id),
}, },
DevtoolScriptControlMsg::GetRootNode(id, reply) => { DevtoolScriptControlMsg::GetRootNode(id, reply) => {
devtools::handle_get_root_node(&documents, id, reply) devtools::handle_get_root_node(&documents, id, reply, can_gc)
}, },
DevtoolScriptControlMsg::GetDocumentElement(id, reply) => { DevtoolScriptControlMsg::GetDocumentElement(id, reply) => {
devtools::handle_get_document_element(&documents, id, reply) devtools::handle_get_document_element(&documents, id, reply, can_gc)
}, },
DevtoolScriptControlMsg::GetChildren(id, node_id, reply) => { DevtoolScriptControlMsg::GetChildren(id, node_id, reply) => {
devtools::handle_get_children(&documents, id, node_id, reply) devtools::handle_get_children(&documents, id, node_id, reply, can_gc)
}, },
DevtoolScriptControlMsg::GetAttributeStyle(id, node_id, reply) => { DevtoolScriptControlMsg::GetAttributeStyle(id, node_id, reply) => {
devtools::handle_get_attribute_style(&documents, id, node_id, reply, can_gc) devtools::handle_get_attribute_style(&documents, id, node_id, reply, can_gc)

View file

@ -84,7 +84,7 @@ DOMInterfaces = {
}, },
'CharacterData': { 'CharacterData': {
'canGc': ['Before', 'After', 'ReplaceWith'] 'canGc': ['Before', 'After', 'Remove', 'ReplaceWith']
}, },
'CountQueuingStrategy': { 'CountQueuingStrategy': {
@ -119,6 +119,14 @@ DOMInterfaces = {
'canGc': ['Item', 'IndexedGetter'], 'canGc': ['Item', 'IndexedGetter'],
}, },
'CSSStyleRule': {
'canGc': ['Style'],
},
'CSSStyleSheet': {
'canGc': ['AddRule', 'DeleteRule', 'GetCssRules', 'GetRules', 'InsertRule', 'RemoveRule'],
},
'Crypto': { 'Crypto': {
'canGc': ['Subtle'], 'canGc': ['Subtle'],
}, },