liangsheng8708
/
ruffle
mirror of https://mirror.ghproxy.com/https://github.com/ruffle-rs/ruffle.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Swap in newly constructed nodes *before* filling them with content. 

Fixes a bug where new XML("<node />").childNode[0].parentNode did NOT refer to the overall document object, but to a phantom text node.

This is because the swap operation used to construct an XMLObject's node in-place was happening AFTER parsing, which means that referents already existed to the temporary XMLNode created by XMLObject::new. swap is not to be called after tree structure has been created; it does not update referents to the swapped nodes.

In the future I should examine the implications of explicitly reconstructing already existing nodes, e.g. through XML.apply(some_xml). Right now, the existing node will be swapped with a new one, and two nodes will exist pointing to the same script object, which is a huge problem with our overall design. We should, at the very least, disassociate swapped nodes from their script object, just in case they still have referents.

Ideally, we wouldn't have to swap nodes, but to avoid a swap, I'd have to instead have a second layer of indirection just to hold a rewritable pointer that every XMLObject points to. This isn't really worth it unless I HAVE to do it, so I'm not going to do it.
This commit is contained in:
David Wendt 2019-12-27 20:55:15 -07:00
parent 568d90f4dc
commit 8c5dcfe662
4 changed files with 92 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
core/src/avm1/globals/xml.rs
View File

