Skip to content

Commit 773d67f

Browse files
yerkeTG199
authored andcommitted
refactor: add CanGc as argument to methods in Document (servo#36392)
Add CanGc as arguments in methods in Document Testing: These changes do not require tests because they are a refactor. Addressed part of servo#34573. Signed-off-by: Yerkebulan Tulibergenov <[email protected]>
1 parent 81d01d3 commit 773d67f

File tree

6 files changed

+61
-55
lines changed

6 files changed

+61
-55
lines changed

components/script/dom/document.rs

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2964,9 +2964,14 @@ impl Document {
29642964

29652965
/// <https://html.spec.whatwg.org/multipage/#the-end> step 3.
29662966
/// <https://html.spec.whatwg.org/multipage/#prepare-a-script> step 22.d.
2967-
pub(crate) fn deferred_script_loaded(&self, element: &HTMLScriptElement, result: ScriptResult) {
2967+
pub(crate) fn deferred_script_loaded(
2968+
&self,
2969+
element: &HTMLScriptElement,
2970+
result: ScriptResult,
2971+
can_gc: CanGc,
2972+
) {
29682973
self.deferred_scripts.loaded(element, result);
2969-
self.process_deferred_scripts(CanGc::note());
2974+
self.process_deferred_scripts(can_gc);
29702975
}
29712976

29722977
/// <https://html.spec.whatwg.org/multipage/#the-end> step 3.
@@ -4855,20 +4860,20 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
48554860
}
48564861

48574862
// https://drafts.csswg.org/cssom/#dom-document-stylesheets
4858-
fn StyleSheets(&self) -> DomRoot<StyleSheetList> {
4863+
fn StyleSheets(&self, can_gc: CanGc) -> DomRoot<StyleSheetList> {
48594864
self.stylesheet_list.or_init(|| {
48604865
StyleSheetList::new(
48614866
&self.window,
48624867
StyleSheetListOwner::Document(Dom::from_ref(self)),
4863-
CanGc::note(),
4868+
can_gc,
48644869
)
48654870
})
48664871
}
48674872

48684873
// https://dom.spec.whatwg.org/#dom-document-implementation
4869-
fn Implementation(&self) -> DomRoot<DOMImplementation> {
4874+
fn Implementation(&self, can_gc: CanGc) -> DomRoot<DOMImplementation> {
48704875
self.implementation
4871-
.or_init(|| DOMImplementation::new(self, CanGc::note()))
4876+
.or_init(|| DOMImplementation::new(self, can_gc))
48724877
}
48734878

48744879
// https://dom.spec.whatwg.org/#dom-document-url
@@ -4996,7 +5001,11 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
49965001
}
49975002

49985003
// https://dom.spec.whatwg.org/#dom-document-getelementsbytagname
4999-
fn GetElementsByTagName(&self, qualified_name: DOMString) -> DomRoot<HTMLCollection> {
5004+
fn GetElementsByTagName(
5005+
&self,
5006+
qualified_name: DOMString,
5007+
can_gc: CanGc,
5008+
) -> DomRoot<HTMLCollection> {
50005009
let qualified_name = LocalName::from(&*qualified_name);
50015010
if let Some(entry) = self.tag_map.borrow_mut().get(&qualified_name) {
50025011
return DomRoot::from_ref(entry);
@@ -5005,7 +5014,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
50055014
&self.window,
50065015
self.upcast(),
50075016
qualified_name.clone(),
5008-
CanGc::note(),
5017+
can_gc,
50095018
);
50105019
self.tag_map
50115020
.borrow_mut()
@@ -5018,27 +5027,24 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
50185027
&self,
50195028
maybe_ns: Option<DOMString>,
50205029
tag_name: DOMString,
5030+
can_gc: CanGc,
50215031
) -> DomRoot<HTMLCollection> {
50225032
let ns = namespace_from_domstring(maybe_ns);
50235033
let local = LocalName::from(tag_name);
50245034
let qname = QualName::new(None, ns, local);
50255035
if let Some(collection) = self.tagns_map.borrow().get(&qname) {
50265036
return DomRoot::from_ref(collection);
50275037
}
5028-
let result = HTMLCollection::by_qual_tag_name(
5029-
&self.window,
5030-
self.upcast(),
5031-
qname.clone(),
5032-
CanGc::note(),
5033-
);
5038+
let result =
5039+
HTMLCollection::by_qual_tag_name(&self.window, self.upcast(), qname.clone(), can_gc);
50345040
self.tagns_map
50355041
.borrow_mut()
50365042
.insert(qname, Dom::from_ref(&*result));
50375043
result
50385044
}
50395045

50405046
// https://dom.spec.whatwg.org/#dom-document-getelementsbyclassname
5041-
fn GetElementsByClassName(&self, classes: DOMString) -> DomRoot<HTMLCollection> {
5047+
fn GetElementsByClassName(&self, classes: DOMString, can_gc: CanGc) -> DomRoot<HTMLCollection> {
50425048
let class_atoms: Vec<Atom> = split_html_space_chars(&classes).map(Atom::from).collect();
50435049
if let Some(collection) = self.classes_map.borrow().get(&class_atoms) {
50445050
return DomRoot::from_ref(collection);
@@ -5047,7 +5053,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
50475053
&self.window,
50485054
self.upcast(),
50495055
class_atoms.clone(),
5050-
CanGc::note(),
5056+
can_gc,
50515057
);
50525058
self.classes_map
50535059
.borrow_mut()
@@ -5257,7 +5263,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
52575263
}
52585264

52595265
// https://dom.spec.whatwg.org/#dom-document-adoptnode
5260-
fn AdoptNode(&self, node: &Node) -> Fallible<DomRoot<Node>> {
5266+
fn AdoptNode(&self, node: &Node, can_gc: CanGc) -> Fallible<DomRoot<Node>> {
52615267
// Step 1.
52625268
if node.is::<Document>() {
52635269
return Err(Error::NotSupported);
@@ -5269,7 +5275,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
52695275
}
52705276

52715277
// Step 3.
5272-
Node::adopt(node, self, CanGc::note());
5278+
Node::adopt(node, self, can_gc);
52735279

52745280
// Step 4.
52755281
Ok(DomRoot::from_ref(node))
@@ -5358,8 +5364,9 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
53585364
root: &Node,
53595365
what_to_show: u32,
53605366
filter: Option<Rc<NodeFilter>>,
5367+
can_gc: CanGc,
53615368
) -> DomRoot<NodeIterator> {
5362-
NodeIterator::new(self, root, what_to_show, filter, CanGc::note())
5369+
NodeIterator::new(self, root, what_to_show, filter, can_gc)
53635370
}
53645371

53655372
// https://dom.spec.whatwg.org/#dom-document-createtreewalker
@@ -5476,7 +5483,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
54765483
}
54775484

54785485
// https://html.spec.whatwg.org/multipage/#dom-document-body
5479-
fn SetBody(&self, new_body: Option<&HTMLElement>) -> ErrorResult {
5486+
fn SetBody(&self, new_body: Option<&HTMLElement>, can_gc: CanGc) -> ErrorResult {
54805487
// Step 1.
54815488
let new_body = match new_body {
54825489
Some(new_body) => new_body,
@@ -5502,7 +5509,7 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
55025509
// Step 3.
55035510
(Some(ref root), Some(child)) => {
55045511
let root = root.upcast::<Node>();
5505-
root.ReplaceChild(new_body.upcast(), child.upcast(), CanGc::note())
5512+
root.ReplaceChild(new_body.upcast(), child.upcast(), can_gc)
55065513
.unwrap();
55075514
},
55085515

@@ -5512,48 +5519,48 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
55125519
// Step 5.
55135520
(Some(ref root), &None) => {
55145521
let root = root.upcast::<Node>();
5515-
root.AppendChild(new_body.upcast(), CanGc::note()).unwrap();
5522+
root.AppendChild(new_body.upcast(), can_gc).unwrap();
55165523
},
55175524
}
55185525
Ok(())
55195526
}
55205527

55215528
// https://html.spec.whatwg.org/multipage/#dom-document-getelementsbyname
5522-
fn GetElementsByName(&self, name: DOMString) -> DomRoot<NodeList> {
5523-
NodeList::new_elements_by_name_list(self.window(), self, name, CanGc::note())
5529+
fn GetElementsByName(&self, name: DOMString, can_gc: CanGc) -> DomRoot<NodeList> {
5530+
NodeList::new_elements_by_name_list(self.window(), self, name, can_gc)
55245531
}
55255532

55265533
// https://html.spec.whatwg.org/multipage/#dom-document-images
5527-
fn Images(&self) -> DomRoot<HTMLCollection> {
5534+
fn Images(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
55285535
self.images.or_init(|| {
55295536
HTMLCollection::new_with_filter_fn(
55305537
&self.window,
55315538
self.upcast(),
55325539
|element, _| element.is::<HTMLImageElement>(),
5533-
CanGc::note(),
5540+
can_gc,
55345541
)
55355542
})
55365543
}
55375544

55385545
// https://html.spec.whatwg.org/multipage/#dom-document-embeds
5539-
fn Embeds(&self) -> DomRoot<HTMLCollection> {
5546+
fn Embeds(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
55405547
self.embeds.or_init(|| {
55415548
HTMLCollection::new_with_filter_fn(
55425549
&self.window,
55435550
self.upcast(),
55445551
|element, _| element.is::<HTMLEmbedElement>(),
5545-
CanGc::note(),
5552+
can_gc,
55465553
)
55475554
})
55485555
}
55495556

55505557
// https://html.spec.whatwg.org/multipage/#dom-document-plugins
5551-
fn Plugins(&self) -> DomRoot<HTMLCollection> {
5552-
self.Embeds()
5558+
fn Plugins(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
5559+
self.Embeds(can_gc)
55535560
}
55545561

55555562
// https://html.spec.whatwg.org/multipage/#dom-document-links
5556-
fn Links(&self) -> DomRoot<HTMLCollection> {
5563+
fn Links(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
55575564
self.links.or_init(|| {
55585565
HTMLCollection::new_with_filter_fn(
55595566
&self.window,
@@ -5562,53 +5569,53 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
55625569
(element.is::<HTMLAnchorElement>() || element.is::<HTMLAreaElement>()) &&
55635570
element.has_attribute(&local_name!("href"))
55645571
},
5565-
CanGc::note(),
5572+
can_gc,
55665573
)
55675574
})
55685575
}
55695576

55705577
// https://html.spec.whatwg.org/multipage/#dom-document-forms
5571-
fn Forms(&self) -> DomRoot<HTMLCollection> {
5578+
fn Forms(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
55725579
self.forms.or_init(|| {
55735580
HTMLCollection::new_with_filter_fn(
55745581
&self.window,
55755582
self.upcast(),
55765583
|element, _| element.is::<HTMLFormElement>(),
5577-
CanGc::note(),
5584+
can_gc,
55785585
)
55795586
})
55805587
}
55815588

55825589
// https://html.spec.whatwg.org/multipage/#dom-document-scripts
5583-
fn Scripts(&self) -> DomRoot<HTMLCollection> {
5590+
fn Scripts(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
55845591
self.scripts.or_init(|| {
55855592
HTMLCollection::new_with_filter_fn(
55865593
&self.window,
55875594
self.upcast(),
55885595
|element, _| element.is::<HTMLScriptElement>(),
5589-
CanGc::note(),
5596+
can_gc,
55905597
)
55915598
})
55925599
}
55935600

55945601
// https://html.spec.whatwg.org/multipage/#dom-document-anchors
5595-
fn Anchors(&self) -> DomRoot<HTMLCollection> {
5602+
fn Anchors(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
55965603
self.anchors.or_init(|| {
55975604
HTMLCollection::new_with_filter_fn(
55985605
&self.window,
55995606
self.upcast(),
56005607
|element, _| {
56015608
element.is::<HTMLAnchorElement>() && element.has_attribute(&local_name!("href"))
56025609
},
5603-
CanGc::note(),
5610+
can_gc,
56045611
)
56055612
})
56065613
}
56075614

56085615
// https://html.spec.whatwg.org/multipage/#dom-document-applets
5609-
fn Applets(&self) -> DomRoot<HTMLCollection> {
5616+
fn Applets(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
56105617
self.applets
5611-
.or_init(|| HTMLCollection::always_empty(&self.window, self.upcast(), CanGc::note()))
5618+
.or_init(|| HTMLCollection::always_empty(&self.window, self.upcast(), can_gc))
56125619
}
56135620

56145621
// https://html.spec.whatwg.org/multipage/#dom-document-location
@@ -5621,8 +5628,8 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
56215628
}
56225629

56235630
// https://dom.spec.whatwg.org/#dom-parentnode-children
5624-
fn Children(&self) -> DomRoot<HTMLCollection> {
5625-
HTMLCollection::children(&self.window, self.upcast(), CanGc::note())
5631+
fn Children(&self, can_gc: CanGc) -> DomRoot<HTMLCollection> {
5632+
HTMLCollection::children(&self.window, self.upcast(), can_gc)
56265633
}
56275634

56285635
// https://dom.spec.whatwg.org/#dom-parentnode-firstelementchild
@@ -6192,12 +6199,9 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
61926199
}
61936200

61946201
// https://w3c.github.io/selection-api/#dom-document-getselection
6195-
fn GetSelection(&self) -> Option<DomRoot<Selection>> {
6202+
fn GetSelection(&self, can_gc: CanGc) -> Option<DomRoot<Selection>> {
61966203
if self.has_browsing_context {
6197-
Some(
6198-
self.selection
6199-
.or_init(|| Selection::new(self, CanGc::note())),
6200-
)
6204+
Some(self.selection.or_init(|| Selection::new(self, can_gc)))
62016205
} else {
62026206
None
62036207
}

components/script/dom/htmlscriptelement.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ fn finish_fetching_a_classic_script(
360360
},
361361
ExternalScriptKind::Deferred => {
362362
document = elem.parser_document.as_rooted();
363-
document.deferred_script_loaded(elem, load);
363+
document.deferred_script_loaded(elem, load, can_gc);
364364
},
365365
ExternalScriptKind::ParsingBlocking => {
366366
document = elem.parser_document.as_rooted();

components/script/dom/window.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1523,7 +1523,9 @@ impl WindowMethods<crate::DomTypeHolder> for Window {
15231523

15241524
// https://w3c.github.io/selection-api/#dom-window-getselection
15251525
fn GetSelection(&self) -> Option<DomRoot<Selection>> {
1526-
self.document.get().and_then(|d| d.GetSelection())
1526+
self.document
1527+
.get()
1528+
.and_then(|d| d.GetSelection(CanGc::note()))
15271529
}
15281530

15291531
// https://dom.spec.whatwg.org/#dom-window-event

components/script/script_module.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -992,7 +992,7 @@ impl ModuleOwner {
992992
.has_attribute(&local_name!("async"));
993993

994994
if !asynch && (*script.root()).get_parser_inserted() {
995-
document.deferred_script_loaded(&script.root(), load);
995+
document.deferred_script_loaded(&script.root(), load, can_gc);
996996
} else if !asynch && !(*script.root()).get_non_blocking() {
997997
document.asap_in_order_script_loaded(&script.root(), load, can_gc);
998998
} else {

components/script/webdriver_handlers.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ pub(crate) fn handle_find_element_tag_name(
553553
pipeline: PipelineId,
554554
selector: String,
555555
reply: IpcSender<Result<Option<String>, ErrorStatus>>,
556-
_can_gc: CanGc,
556+
can_gc: CanGc,
557557
) {
558558
reply
559559
.send(
@@ -562,7 +562,7 @@ pub(crate) fn handle_find_element_tag_name(
562562
.ok_or(ErrorStatus::UnknownError)
563563
.map(|document| {
564564
document
565-
.GetElementsByTagName(DOMString::from(selector))
565+
.GetElementsByTagName(DOMString::from(selector), can_gc)
566566
.elements_iter()
567567
.next()
568568
})
@@ -621,14 +621,14 @@ pub(crate) fn handle_find_elements_tag_name(
621621
pipeline: PipelineId,
622622
selector: String,
623623
reply: IpcSender<Result<Vec<String>, ErrorStatus>>,
624-
_can_gc: CanGc,
624+
can_gc: CanGc,
625625
) {
626626
reply
627627
.send(
628628
documents
629629
.find_document(pipeline)
630630
.ok_or(ErrorStatus::UnknownError)
631-
.map(|document| document.GetElementsByTagName(DOMString::from(selector)))
631+
.map(|document| document.GetElementsByTagName(DOMString::from(selector), can_gc))
632632
.map(|nodes| {
633633
nodes
634634
.elements_iter()

components/script_bindings/codegen/Bindings.conf

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ DOMInterfaces = {
158158

159159
'Document': {
160160
'additionalTraits': ["crate::interfaces::DocumentHelpers"],
161-
'canGc': ['Close', 'CreateElement', 'CreateElementNS', 'ImportNode', 'SetTitle', 'Write', 'Writeln', 'CreateEvent', 'CreateRange', 'Open', 'Open_', 'CreateComment', 'CreateAttribute', 'CreateAttributeNS', 'CreateDocumentFragment', 'CreateTextNode', 'CreateCDATASection', 'CreateProcessingInstruction', 'Prepend', 'Append', 'ReplaceChildren', 'SetBgColor', 'SetFgColor', 'Fonts', 'ElementFromPoint', 'ElementsFromPoint', 'ExitFullscreen', 'CreateExpression', 'CreateNSResolver', 'Evaluate'],
161+
'canGc': ['Close', 'CreateElement', 'CreateElementNS', 'ImportNode', 'SetTitle', 'Write', 'Writeln', 'CreateEvent', 'CreateRange', 'Open', 'Open_', 'CreateComment', 'CreateAttribute', 'CreateAttributeNS', 'CreateDocumentFragment', 'CreateTextNode', 'CreateCDATASection', 'CreateProcessingInstruction', 'Prepend', 'Append', 'ReplaceChildren', 'SetBgColor', 'SetFgColor', 'Fonts', 'ElementFromPoint', 'ElementsFromPoint', 'ExitFullscreen', 'CreateExpression', 'CreateNSResolver', 'Evaluate', 'StyleSheets', 'Implementation', 'GetElementsByTagName', 'GetElementsByTagNameNS', 'GetElementsByClassName', 'AdoptNode', 'CreateNodeIterator', 'SetBody', 'GetElementsByName', 'Images', 'Embeds', 'Plugins', 'Links', 'Forms', 'Scripts', 'Anchors', 'Applets', 'Children', 'GetSelection'],
162162
},
163163

164164
'DocumentFragment': {

0 commit comments

Comments
 (0)