On 10/30/2013 11:26 PM, Jack Moffitt wrote:
Right now we have this TreeNode abstraction that has been around for a
long time and has lost its original purpose (whatever that was). It's
only job right now appears to be to break the cyclic dependency
between the script and style crates.

Does anyone have strong feelings as to whether this is a worthwhile
reason to keep TreeNode around? If we get rid of it, I assume that
style will have to live in the script crate as it did before.

I don't think it should be kept in its current incarnation, at least.

For example, |add_child| doesn't do any checks to make sure that the resulting DOM tree is correct, and we already use it to append DocumentType nodes to the root Element. It seems to be that keeping such a function around, in a way that anyone else than the Node mutation algorithms can call it, is a recipe for messing up our invariants.

Also, the way it's written right now (|set!(next_sibling_node, set_prev_sibling, get!(child_node, prev_sibling));|, to pick but one line) is pretty unreadable, and I've had trouble adding assertions to ensure the correctness (I don't remember what the issue was exactly; possibly a lack of Eq).

So if we want to keep some abstraction outside of script—which might be useful, given that whole-crate compilation makes large crates quite painful—it'll need quite a bit of work, and we should at least consider making it only expose a read-only view.

HTH
Ms2ger
_______________________________________________
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Reply via email to