Skip to content

Commit 0998a5b

Browse files
committed
avm1: Replace GcCell with Gc in Xml
This refactor replaces GcCell with Gc and uses interior mutability instead.
1 parent 3c85d80 commit 0998a5b

File tree

1 file changed

+26
-22
lines changed

1 file changed

+26
-22
lines changed

core/src/avm1/globals/xml.rs

+26-22
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! XML class
22
3+
use std::cell::Cell;
4+
35
use crate::avm1::function::{Executable, ExecutionReason, FunctionObject};
46
use crate::avm1::property_decl::{define_properties_on, Declaration};
57
use crate::avm1::{
@@ -9,7 +11,9 @@ use crate::avm_warn;
911
use crate::backend::navigator::Request;
1012
use crate::string::{AvmString, StringContext, WStr, WString};
1113
use crate::xml::{custom_unescape, XmlNode, ELEMENT_NODE, TEXT_NODE};
12-
use gc_arena::{Collect, GcCell};
14+
use gc_arena::barrier::unlock;
15+
use gc_arena::lock::Lock;
16+
use gc_arena::{Collect, Gc};
1317
use quick_xml::errors::IllFormedError;
1418
use quick_xml::events::attributes::AttrError;
1519
use quick_xml::{events::Event, Reader};
@@ -55,7 +59,7 @@ pub enum XmlStatus {
5559

5660
#[derive(Copy, Clone, Collect)]
5761
#[collect(no_drop)]
58-
pub struct Xml<'gc>(GcCell<'gc, XmlData<'gc>>);
62+
pub struct Xml<'gc>(Gc<'gc, XmlData<'gc>>);
5963

6064
#[derive(Clone, Collect)]
6165
#[collect(no_drop)]
@@ -64,10 +68,10 @@ pub struct XmlData<'gc> {
6468
root: XmlNode<'gc>,
6569

6670
/// The XML declaration, if set.
67-
xml_decl: Option<AvmString<'gc>>,
71+
xml_decl: Lock<Option<AvmString<'gc>>>,
6872

6973
/// The XML doctype, if set.
70-
doctype: Option<AvmString<'gc>>,
74+
doctype: Lock<Option<AvmString<'gc>>>,
7175

7276
/// The document's ID map.
7377
///
@@ -77,7 +81,7 @@ pub struct XmlData<'gc> {
7781
id_map: ScriptObject<'gc>,
7882

7983
/// The last parse error encountered, if any.
80-
status: XmlStatus,
84+
status: Cell<XmlStatus>,
8185
}
8286

8387
impl<'gc> Xml<'gc> {
@@ -88,14 +92,14 @@ impl<'gc> Xml<'gc> {
8892
let mut root = XmlNode::new(gc_context, ELEMENT_NODE, None);
8993
root.introduce_script_object(gc_context, object);
9094

91-
let xml = Self(GcCell::new(
95+
let xml = Self(Gc::new(
9296
gc_context,
9397
XmlData {
9498
root,
95-
xml_decl: None,
96-
doctype: None,
99+
xml_decl: Lock::new(None),
100+
doctype: Lock::new(None),
97101
id_map: ScriptObject::new(context, None),
98-
status: XmlStatus::NoError,
102+
status: Cell::new(XmlStatus::NoError),
99103
},
100104
));
101105
object.set_native(gc_context, NativeObject::Xml(xml));
@@ -104,33 +108,33 @@ impl<'gc> Xml<'gc> {
104108

105109
/// Yield the document in node form.
106110
pub fn root(self) -> XmlNode<'gc> {
107-
self.0.read().root
111+
self.0.root
108112
}
109113

110114
/// Retrieve the XML declaration of this document.
111115
fn xml_decl(self) -> Option<AvmString<'gc>> {
112-
self.0.read().xml_decl
116+
self.0.xml_decl.get()
113117
}
114118

115119
/// Retrieve the first DocType node in the document.
116120
fn doctype(self) -> Option<AvmString<'gc>> {
117-
self.0.read().doctype
121+
self.0.doctype.get()
118122
}
119123

120124
/// Obtain the script object for the document's `idMap` property.
121125
fn id_map(self) -> ScriptObject<'gc> {
122-
self.0.read().id_map
126+
self.0.id_map
123127
}
124128

125129
fn status(self) -> XmlStatus {
126-
self.0.read().status
130+
self.0.status.get()
127131
}
128132

129133
/// Replace the contents of this document with the result of parsing a string.
130134
///
131135
/// This method does not yet actually remove existing node contents.
132136
fn parse(
133-
&self,
137+
self,
134138
activation: &mut Activation<'_, 'gc>,
135139
data: &WStr,
136140
ignore_white: bool,
@@ -139,11 +143,11 @@ impl<'gc> Xml<'gc> {
139143
let mut parser = Reader::from_str(&data_utf8);
140144
let mut open_tags = vec![self.root()];
141145

142-
self.0.write(activation.gc()).status = XmlStatus::NoError;
146+
self.0.status.set(XmlStatus::NoError);
143147

144148
loop {
145149
let event = parser.read_event().map_err(|error| {
146-
self.0.write(activation.gc()).status = match error {
150+
self.0.status.set(match error {
147151
quick_xml::Error::Syntax(_)
148152
| quick_xml::Error::InvalidAttr(AttrError::ExpectedEq(_))
149153
| quick_xml::Error::InvalidAttr(AttrError::Duplicated(_, _)) => {
@@ -165,7 +169,7 @@ impl<'gc> Xml<'gc> {
165169
// quick_xml::Error::UnexpectedBang
166170
// quick_xml::Error::TextNotFound
167171
// quick_xml::Error::EscapeError(_)
168-
};
172+
});
169173
error
170174
})?;
171175

@@ -211,8 +215,8 @@ impl<'gc> Xml<'gc> {
211215
let mut xml_decl = WString::from_buf(b"<?".to_vec());
212216
xml_decl.push_str(WStr::from_units(&*bd));
213217
xml_decl.push_str(WStr::from_units(b"?>"));
214-
self.0.write(activation.gc()).xml_decl =
215-
Some(AvmString::new(activation.gc(), xml_decl));
218+
let xml_decl = Some(AvmString::new(activation.gc(), xml_decl));
219+
unlock!(Gc::write(activation.gc(), self.0), XmlData, xml_decl).set(xml_decl);
216220
}
217221
Event::DocType(bt) => {
218222
// TODO: `quick-xml` is case-insensitive for DOCTYPE declarations,
@@ -222,8 +226,8 @@ impl<'gc> Xml<'gc> {
222226
let mut doctype = WString::from_buf(b"<!DOCTYPE ".to_vec());
223227
doctype.push_str(WStr::from_units(&*bt.escape_ascii().collect::<Vec<_>>()));
224228
doctype.push_byte(b'>');
225-
self.0.write(activation.gc()).doctype =
226-
Some(AvmString::new(activation.gc(), doctype));
229+
let doctype = Some(AvmString::new(activation.gc(), doctype));
230+
unlock!(Gc::write(activation.gc(), self.0), XmlData, doctype).set(doctype);
227231
}
228232
Event::Eof => break,
229233
_ => {}

0 commit comments

Comments
 (0)