@ -532,18 +532,18 @@ pub fn xml_constructor<'gc>(
this.as_xml_node(),
) {
(Some(Ok(ref string)), Some(ref mut this_node)) => {
let xmldoc = XMLDocument::from_str(ac.gc_context, string)?;
xmldoc
.as_node()
.introduce_script_object(ac.gc_context, this);
this_node.swap(ac.gc_context, xmldoc.as_node());
let xmldoc = XMLDocument::new(ac.gc_context);
let mut xmlnode = xmldoc.as_node();
xmlnode.introduce_script_object(ac.gc_context, this);
this_node.swap(ac.gc_context, xmlnode);
this_node.replace_with_str(ac.gc_context, string)?;
}
(None, Some(ref mut this_node)) => {
let xmldoc = XMLDocument::new(ac.gc_context);
xmldoc
.as_node()
.introduce_script_object(ac.gc_context, this);
this_node.swap(ac.gc_context, xmldoc.as_node());
let mut xmlnode = xmldoc.as_node();
xmlnode.introduce_script_object(ac.gc_context, this);
this_node.swap(ac.gc_context, xmlnode);
}
//Non-string argument or not an XML document
_ => {}

61
core/src/xml/document.rs
View File

@ -1,9 +1,7 @@
//! XML Document
use crate::xml::{Error, XMLNode};
use crate::xml::XMLNode;
use gc_arena::{Collect, GcCell, MutationContext};
use quick_xml::events::Event;
use quick_xml::Reader;
use std::fmt;
/// The entirety of an XML document.
@ -29,63 +27,6 @@ impl<'gc> XMLDocument<'gc> {
document
}
/// Ensure that a newly-encountered node is added to an ongoing parsing
/// stack, or to the document itself if the parsing stack is empty.
fn add_child_to_tree(
&mut self,
mc: MutationContext<'gc, '_>,
open_tags: &mut Vec<XMLNode<'gc>>,
child: XMLNode<'gc>,
) -> Result<(), Error> {
if let Some(node) = open_tags.last_mut() {
node.append_child(mc, child)?;
} else {
self.as_node().append_child(mc, child)?;
}
Ok(())
}
pub fn from_str(mc: MutationContext<'gc, '_>, data: &str) -> Result<Self, Error> {
let mut parser = Reader::from_str(data);
let mut buf = Vec::new();
let mut document = Self::new(mc);
let mut open_tags: Vec<XMLNode<'gc>> = Vec::new();
loop {
match parser.read_event(&mut buf)? {
Event::Start(bs) => {
let child = XMLNode::from_start_event(mc, bs, document)?;
document.add_child_to_tree(mc, &mut open_tags, child)?;
open_tags.push(child);
}
Event::Empty(bs) => {
let child = XMLNode::from_start_event(mc, bs, document)?;
document.add_child_to_tree(mc, &mut open_tags, child)?;
}
Event::End(_) => {
open_tags.pop();
}
Event::Text(bt) => {
let child = XMLNode::text_from_text_event(mc, bt, document)?;
if child.node_value().as_deref() != Some("") {
document.add_child_to_tree(mc, &mut open_tags, child)?;
}
}
Event::Comment(bt) => {
let child = XMLNode::comment_from_text_event(mc, bt, document)?;
if child.node_value().as_deref() != Some("") {
document.add_child_to_tree(mc, &mut open_tags, child)?;
}
}
Event::Eof => break,
_ => {}
}
}
Ok(document)
}
/// Yield the document in node form.
///
/// If the document does not have a node, then this function will panic.

17
core/src/xml/tests.rs
View File

@ -8,7 +8,10 @@ use gc_arena::rootless_arena;
#[test]
fn parse_single_element() {
rootless_arena(|mc| {
let xml = XMLDocument::from_str(mc, "<test></test>").expect("Parsed document");
let xml = XMLDocument::new(mc);
xml.as_node()
.replace_with_str(mc, "<test></test>")
.expect("Parsed document");
dbg!(xml);
let mut roots = xml
.as_node()
@ -30,11 +33,13 @@ fn parse_single_element() {
#[test]
fn double_ended_children() {
rootless_arena(|mc| {
let xml = XMLDocument::from_str(
mc,
"<test></test><test2></test2><test3></test3><test4></test4><test5></test5>",
)
.expect("Parsed document");
let xml = XMLDocument::new(mc);
xml.as_node()
.replace_with_str(
mc,
"<test></test><test2></test2><test3></test3><test4></test4><test5></test5>",
)
.expect("Parsed document");
let mut roots = xml
.as_node()

74
core/src/xml/tree.rs
View File

@ -2,13 +2,13 @@
use crate::avm1::xml_attributes_object::XMLAttributesObject;
use crate::avm1::xml_object::XMLObject;
use crate::avm1::Object;
use crate::avm1::{Object, TObject};
use crate::xml;
use crate::xml::{Error, XMLDocument, XMLName};
use gc_arena::{Collect, GcCell, MutationContext};
use quick_xml::events::attributes::Attribute;
use quick_xml::events::{BytesEnd, BytesStart, BytesText, Event};
use quick_xml::Writer;
use quick_xml::{Reader, Writer};
use std::borrow::Cow;
use std::collections::BTreeMap;
use std::fmt;
@ -166,6 +166,73 @@ impl<'gc> XMLNode<'gc> {
))
}
/// Ensure that a newly-encountered node is added to an ongoing parsing
/// stack, or to the document root itself if the parsing stack is empty.
fn add_child_to_tree(
&mut self,
mc: MutationContext<'gc, '_>,
open_tags: &mut Vec<XMLNode<'gc>>,
child: XMLNode<'gc>,
) -> Result<(), Error> {
if let Some(node) = open_tags.last_mut() {
node.append_child(mc, child)?;
} else {
self.append_child(mc, child)?;
}
Ok(())
}
/// Replace the contents of this node with the result of parsing a string.
///
/// Node replacements are only supported on document root nodes; elements
/// may work but will be incorrect.
///
/// Also, this method does not yet actually remove existing node contents.
pub fn replace_with_str(
&mut self,
mc: MutationContext<'gc, '_>,
data: &str,
) -> Result<(), Error> {
let mut parser = Reader::from_str(data);
let mut buf = Vec::new();
let document = self.document();
let mut open_tags: Vec<XMLNode<'gc>> = Vec::new();
loop {
match parser.read_event(&mut buf)? {
Event::Start(bs) => {
let child = XMLNode::from_start_event(mc, bs, document)?;
self.add_child_to_tree(mc, &mut open_tags, child)?;
open_tags.push(child);
}
Event::Empty(bs) => {
let child = XMLNode::from_start_event(mc, bs, document)?;
self.add_child_to_tree(mc, &mut open_tags, child)?;
}
Event::End(_) => {
open_tags.pop();
}
Event::Text(bt) => {
let child = XMLNode::text_from_text_event(mc, bt, document)?;
if child.node_value().as_deref() != Some("") {
self.add_child_to_tree(mc, &mut open_tags, child)?;
}
}
Event::Comment(bt) => {
let child = XMLNode::comment_from_text_event(mc, bt, document)?;
if child.node_value().as_deref() != Some("") {
self.add_child_to_tree(mc, &mut open_tags, child)?;
}
}
Event::Eof => break,
_ => {}
}
}
Ok(())
}
/// Construct an XML Element node from a `quick_xml` `BytesStart` event.
///
/// The returned node will always be an `Element`, and it must only contain
@ -762,7 +829,8 @@ impl<'gc> XMLNode<'gc> {
///
/// After this function completes, the current `XMLNode` will contain all
/// data present in the `other` node, and vice versa. References to the node
/// within the tree will *not* be updated.
/// will *not* be updated: it is a logic error to swap nodes that have
/// existing referents.
pub fn swap(&mut self, gc_context: MutationContext<'gc, '_>, other: Self) {
if !GcCell::ptr_eq(self.0, other.0) {
swap(