Mutation observer API #16668
Closed
Mutation observer API #16668
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @asajeffrey (or someone else) soon. |
|
Heads up! This PR modifies the following files:
|
|
|
highfive
added
the
S-awaiting-review
KiChjang
reviewed
Apr 30, 2017
components/script/dom/element.rs
Outdated
| @@ -2816,6 +2829,11 @@ impl Element { | |||
| self.set_disabled_state(has_disabled_attrib); | |||
| self.set_enabled_state(!has_disabled_attrib); | |||
| } | |||
|
|
|||
| pub fn get_node(&self) -> &Node { | |||
There was a problem hiding this comment.
This looks like an extraneous function. For places where you are supplied with an Element, you can call element.upcast::<Node>() to get a Root<Node>.
|
This does not compile as written. When submitting code, can you please be explicit about the status of it and what assistance you are looking for? |
|
Hi Josh, Currently it reports this error: But I could not figure out the cause for this error as we haven't called the method "trace" in the mutationobserver.rs file that is reported in the error. Could you please help us in figuring out the cause for this error and how to fix it? |
jdm
reviewed
May 1, 2017
| use std::rc::Rc; | ||
|
|
||
| #[dom_struct] | ||
| pub struct MutationObserver { | ||
| reflector_: Reflector, | ||
| #[ignore_heap_size_of = "can't measure Rc values"] | ||
| callback: Rc<MutationCallback>, | ||
| record_queue: Vec<Root<MutationRecord>>, | ||
| #[ignore_heap_size_of = "can't measure DomRefCell values"] | ||
| options: Cell<MutationObserverInit>, |
There was a problem hiding this comment.
Rather than storing an instance of this, store individual fields that correspond to the members of MutationObserverInit that matter - want_attributes: Cell<bool>, want_attribute_old_value: Cell<bool>, and attribute_filter: DOMRefCell<Vec<DOMString>>.
jdm
requested changes
May 1, 2017
| } | ||
| } | ||
| // Step 5 | ||
| let pipelineId = PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }; |
There was a problem hiding this comment.
This value should be obtained via target.global().pipeline_id() instead.
| // // https://dom.spec.whatwg.org/#dom-mutationrecord-nextsibling | ||
| // fn GetNextSibling(&self) -> Option<Root<Node>> { | ||
| // self.next_sibling.get() | ||
| // } |
There was a problem hiding this comment.
No need to add this commented out code.
components/script/dom/node.rs
Outdated
| @@ -138,6 +140,9 @@ pub struct Node { | |||
| /// node is finalized. | |||
| style_and_layout_data: Cell<Option<OpaqueStyleAndLayoutData>>, | |||
|
|
|||
| /// vector of mutation observer objects. | |||
There was a problem hiding this comment.
/// Registered observers for this node.
components/script/dom/node.rs
Outdated
| @@ -349,7 +354,16 @@ impl Node { | |||
| kid.teardown(); | |||
| } | |||
| } | |||
| /// method on Node that returns Vec<Root<MutationObserver>> | |||
| pub fn registered_mutation_observers_for_type(&self) -> &DOMRefCell<Vec<MutationObserver>> { | |||
There was a problem hiding this comment.
Given that we don't pass any kind of type argument here, let's rename this to registered_mutation_observers.
components/script/dom/node.rs
Outdated
|
|
||
| /// add a registered observer | ||
| pub fn add_registered_mutation_observer(&self, observer: MutationObserver) -> Fallible<()> { |
There was a problem hiding this comment.
This should be a &MutationObserver argument, and it does not need a return value.
components/script/microtask.rs
Outdated
| // pub callback: Rc<MutationCallback>, | ||
| pub pipeline: PipelineId, | ||
| // pub mutations: Vec<Root<MutationRecord>>, | ||
| // pub observer: MutationObserver, |
There was a problem hiding this comment.
What's going on with these commented out members and the commented out code that uses them?
components/script/script_thread.rs
Outdated
| }) | ||
| } | ||
|
|
||
| pub fn get_mutation_observer_compound_microtask_queued() -> bool { |
There was a problem hiding this comment.
Let's call this is_mutation_observer_compound_microtask_queued.
components/script/script_thread.rs
Outdated
| pub fn get_mutation_observer() -> DOMRefCell<Vec<JS<MutationObserver>>> { | ||
| SCRIPT_THREAD_ROOT.with(|root| { | ||
| let script_thread = unsafe { &*root.get().unwrap() }; | ||
| return script_thread.mutation_observers.clone(); |
There was a problem hiding this comment.
We should be returning a reference instead of cloning here.
| let callback: Rc<MutationCallback>; | ||
| let observer: MutationObserver; | ||
| observer.options = Cell::from(*options); | ||
| target.add_registered_mutation_observer(observer); |
There was a problem hiding this comment.
Shouldn't be we adding self here? However, we should only add a new observer if no matching one was found in the list (hence otherwise in step 8).
| (given_namespace != ""); | ||
| let condition4: bool = given_name == "characterData" && | ||
| registered_observer.options.into_inner().characterData == Some(false); | ||
| let condition5: bool = given_name == "childList" && |
There was a problem hiding this comment.
Why are we comparing given_name here? Since we only support mutations on attributes, we only need to care about specification text that refers to the "type" variable being "attributes".
jdm
reviewed
May 1, 2017
| @@ -30,7 +30,6 @@ pub struct MutationRecord { | |||
| old_value: Option<DOMString>, | |||
| } | |||
|
|
|||
| #[allow(unrooted_must_root)] | |||
| impl MutationRecord { | |||
| pub fn new(record_type: DOMString, target: &Node) -> MutationRecord { | |||
There was a problem hiding this comment.
This function should be returning Root<MutationRecord> by calling reflect_dom_object, like the constructor for MutationObserver.
jdm
reviewed
May 1, 2017
| let enqueuedMutationCallback = EnqueuedMutationCallback { pipeline: target.global().pipeline_id() }; | ||
| let cx: *mut JSContext = JSContext::new(); | ||
| let callback: *mut JSObject = JSObject::new(); | ||
| let mutationCallback: Rc<MutationCallback> = MutationCallback::new(cx, callback); |
There was a problem hiding this comment.
The callback used should be the one stored as a member of MutationObserver.
There was a problem hiding this comment.
Hi Josh,
I have changed this by including a &self in the queue_a_mutation_record method's arguments and then calling it as:
let enqueuedMutationCallback = EnqueuedMutationCallback { callback: self.callback, pipeline: target.global().pipeline_id() };
but this gives the following error:
error[E0507]: cannot move out of borrowed content
--> /home/sumit/Dropbox/NCSU/OODD/FinalProject/servo/components/script/dom/mutationobserver.rs:170:77
|
170 | let enqueuedMutationCallback = EnqueuedMutationCallback { callback: self.callback,
| ^^^^ cannot move out of borrowed content
Also, the same error is there for microtask.rs (line 92) in the following line:
let observer = MutationObserver::Constructor(&global, job.callback);
error:
error[E0507]: cannot move out of borrowed content
--> /home/sumit/Dropbox/NCSU/OODD/FinalProject/servo/components/script/microtask.rs:92:83
|
92 | let observer = MutationObserver::Constructor(&global, job.callback);
| ^^^ cannot move out of borrowed content
Can you please help me out with this error?
There was a problem hiding this comment.
To best understand the issue, you should read the documentation about ownership. In this case, Rc values are trivially cloneable so you can use .clone() to avoid the error.
jdm
requested changes
May 2, 2017
|
|
||
| /// https://dom.spec.whatwg.org/#queue-a-mutation-observer-compound-microtask | ||
| /// Queue a Mutation Observer compound Microtask. | ||
| pub fn queueMutationObserverCompoundMicrotask(compoundMicrotask: Microtask) { |
There was a problem hiding this comment.
Rather than accepting a Microtask argument here, this method should create it. The different callers that invoke this method do not need to customize any behaviour for the resulting microtask.
| // Step 2 | ||
| let notifyList = ScriptThread::get_mutation_observer(); | ||
| // Step 3, Step 4 not needed as Servo doesn't implement anything related to slots yet. | ||
| // Step 5: Ignore the specific text about execute a compound microtask. |
There was a problem hiding this comment.
Sorry, I think I said something that was misinterpreted. We definitely need step 5 or nothing actually happens here and the mutation observers' callbacks are never invoked. Add a TODO for step 5.3, but the rest are necessary.
| let mut given_namespace = ""; | ||
| let mut old_value = ""; | ||
| // Step 1 | ||
| let mut interestedObservers: Vec<JS<MutationObserver>> = vec![]; |
There was a problem hiding this comment.
This needs to be Vec<Root<MutationObserver>>.
| (given_namespace != ""); | ||
| let condition4: bool = given_name == "characterData" && | ||
| registered_observer.character_data.get() == false; | ||
| let condition5: bool = given_name == "childList" && |
There was a problem hiding this comment.
Just to be clear about my earlier comments - given_name is entirely unrelated to the specification variable "type", so comparing it against values like "childList" or "attribute" does not make sense. given_name is the name of the attribute that was mutated; that is all.
There was a problem hiding this comment.
Hi Josh,
Sorry, but I am still a bit unclear on this. I understood that given_name is the value which is mutating and is stored in the Mutation enum that we created. We don't need to compare with this value.
What I could not figure out is where this value type is coming from?
Does it refer to the type stored in the MutationRecord?
and if that is the case, should we pass a MutationRecord in this method as an argument as well?
| registered.character_data_old_value.set(options.characterDataOldValue.unwrap()); | ||
| registered.child_list.set(options.childList); | ||
| registered.subtree.set(options.subtree); | ||
| registered.attribute_filter == DOMRefCell::new(options.attributeFilter.clone().unwrap()); |
There was a problem hiding this comment.
This does not do what you want. *registered.attribute_filter.borrow_mut() = ....
components/script/microtask.rs
Outdated
| @@ -28,6 +33,7 @@ pub struct MicrotaskQueue { | |||
| #[derive(JSTraceable, HeapSizeOf)] | |||
| pub enum Microtask { | |||
| Promise(EnqueuedPromiseCallback), | |||
| Mutation(EnqueuedMutationCallback), | |||
There was a problem hiding this comment.
Let's call this NotifyMutationObservers.
components/script/microtask.rs
Outdated
| @@ -71,6 +85,15 @@ impl MicrotaskQueue { | |||
| let _ = job.callback.Call_(&*target, ExceptionHandling::Report); | |||
| } | |||
| } | |||
| Microtask::Mutation(ref job) => { | |||
| if let Some(target) = target_provider(job.pipeline) { | |||
There was a problem hiding this comment.
Here we should be invoking Microtask::notifyMutationObservers, since this microtask should be implementing the task queued in step 3 of https://dom.spec.whatwg.org/#queue-a-mutation-observer-compound-microtask.
components/script/script_thread.rs
Outdated
| @@ -583,6 +600,14 @@ impl ScriptThread { | |||
| }) | |||
| } | |||
|
|
|||
| pub fn get_mutation_observer() -> &'static DOMRefCell<Vec<JS<MutationObserver>>> { | |||
There was a problem hiding this comment.
Let's do the following instead:
pub fn get_mutation_observer() -> DOMRefCell<Vec<Root<MutationObserver>>> {
SCRIPT_THREAD_ROOT.with(|root| {
let script_thread = unsafe { &*root.get().unwrap() };
let observers = script_thread.mutation_observers.iter().map(|o| Root::from_ref(&*o)).collect();
return observers;
})
}There was a problem hiding this comment.
Whoops, I miswrote that:
pub fn get_mutation_observer() -> Vec<Root<MutationObserver>> {
SCRIPT_THREAD_ROOT.with(|root| {
let script_thread = unsafe { &*root.get().unwrap() };
let observers = script_thread.mutation_observers.borrow().iter().map(|o| Root::from_ref(&**o)).collect();
return observers;
})
}| self.attribute_namespace = Some(attr_namespace); | ||
| } | ||
| // setter for attr_oldvalue | ||
| pub fn SetoldValue(&mut self, attr_oldvalue: DOMString) { |
There was a problem hiding this comment.
These should not be &mut self; instead, use &self and interior mutability by storing DOMRefCell<Option<DOMString>> and use *self.attribute_name.borrow_mut() = ....
|
|
||
| //https://dom.spec.whatwg.org/#queueing-a-mutation-record | ||
| //Queuing a mutation record | ||
| pub fn queue_a_mutation_record(&self, target: &Node, attr_type: Mutation) { |
There was a problem hiding this comment.
We shouldn't need &self here with the other changes requested.
|
The changes I've requested and suggested should remove the compilation errors that are currently reported. |
asajeffrey
reviewed
May 2, 2017
components/script/dom/attr.rs
Outdated
| let newValue = DOMString::from(value.to_string()); | ||
| let attributeSpec = Mutation::Attribute { name, namespace, oldValue, newValue }; | ||
| let callback: Rc<MutationCallback>; | ||
| let mo = MutationObserver::Constructor(window_from_node(owner.upcast::<Node>()).deref(), callback); |
There was a problem hiding this comment.
Why is a new MutationObserver being created here? Shouldn't setting an attribute notify the existing obervers, not create a new one?
There was a problem hiding this comment.
This is addressed by the comments I requested.
components/script/dom/attr.rs
Outdated
| let attributeSpec = Mutation::Attribute { name, namespace, oldValue, newValue }; | ||
| let callback: Rc<MutationCallback>; | ||
| let mo = MutationObserver::Constructor(window_from_node(owner.upcast::<Node>()).deref(), callback); | ||
| let moRef = mo.unwrap(); |
There was a problem hiding this comment.
If we really need to unwrap here, can we use expect rather than unwrap?
components/script/dom/element.rs
Outdated
| let mo = MutationObserver::Constructor(window_from_node(&self.node).deref(), callback); | ||
| let moRef = mo.unwrap(); | ||
| let observer: &MutationObserver = moRef.deref(); | ||
| MutationObserver::queue_a_mutation_record(observer, &self.node, attributeSpec); |
components/script/dom/element.rs
Outdated
| let mo = MutationObserver::Constructor(window_from_node(&self.node).deref(), callback); | ||
| let moRef = mo.unwrap(); | ||
| let observer: &MutationObserver = moRef.deref(); | ||
| MutationObserver::queue_a_mutation_record(observer, &self.node, attributeSpec); |
| let given_name = &*name.clone(); | ||
| let given_namespace = &*namespace.clone(); | ||
| let old_value = &*oldValue.clone(); | ||
| }, |
There was a problem hiding this comment.
This is defining new local variables (which immediately go out of scope), not mutating the existing variables.
| //Step 4.3-4.6- TODO currently not relevant to mutation records for attribute mutations | ||
| //Step 4.7 | ||
| if pairedStrings[i] != "" { | ||
| record.SetoldValue(pairedStrings[i].clone()); |
There was a problem hiding this comment.
Why do the indexes of interestedObserver and pairedStrings line up?
| let mut i = 0; | ||
| for observer in &interestedObservers { | ||
| //Step 4.1 | ||
| let mut record = &MutationRecord::new(DOMString::from(&*given_name), target); |
There was a problem hiding this comment.
Why the &? This will give record the type &Root<MutationRecord>. If you make MutationRecord use interior mutability, you won't need the mut annotation on this variable.
| // Step 1: If either options’ attributeOldValue or attributeFilter is present and | ||
| // options’ attributes is omitted, set options’ attributes to true. | ||
| if (options.attributeOldValue.is_some() || options.attributeFilter.is_some()) && options.attributes.is_none() { | ||
| // options.attributes = Some(true); |
There was a problem hiding this comment.
Should this be options.attributes.set(Some(true))? (and ditto the other setters)
components/script/dom/node.rs
Outdated
| @@ -349,7 +354,15 @@ impl Node { | |||
| kid.teardown(); | |||
| } | |||
| } | |||
| /// method on Node that returns Vec<Root<MutationObserver>> | |||
| pub fn registered_mutation_observers(&self) -> &DOMRefCell<Vec<JS<MutationObserver>>> { | |||
There was a problem hiding this comment.
It's difficult to make sure the dynamic checks in borrowing a DOMRefCell succeed if you let the cell escape like this. Better(?) to move any functionality which depends on the mutation observers into Node?
| @@ -9,7 +9,8 @@ | |||
| // https://dom.spec.whatwg.org/#mutationobserver | |||
| [Pref="dom.mutation_observer.enabled", Constructor(MutationCallback callback)] | |||
| interface MutationObserver { | |||
| //void observe(Node target, optional MutationObserverInit options); | |||
| [Throws] | |||
There was a problem hiding this comment.
Why the Throws marker? It's not in the whatwg idl: https://dom.spec.whatwg.org/#mutationobserver
There was a problem hiding this comment.
That's a necessary addition for any API that can throw exceptions.
|
@srivassumit To avoid confusion, please focus on addressing my previous review comments. Many/all of the ones from @asajeffrey will be made redundant after addressing mine. |
|
Sure Josh, |
jdm
added
S-needs-code-changes
highfive
added
S-awaiting-review
jdm
requested changes
May 3, 2017
components/script/dom/element.rs
Outdated
| @@ -96,6 +99,7 @@ use std::convert::TryFrom; | |||
| use std::default::Default; | |||
| use std::fmt; | |||
| use std::rc::Rc; | |||
| use std::sync::Arc; | |||
There was a problem hiding this comment.
You will need to revert this change; it looks like a rebase error to me.
|
Hooray, it compiles now \o/ Are you stuck now, or should I wait to review until the callback invocation code is implemented? |
|
Thanks @jdm , I am still trying to figure out how to implement the callback invocation code. I will push that in shortly as soon as I implement it. However, I am stuck on the part about given_name on mutationobserver.rs (line 130)... I am not sure which value does |
|
When the spec talks about checking |
|
okay, so just to be clear, Something like: and then check for those cases where the value of Is this understanding correct? |
|
That's a literal reading of the specification. What I'm saying is that you can either omit any check for type entirely in your implementation, or you can explicitly use the |
|
I tried implementing the callback invocation and changed the part where but I am facing some errors: for the callback invocation, it says: For the Can you please guide us here? |
|
💔 Test failed - mac-dev-unit |
|
hi @jdm Could you please advice on how should we proceed? I had updated the test expectations, but it says that tests have failed in the homu build. |
|
The failures come from |
jdm
requested changes
May 9, 2017
tests/unit/script/size_of.rs
Outdated
| sizeof_checker!(size_div, HTMLDivElement, 376); | ||
| sizeof_checker!(size_span, HTMLSpanElement, 376); | ||
| sizeof_checker!(size_text, Text, 216); | ||
| sizeof_checker!(size_characterdata, CharacterData, 216); |
There was a problem hiding this comment.
According to TravisCI, these numbers still aren't right. Maybe someone else updated the types in a different commit:
thread 'size_of::size_div' panicked at 'Your changes have decreased the stack size of commonly used DOM struct HTMLDivElement from 376 to 368. Good work! Please update the size in tests/unit/script/size_of.rs.', /home/travis/build/servo/servo/tests/unit/script/size_of.rs:36
thread 'size_of::size_element' panicked at 'Your changes have decreased the stack size of commonly used DOM struct Element from 360 to 352. Good work! Please update the size in tests/unit/script/size_of.rs.', /home/travis/build/servo/servo/tests/unit/script/size_of.rs:34
thread 'size_of::size_htmlelement' panicked at 'Your changes have decreased the stack size of commonly used DOM struct HTMLElement from 376 to 368. Good work! Please update the size in tests/unit/script/size_of.rs.', /home/travis/build/servo/servo/tests/unit/script/size_of.rs:35
thread 'size_of::size_span' panicked at 'Your changes have decreased the stack size of commonly used DOM struct HTMLSpanElement from 376 to 368. Good work! Please update the size in tests/unit/script/size_of.rs.', /home/travis/build/servo/servo/tests/unit/script/size_of.rs:37
|
hi @jdm Should I just squash all of my commits in my MutationObserver branch and push it as a single commit? |
|
That would be great, @srivassumit! |
srivassumit
force-pushed
the
MutationObserver
branch
from
f59492a to
7d3d1f6
Compare
|
Hi @jdm, It seems that there was some rebase error and now I am getting merge conflicts.. I am working on resolving these.. then I'll push a new squashed commit. |
srivassumit
force-pushed
the
MutationObserver
branch
from
7d3d1f6 to
73f4c88
Compare
|
hi @jdm, All checks have passed now, can you please review once if it can be merged? |
|
Hi, I'll do some tests using the Mutation Observer API to run AFrame demos. Some compilation warnings should be fixed: |
|
@MortimerGoro Note that only attribute mutations are observable with these changes. |
srivassumit
force-pushed
the
MutationObserver
branch
from
73f4c88 to
515dead
Compare
|
I have fixed the warnings. |
|
Thanks @srivassumit
We need support for childNodes and subtree observers. Is there any plan to work on these? I don't see it in https://github.com/servo/servo/wiki/Mutation-observer-project. I'm blocked by this feature to run AFrame apps on Servo. The other alternatives are implementing Custom Elements but it's planned for a GSoC 2017 project, or implementing the Mutation events/DOMAttrModified which is a deprecated API. We'd like show some AFrame demos running on Servo in the upcoming All Hands. As there isn't much time left, is it a problem if I work on adding the required observers? I wouldn't like to affect assigned work or GSoC projects. |
|
Yes, I have always had plans for getting the rest of the MutationObserver API working by the end of Q2. That being said, if you wanted to implement the missing pieces that you need, that would not cause any problems. I'm planning to merge this code with some small changes today. |
|
Ok, I'll work on adding the required pieces to run AFrame ;) |
|
☔ The latest upstream changes (presumably #16295) made this pull request unmergeable. Please resolve the merge conflicts. |
|
For posterity, this is the full set of changes I've applied in #16883: diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs
index b52d77d..af5f010 100644
--- a/components/script/dom/attr.rs
+++ b/components/script/dom/attr.rs
@@ -10,8 +10,7 @@ use dom::bindings::js::{LayoutJS, MutNullableJS, Root, RootedReference};
use dom::bindings::reflector::{Reflector, reflect_dom_object};
use dom::bindings::str::DOMString;
use dom::element::{AttributeMutation, Element};
-use dom::mutationobserver::Mutation;
-use dom::mutationobserver::MutationObserver;
+use dom::mutationobserver::{Mutation, MutationObserver};
use dom::node::Node;
use dom::virtualmethods::vtable_for;
use dom::window::Window;
@@ -173,13 +172,11 @@ impl AttrMethods for Attr {
impl Attr {
pub fn set_value(&self, mut value: AttrValue, owner: &Element) {
- let name = Atom::from(self.local_name().to_string());
- let namespace = Namespace::from(self.namespace().to_string());
- let oldValue = DOMString::from(self.value().to_string());
- let newValue = DOMString::from(value.to_string());
- let attributeSpec = Mutation::Attribute { name, namespace, oldValue, newValue };
-
- MutationObserver::queue_a_mutation_record(owner.upcast::<Node>(), attributeSpec);
+ let name = self.local_name().clone();
+ let namespace = self.namespace().clone();
+ let old_value = DOMString::from(&**self.value());
+ let mutation = Mutation::Attribute { name, namespace, old_value };
+ MutationObserver::queue_a_mutation_record(owner.upcast::<Node>(), mutation);
assert!(Some(owner) == self.owner().r());
owner.will_mutate_attr(self);
diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs
index b490202..26c9a14 100644
--- a/components/script/dom/element.rs
+++ b/components/script/dom/element.rs
@@ -3,6 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
//! Element nodes.
+
use cssparser::Color;
use devtools_traits::AttrInfo;
use dom::activation::Activatable;
@@ -62,8 +63,7 @@ use dom::htmltablerowelement::{HTMLTableRowElement, HTMLTableRowElementLayoutHel
use dom::htmltablesectionelement::{HTMLTableSectionElement, HTMLTableSectionElementLayoutHelpers};
use dom::htmltemplateelement::HTMLTemplateElement;
use dom::htmltextareaelement::{HTMLTextAreaElement, LayoutHTMLTextAreaElementHelpers};
-use dom::mutationobserver::Mutation;
-use dom::mutationobserver::MutationObserver;
+use dom::mutationobserver::{Mutation, MutationObserver};
use dom::namednodemap::NamedNodeMap;
use dom::node::{CLICK_IN_PROGRESS, ChildrenMutation, LayoutNodeHelpers, Node};
use dom::node::{NodeDamage, SEQUENTIALLY_FOCUSABLE, UnbindContext};
@@ -1006,12 +1006,12 @@ impl Element {
}
pub fn push_attribute(&self, attr: &Attr) {
- let name = Atom::from(attr.local_name().to_string());
- let namespace = Namespace::from(attr.namespace().to_string());
- let oldValue = DOMString::from(attr.value().to_string());
- let newValue = DOMString::from(attr.value().to_string());
- let attributeSpec = Mutation::Attribute { name, namespace, oldValue, newValue };
- MutationObserver::queue_a_mutation_record(&self.node, attributeSpec);
+ let name = attr.local_name().clone();
+ let namespace = attr.namespace().clone();
+ let old_value = DOMString::from(&**attr.value());
+ let mutation = Mutation::Attribute { name, namespace, old_value };
+ MutationObserver::queue_a_mutation_record(&self.node, mutation);
+
assert!(attr.GetOwnerElement().r() == Some(self));
self.will_mutate_attr(attr);
self.attrs.borrow_mut().push(JS::from_ref(attr));
@@ -1139,12 +1139,13 @@ impl Element {
idx.map(|idx| {
let attr = Root::from_ref(&*(*self.attrs.borrow())[idx]);
self.will_mutate_attr(&attr);
- let name = Atom::from(attr.local_name().to_string());
- let namespace = Namespace::from(attr.namespace().to_string());
- let oldValue = DOMString::from(attr.value().to_string());
- let newValue = DOMString::from(attr.value().to_string());
- let attributeSpec = Mutation::Attribute { name, namespace, oldValue, newValue };
- MutationObserver::queue_a_mutation_record(&self.node, attributeSpec);
+
+ let name = attr.local_name().clone();
+ let namespace = attr.namespace().clone();
+ let old_value = DOMString::from(&**attr.value());
+ let mutation = Mutation::Attribute { name, namespace, old_value, };
+ MutationObserver::queue_a_mutation_record(&self.node, mutation);
+
self.attrs.borrow_mut().remove(idx);
attr.set_owner(None);
if attr.namespace() == &ns!() {
diff --git a/components/script/dom/mutationobserver.rs b/components/script/dom/mutationobserver.rs
index 349254d..b174ae4 100644
--- a/components/script/dom/mutationobserver.rs
+++ b/components/script/dom/mutationobserver.rs
@@ -1,8 +1,7 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-#![allow(unused_variables)]
-use core::ops::Deref;
+
use dom::bindings::callback::ExceptionHandling;
use dom::bindings::cell::DOMRefCell;
use dom::bindings::codegen::Bindings::MutationObserverBinding;
@@ -10,18 +9,16 @@ use dom::bindings::codegen::Bindings::MutationObserverBinding::MutationCallback;
use dom::bindings::codegen::Bindings::MutationObserverBinding::MutationObserverBinding::MutationObserverMethods;
use dom::bindings::codegen::Bindings::MutationObserverBinding::MutationObserverInit;
use dom::bindings::error::{Error, Fallible};
-use dom::bindings::js::Root;
+use dom::bindings::js::{JS, Root};
use dom::bindings::reflector::{Reflector, reflect_dom_object};
use dom::bindings::str::DOMString;
use dom::mutationrecord::MutationRecord;
use dom::node::Node;
use dom::window::Window;
use dom_struct::dom_struct;
-use html5ever::Namespace;
+use html5ever::{Namespace, LocalName};
use microtask::Microtask;
use script_thread::ScriptThread;
-use servo_atoms::Atom;
-use std::cell::{Cell, Ref};
use std::rc::Rc;
#[dom_struct]
@@ -30,18 +27,28 @@ pub struct MutationObserver {
#[ignore_heap_size_of = "can't measure Rc values"]
callback: Rc<MutationCallback>,
record_queue: DOMRefCell<Vec<Root<MutationRecord>>>,
- attribute_old_value: Cell<bool>,
- attributes: Cell<bool>,
- character_data: Cell<bool>,
- character_data_old_value: Cell<bool>,
- child_list: Cell<bool>,
- subtree: Cell<bool>,
- attribute_filter: DOMRefCell<Vec<DOMString>>,
}
#[derive(Clone)]
pub enum Mutation {
- Attribute { name: Atom, namespace: Namespace, oldValue: DOMString, newValue: DOMString}
+ Attribute { name: LocalName, namespace: Namespace, old_value: DOMString }
+}
+
+#[derive(HeapSizeOf, JSTraceable)]
+pub struct RegisteredObserver {
+ observer: Root<MutationObserver>,
+ options: ObserverOptions,
+}
+
+#[derive(HeapSizeOf, JSTraceable)]
+pub struct ObserverOptions {
+ attribute_old_value: bool,
+ attributes: bool,
+ character_data: bool,
+ character_data_old_value: bool,
+ child_list: bool,
+ subtree: bool,
+ attribute_filter: Vec<DOMString>,
}
impl MutationObserver {
@@ -55,13 +62,6 @@ impl MutationObserver {
reflector_: Reflector::new(),
callback: callback,
record_queue: DOMRefCell::new(vec![]),
- attribute_filter: DOMRefCell::new(vec![]),
- attribute_old_value: Cell::from(false),
- attributes: Cell::from(false),
- character_data: Cell::from(false),
- character_data_old_value: Cell::from(false),
- child_list: Cell::from(false),
- subtree: Cell::from(false),
}
}
@@ -72,8 +72,7 @@ impl MutationObserver {
}
/// https://dom.spec.whatwg.org/#queue-a-mutation-observer-compound-microtask
- /// Queue a Mutation Observer compound Microtask.
- pub fn queueMutationObserverCompoundMicrotask() {
+ pub fn queue_mutation_observer_compound_microtask() {
// Step 1
if ScriptThread::is_mutation_observer_compound_microtask_queued() {
return;
@@ -81,175 +80,174 @@ impl MutationObserver {
// Step 2
ScriptThread::set_mutation_observer_compound_microtask_queued(true);
// Step 3
- let compoundMicrotask = Microtask::NotifyMutationObservers();
- ScriptThread::enqueue_microtask(compoundMicrotask);
+ ScriptThread::enqueue_microtask(Microtask::NotifyMutationObservers);
}
/// https://dom.spec.whatwg.org/#notify-mutation-observers
- /// Notify Mutation Observers.
- pub fn notifyMutationObservers() -> Fallible<()> {
+ pub fn notify_mutation_observers() {
// Step 1
ScriptThread::set_mutation_observer_compound_microtask_queued(false);
// Step 2
- let notifyList = ScriptThread::get_mutation_observer();
- // Step 3, Step 4 not needed as Servo doesn't implement anything related to slots yet.
- // Step 5: Ignore the specific text about execute a compound microtask.
- for (_, mo) in notifyList.iter().enumerate() {
+ let notify_list = ScriptThread::get_mutation_observers();
+ // TODO: steps 3-4 (slots)
+ // Step 5
+ for mo in ¬ify_list {
let queue: Vec<Root<MutationRecord>> = mo.record_queue.borrow().clone();
- *mo.record_queue.borrow_mut() = Vec::new();
+ mo.record_queue.borrow_mut().clear();
// TODO: Step 5.3 Remove all transient registered observers whose observer is mo.
if !queue.is_empty() {
let _ = mo.callback.Call_(&**mo, queue, &**mo, ExceptionHandling::Report);
}
}
- // Step 6 not needed as Servo doesn't implement anything related to slots yet.
- Ok(())
+ // TODO: Step 6 (slot signals)
}
- //https://dom.spec.whatwg.org/#queueing-a-mutation-record
- //Queuing a mutation record
+ /// https://dom.spec.whatwg.org/#queueing-a-mutation-record
pub fn queue_a_mutation_record(target: &Node, attr_type: Mutation) {
// Step 1
- let mut interestedObservers: Vec<(Root<MutationObserver>, DOMString)> = vec![];
- // Step 2
- let mut nodes: Vec<Root<Node>> = vec![];
- for ancestor in target.inclusive_ancestors() {
- nodes.push(ancestor);
- }
- // Step 3
- for node in &nodes {
- for registered_observer in node.registered_mutation_observers().borrow().iter() {
+ let mut interestedObservers: Vec<(Root<MutationObserver>, Option<DOMString>)> = vec![];
+ // Step 2 & 3
+ for node in target.inclusive_ancestors() {
+ for registered in &*node.registered_mutation_observers() {
+ if &*node != target && !registered.options.subtree {
+ continue;
+ }
+
match attr_type {
- // TODO check for Mutations other than Attribute
- Mutation::Attribute { ref name, ref namespace, ref oldValue, ref newValue } => {
- let condition1: bool = node != &Root::from_ref(target) &&
- !registered_observer.subtree.get();
- let condition2: bool = registered_observer.attributes.get() == false;
- let condition3: bool = !registered_observer.attribute_filter.borrow().is_empty() &&
- (registered_observer.attribute_filter.borrow().iter()
- .find(|s| &**s == &**name).is_none() || namespace != &ns!());
- if !condition1 && !condition2 && !condition3 {
- // Step 3.1.2
- let paired_string = if registered_observer.attribute_old_value.get() {
- DOMString::from(oldValue.deref())
- } else {
- DOMString::from("")
- };
- // Step 3.1.1
- let idx = interestedObservers.iter().position(|&(ref o, _)|
- &**o as *const MutationObserver == &**registered_observer as *const MutationObserver);
- if let Some(idx) = idx {
- interestedObservers[idx].1 = paired_string;
- } else {
- interestedObservers.push((Root::from_ref(registered_observer), paired_string));
+ Mutation::Attribute { ref name, ref namespace, ref old_value } => {
+ // Step 3.1
+ if !registered.options.attributes {
+ continue;
+ }
+ if !registered.options.attribute_filter.is_empty() {
+ if *namespace != ns!() {
+ continue;
}
+ if registered.options.attribute_filter.iter()
+ .find(|s| &**s == &**name).is_some() {
+ continue;
+ }
+ }
+ // Step 3.1.2
+ let paired_string = if registered.options.attribute_old_value {
+ Some(old_value.clone())
+ } else {
+ None
+ };
+ // Step 3.1.1
+ let idx = interestedObservers.iter().position(|&(ref o, _)|
+ &**o as *const _ == &*registered.observer as *const _);
+ if let Some(idx) = idx {
+ interestedObservers[idx].1 = paired_string;
+ } else {
+ interestedObservers.push((Root::from_ref(&*registered.observer),
+ paired_string));
}
}
}
}
}
+
// Step 4
for &(ref observer, ref paired_string) in &interestedObservers {
- // Step 4.1
- let record = &MutationRecord::new(DOMString::from("attributes"), target);
- // Step 4.2
- match attr_type {
- Mutation::Attribute { ref name, ref namespace, ref oldValue, ref newValue } => {
- record.SetAttributeName(DOMString::from(&**name));
- if namespace != &ns!() {
- record.SetAttributeNamespace(DOMString::from(&**namespace));
- }
+ // Steps 4.1-4.7
+ let record = match attr_type {
+ Mutation::Attribute { ref name, ref namespace, .. } => {
+ let namespace = if *namespace != ns!() {
+ Some(namespace)
+ } else {
+ None
+ };
+ MutationRecord::attribute_mutated(target, name, namespace, paired_string.clone())
}
- }
- // Step 4.3-4.6- TODO currently not relevant to mutation records for attribute mutations
- // Step 4.7
- if paired_string != "" {
- record.SetoldValue(paired_string.clone());
- }
+ };
// Step 4.8
- observer.record_queue.borrow_mut().push(record.clone());
+ observer.record_queue.borrow_mut().push(record);
}
+
// Step 5
- MutationObserver::queueMutationObserverCompoundMicrotask();
+ MutationObserver::queue_mutation_observer_compound_microtask();
}
}
impl MutationObserverMethods for MutationObserver {
/// https://dom.spec.whatwg.org/#dom-mutationobserver-observe
- /// MutationObserver.observe method
fn Observe(&self, target: &Node, options: &MutationObserverInit) -> Fallible<()> {
- let mut options_cp: MutationObserverInit = MutationObserverInit {
- attributeFilter: options.attributeFilter.clone(),
- attributeOldValue: options.attributeOldValue,
- attributes: options.attributes,
- characterData: options.characterData,
- characterDataOldValue: options.characterDataOldValue,
- childList: options.childList,
- subtree: options.subtree
- };
- // Step 1: If either options’ attributeOldValue or attributeFilter is present and
- // options’ attributes is omitted, set options’ attributes to true.
- if (options_cp.attributeOldValue.is_some() || options_cp.attributeFilter.is_some()) &&
- options_cp.attributes.is_none() {
- options_cp.attributes = Some(true);
+ let attribute_filter = options.attributeFilter.clone().unwrap_or(vec![]);
+ let attribute_old_value = options.attributeOldValue.unwrap_or(false);
+ let mut attributes = options.attributes.unwrap_or(false);
+ let mut character_data = options.characterData.unwrap_or(false);
+ let character_data_old_value = options.characterDataOldValue.unwrap_or(false);
+ let child_list = options.childList;
+ let subtree = options.subtree;
+
+ // Step 1
+ if (options.attributeOldValue.is_some() || options.attributeFilter.is_some()) &&
+ options.attributes.is_none() {
+ attributes = true;
}
- // Step2: If options’ characterDataOldValue is present and options’ characterData is omitted,
- // set options’ characterData to true.
- if options_cp.characterDataOldValue.is_some() && options_cp.characterData.is_some() {
- options_cp.characterData = Some(true);
+
+ // Step 2
+ if options.characterDataOldValue.is_some() && options.characterData.is_none() {
+ character_data = true;
}
- // Step3: If none of options’ childList, attributes, and characterData is true, throw a TypeError.
- if !options_cp.childList && !options_cp.attributes.unwrap_or(false) &&
- !options_cp.characterData.unwrap_or(false) {
- return Err(Error::Type("childList, attributes, and characterData not true".to_owned()));
+
+ // Step 3
+ if !child_list && !attributes && !character_data {
+ return Err(Error::Type("One of childList, attributes, or characterData must be true".into()));
}
- // Step4: If options’ attributeOldValue is true and options’ attributes is false, throw a TypeError.
- if options_cp.attributeOldValue.unwrap_or(false) && !options_cp.attributes.unwrap_or(false) {
- return Err(Error::Type("attributeOldValue is true but attributes is false".to_owned()));
+
+ // Step 4
+ if attribute_old_value && !attributes {
+ return Err(Error::Type("attributeOldValue is true but attributes is false".into()));
}
- // Step5: If options’ attributeFilter is present and options’ attributes is false, throw a TypeError.
- if options_cp.attributeFilter.is_some() && !options_cp.attributes.unwrap_or(false) {
- return Err(Error::Type("attributeFilter is present but attributes is false".to_owned()));
+
+ // Step 5
+ if options.attributeFilter.is_some() && !attributes {
+ return Err(Error::Type("attributeFilter is present but attributes is false".into()));
}
- // Step6: If options’ characterDataOldValue is true and options’ characterData is false, throw a TypeError.
- if options_cp.characterDataOldValue.unwrap_or(false) && !options_cp.characterData.unwrap_or(false) {
- return Err(Error::Type("characterDataOldValue is true but characterData is false".to_owned()));
+
+ // Step 6
+ if character_data_old_value && !character_data {
+ return Err(Error::Type("characterDataOldValue is true but characterData is false".into()));
}
+
// Step 7
- for registered in target.registered_mutation_observers().borrow().iter() {
- if &**registered as *const MutationObserver == self as *const MutationObserver {
+ let add_new_observer = {
+ let mut replaced = false;
+ for registered in &mut *target.registered_mutation_observers() {
+ if &*registered.observer as *const MutationObserver != self as *const MutationObserver {
+ continue;
+ }
// TODO: remove matching transient registered observers
- registered.attribute_old_value.set(options_cp.attributeOldValue.unwrap_or(false));
- registered.attributes.set(options_cp.attributes.unwrap_or(false));
- registered.character_data.set(options_cp.characterData.unwrap_or(false));
- registered.character_data_old_value.set(options_cp.characterDataOldValue.unwrap_or(false));
- registered.child_list.set(options_cp.childList);
- registered.subtree.set(options_cp.subtree);
- *registered.attribute_filter.borrow_mut() = options_cp.attributeFilter.clone().unwrap();
- }
- }
- // Step 8
- let observers = target.registered_mutation_observers();
- let idx = observers.borrow().iter().position(|registered| {
- &**registered as *const MutationObserver == self as *const MutationObserver
- });
- let registered = match idx {
- Some(idx) => Ref::map(observers.borrow(), |obs| &*obs[idx]),
- None => {
- target.add_registered_mutation_observer(self);
- let idx = observers.borrow().len() - 1;
- Ref::map(observers.borrow(), |obs| &*obs[idx])
+ registered.options.attribute_old_value = attribute_old_value;
+ registered.options.attributes = attributes;
+ registered.options.character_data = character_data;
+ registered.options.character_data_old_value = character_data_old_value;
+ registered.options.child_list = child_list;
+ registered.options.subtree = subtree;
+ registered.options.attribute_filter = attribute_filter.clone();
+ replaced = true;
}
+ !replaced
};
- // TODO: remove matching transient registered observers
- registered.attribute_old_value.set(options_cp.attributeOldValue.unwrap_or(false));
- registered.attributes.set(options_cp.attributes.unwrap_or(false));
- registered.character_data.set(options_cp.characterData.unwrap_or(false));
- registered.character_data_old_value.set(options_cp.characterDataOldValue.unwrap_or(false));
- registered.child_list.set(options_cp.childList);
- registered.subtree.set(options_cp.subtree);
- *registered.attribute_filter.borrow_mut() = options_cp.attributeFilter.clone().unwrap_or(vec![]);
+
+ // Step 8
+ if add_new_observer {
+ target.registered_mutation_observers().push(RegisteredObserver {
+ observer: Root::from_ref(self),
+ options: ObserverOptions {
+ attributes,
+ attribute_old_value,
+ character_data,
+ character_data_old_value,
+ subtree,
+ attribute_filter,
+ child_list
+ },
+ });
+ }
Ok(())
}
diff --git a/components/script/dom/mutationrecord.rs b/components/script/dom/mutationrecord.rs
index efd186e..439f4ae 100644
--- a/components/script/dom/mutationrecord.rs
+++ b/components/script/dom/mutationrecord.rs
@@ -2,9 +2,6 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-use core::default::Default;
-use core::ops::Deref;
-use dom::bindings::cell::DOMRefCell;
use dom::bindings::codegen::Bindings::MutationRecordBinding::MutationRecordBinding;
use dom::bindings::codegen::Bindings::MutationRecordBinding::MutationRecordBinding::MutationRecordMethods;
use dom::bindings::js::{JS, Root};
@@ -13,57 +10,46 @@ use dom::bindings::str::DOMString;
use dom::node::{Node, window_from_node};
use dom::nodelist::NodeList;
use dom_struct::dom_struct;
+use html5ever::{LocalName, Namespace};
#[dom_struct]
pub struct MutationRecord {
reflector_: Reflector,
-
- //property for record type
record_type: DOMString,
-
- //property for target node
target: JS<Node>,
-
- //property for attribute name
- attribute_name: DOMRefCell<Option<DOMString>>,
-
- //property for attribute namespace
- attribute_namespace: DOMRefCell<Option<DOMString>>,
-
- //property for old value
- old_value: DOMRefCell<Option<DOMString>>,
+ attribute_name: Option<DOMString>,
+ attribute_namespace: Option<DOMString>,
+ old_value: Option<DOMString>,
}
impl MutationRecord {
- pub fn new(record_type: DOMString, target: &Node) -> Root<MutationRecord> {
- let boxed_record = box MutationRecord::new_inherited(record_type, target);
- return reflect_dom_object(boxed_record, window_from_node(target).deref(),
- MutationRecordBinding::Wrap);
+ #[allow(unrooted_must_root)]
+ pub fn attribute_mutated(target: &Node,
+ attribute_name: &LocalName,
+ attribute_namespace: Option<&Namespace>,
+ old_value: Option<DOMString>) -> Root<MutationRecord> {
+ let record = box MutationRecord::new_inherited("attributes",
+ target,
+ Some(DOMString::from(&**attribute_name)),
+ attribute_namespace.map(|n| DOMString::from(&**n)),
+ old_value);
+ reflect_dom_object(record, &*window_from_node(target), MutationRecordBinding::Wrap)
}
- fn new_inherited(record_type: DOMString, target: &Node) -> MutationRecord {
+ fn new_inherited(record_type: &str,
+ target: &Node,
+ attribute_name: Option<DOMString>,
+ attribute_namespace: Option<DOMString>,
+ old_value: Option<DOMString>) -> MutationRecord {
MutationRecord {
reflector_: Reflector::new(),
- record_type: record_type,
+ record_type: DOMString::from(record_type),
target: JS::from_ref(target),
- attribute_name: Default::default(),
- attribute_namespace: Default::default(),
- old_value: Default::default(),
+ attribute_name: attribute_name,
+ attribute_namespace: attribute_namespace,
+ old_value: old_value,
}
}
-
- // setter for attr_name
- pub fn SetAttributeName(&self, attr_name: DOMString) {
- *self.attribute_name.borrow_mut() = Some(attr_name);
- }
- // setter for attr_namespace
- pub fn SetAttributeNamespace(&self, attr_namespace: DOMString) {
- *self.attribute_namespace.borrow_mut() = Some(attr_namespace);
- }
- // setter for oldvalue
- pub fn SetoldValue(&self, attr_oldvalue: DOMString) {
- *self.old_value.borrow_mut() = Some(attr_oldvalue);
- }
}
impl MutationRecordMethods for MutationRecord {
@@ -74,33 +60,33 @@ impl MutationRecordMethods for MutationRecord {
// https://dom.spec.whatwg.org/#dom-mutationrecord-target
fn Target(&self) -> Root<Node> {
- return Root::from_ref(&*self.target);
+ Root::from_ref(&*self.target)
}
// https://dom.spec.whatwg.org/#dom-mutationrecord-attributename
fn GetAttributeName(&self) -> Option<DOMString> {
- self.attribute_name.borrow().clone()
+ self.attribute_name.clone()
}
// https://dom.spec.whatwg.org/#dom-mutationrecord-attributenamespace
fn GetAttributeNamespace(&self) -> Option<DOMString> {
- self.attribute_namespace.borrow().clone()
+ self.attribute_namespace.clone()
}
// https://dom.spec.whatwg.org/#dom-mutationrecord-oldvalue
fn GetOldValue(&self) -> Option<DOMString> {
- self.old_value.borrow().clone()
+ self.old_value.clone()
}
// https://dom.spec.whatwg.org/#dom-mutationrecord-addednodes
fn AddedNodes(&self) -> Root<NodeList> {
- let window = window_from_node(self.target.deref());
+ let window = window_from_node(&*self.target);
NodeList::empty(&window)
}
// https://dom.spec.whatwg.org/#dom-mutationrecord-removednodes
fn RemovedNodes(&self) -> Root<NodeList> {
- let window = window_from_node(self.target.deref());
+ let window = window_from_node(&*self.target);
NodeList::empty(&window)
}
diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
index f8fe3b3..42a9764 100644
--- a/components/script/dom/node.rs
+++ b/components/script/dom/node.rs
@@ -47,7 +47,7 @@ use dom::htmllinkelement::HTMLLinkElement;
use dom::htmlmetaelement::HTMLMetaElement;
use dom::htmlstyleelement::HTMLStyleElement;
use dom::htmltextareaelement::{HTMLTextAreaElement, LayoutHTMLTextAreaElementHelpers};
-use dom::mutationobserver::MutationObserver;
+use dom::mutationobserver::RegisteredObserver;
use dom::nodelist::NodeList;
use dom::processinginstruction::ProcessingInstruction;
use dom::range::WeakRangeVec;
@@ -74,7 +74,7 @@ use selectors::matching::matches_selector_list;
use selectors::parser::SelectorList;
use servo_url::ServoUrl;
use std::borrow::ToOwned;
-use std::cell::{Cell, UnsafeCell};
+use std::cell::{Cell, UnsafeCell, RefMut};
use std::cmp::max;
use std::default::Default;
use std::iter;
@@ -141,7 +141,7 @@ pub struct Node {
style_and_layout_data: Cell<Option<OpaqueStyleAndLayoutData>>,
/// Registered observers for this node.
- mutation_observers: DOMRefCell<Vec<Root<MutationObserver>>>,
+ mutation_observers: DOMRefCell<Vec<RegisteredObserver>>,
unique_id: UniqueId,
}
@@ -368,14 +368,9 @@ impl Node {
}
}
- /// method on Node that returns Vec<Root<MutationObserver>>
- pub fn registered_mutation_observers(&self) -> &DOMRefCell<Vec<Root<MutationObserver>>> {
- return &self.mutation_observers;
- }
-
- /// add a registered observer
- pub fn add_registered_mutation_observer(&self, observer: &MutationObserver) {
- self.mutation_observers.borrow_mut().push(Root::from_ref(observer));
+ /// Return all registered mutation observers for this node.
+ pub fn registered_mutation_observers(&self) -> RefMut<Vec<RegisteredObserver>> {
+ self.mutation_observers.borrow_mut()
}
/// Dumps the subtree rooted at this node, for debugging.
diff --git a/components/script/microtask.rs b/components/script/microtask.rs
index 8b2941a..7fadf11 100644
--- a/components/script/microtask.rs
+++ b/components/script/microtask.rs
@@ -29,7 +29,7 @@ pub struct MicrotaskQueue {
#[derive(JSTraceable, HeapSizeOf)]
pub enum Microtask {
Promise(EnqueuedPromiseCallback),
- NotifyMutationObservers(),
+ NotifyMutationObservers,
}
/// A promise callback scheduled to run during the next microtask checkpoint (#4283).
@@ -73,8 +73,8 @@ impl MicrotaskQueue {
let _ = job.callback.Call_(&*target, ExceptionHandling::Report);
}
}
- Microtask::NotifyMutationObservers() => {
- let _ = MutationObserver::notifyMutationObservers();
+ Microtask::NotifyMutationObservers => {
+ MutationObserver::notify_mutation_observers();
}
}
}
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 7e0147c..d687b54 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -485,7 +485,7 @@ pub struct ScriptThread {
mutation_observer_compound_microtask_queued: Cell<bool>,
/// The unit of related similar-origin browsing contexts' list of MutationObserver objects
- mutation_observers: DOMRefCell<Vec<Root<MutationObserver>>>,
+ mutation_observers: DOMRefCell<Vec<JS<MutationObserver>>>,
/// A handle to the webvr thread, if available
webvr_thread: Option<IpcSender<WebVRMsg>>,
@@ -596,15 +596,14 @@ impl ScriptThread {
let script_thread = unsafe { &*root.get().unwrap() };
script_thread.mutation_observers
.borrow_mut()
- .push(Root::from_ref(observer));
+ .push(JS::from_ref(observer));
})
}
- pub fn get_mutation_observer() -> Vec<Root<MutationObserver>> {
+ pub fn get_mutation_observers() -> Vec<Root<MutationObserver>> {
SCRIPT_THREAD_ROOT.with(|root| {
let script_thread = unsafe { &*root.get().unwrap() };
- let observers = script_thread.mutation_observers.borrow().iter().map(|o| Root::from_ref(&**o)).collect();
- return observers;
+ script_thread.mutation_observers.borrow().iter().map(|o| Root::from_ref(&**o)).collect()
})
}
diff --git a/tests/wpt/metadata/dom/nodes/Document-createElement.html.ini b/tests/wpt/metadata/dom/nodes/Document-createElement.html.ini
index 10a349d..1d72b6f 100644
--- a/tests/wpt/metadata/dom/nodes/Document-createElement.html.ini
+++ b/tests/wpt/metadata/dom/nodes/Document-createElement.html.ini
@@ -1,6 +1,5 @@
[Document-createElement.html]
type: testharness
- expected: TIMEOUT
[createElement(undefined) in XML document]
expected: FAIL
diff --git a/tests/wpt/metadata/dom/nodes/Document-createElementNS.html.ini b/tests/wpt/metadata/dom/nodes/Document-createElementNS.html.ini
index 91f1430..ff6e62d 100644
--- a/tests/wpt/metadata/dom/nodes/Document-createElementNS.html.ini
+++ b/tests/wpt/metadata/dom/nodes/Document-createElementNS.html.ini
@@ -1,6 +1,5 @@
[Document-createElementNS.html]
type: testharness
- expected: TIMEOUT
[createElementNS test in XML document: null,null,null]
expected: FAIL
diff --git a/tests/wpt/metadata/dom/nodes/Element-matches.html.ini b/tests/wpt/metadata/dom/nodes/Element-matches.html.ini
index 8eac1e9..1ab2c42 100644
--- a/tests/wpt/metadata/dom/nodes/Element-matches.html.ini
+++ b/tests/wpt/metadata/dom/nodes/Element-matches.html.ini
@@ -1,6 +1,5 @@
[Element-matches.html]
type: testharness
- expected: TIMEOUT
[In-document Element.matches: Universal selector, matching all children of the specified reference element (with refNode Element): >*]
expected: FAIL
diff --git a/tests/wpt/metadata/dom/nodes/Element-webkitMatchesSelector.html.ini b/tests/wpt/metadata/dom/nodes/Element-webkitMatchesSelector.html.ini
index 5d789d8..8806031 100644
--- a/tests/wpt/metadata/dom/nodes/Element-webkitMatchesSelector.html.ini
+++ b/tests/wpt/metadata/dom/nodes/Element-webkitMatchesSelector.html.ini
@@ -1,6 +1,5 @@
[Element-webkitMatchesSelector.html]
type: testharness
- expected: TIMEOUT
[In-document Element.webkitMatchesSelector: Descendant combinator '>>', matching element that is a descendant of an element with id (with no refNodes): #descendant>>div]
expected: FAIL
diff --git a/tests/wpt/metadata/dom/nodes/MutationObserver-attributes.html.ini b/tests/wpt/metadata/dom/nodes/MutationObserver-attributes.html.ini
index 0098ab9..52a6a92 100644
--- a/tests/wpt/metadata/dom/nodes/MutationObserver-attributes.html.ini
+++ b/tests/wpt/metadata/dom/nodes/MutationObserver-attributes.html.ini
@@ -21,3 +21,15 @@
[attributes Element.setAttributeNS: prefixed attribute creation mutation]
expected: FAIL
+ [attributes Element.className: empty string update mutation]
+ expected: FAIL
+
+ [attributes/attributeFilter Element.id/Element.className: update mutation]
+ expected: FAIL
+
+ [attributes/attributeFilter Element.id/Element.className: multiple filter update mutation]
+ expected: FAIL
+
+ [attributeFilter alone Element.id/Element.className: multiple filter update mutation]
+ expected: FAIL
+
diff --git a/tests/wpt/metadata/dom/nodes/MutationObserver-characterData.html.ini b/tests/wpt/metadata/dom/nodes/MutationObserver-characterData.html.ini
index 314752c..6e8512d 100644
--- a/tests/wpt/metadata/dom/nodes/MutationObserver-characterData.html.ini
+++ b/tests/wpt/metadata/dom/nodes/MutationObserver-characterData.html.ini
@@ -2,59 +2,59 @@
type: testharness
expected: TIMEOUT
[characterData Text.data: simple mutation without oldValue]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.data: simple mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.appendData: simple mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.appendData: empty string mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.appendData: null string mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.insertData: simple mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.insertData: empty string mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.insertData: null string mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.deleteData: simple mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.deleteData: empty mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.replaceData: simple mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Text.replaceData: empty mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData ProcessingInstruction: data mutations]
- expected: FAIL
+ expected: TIMEOUT
[characterData Comment: data mutations]
- expected: FAIL
+ expected: TIMEOUT
[characterData Range.deleteContents: child and data removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Range.deleteContents: child and data removal mutation (2)]
- expected: FAIL
+ expected: TIMEOUT
[characterData Range.extractContents: child and data removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[characterData Range.extractContents: child and data removal mutation (2)]
- expected: FAIL
+ expected: TIMEOUT
[characterData/characterDataOldValue alone Text.data: simple mutation]
- expected: FAIL
+ expected: TIMEOUT
diff --git a/tests/wpt/metadata/dom/nodes/MutationObserver-childList.html.ini b/tests/wpt/metadata/dom/nodes/MutationObserver-childList.html.ini
index 67d0746..91a9972 100644
--- a/tests/wpt/metadata/dom/nodes/MutationObserver-childList.html.ini
+++ b/tests/wpt/metadata/dom/nodes/MutationObserver-childList.html.ini
@@ -1,96 +1,90 @@
[MutationObserver-childList.html]
type: testharness
expected: TIMEOUT
- [childList Node.nodeValue: no mutation]
- expected: FAIL
-
[childList Node.textContent: replace content mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.textContent: no previous content mutation]
- expected: FAIL
-
- [childList Node.textContent: textContent no mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.textContent: empty string mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.normalize mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.normalize mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.insertBefore: addition mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.insertBefore: removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.insertBefore: removal and addition mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.insertBefore: fragment addition mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.insertBefore: fragment removal mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.insertBefore: last child addition mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.appendChild: addition mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.appendChild: removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.appendChild: removal and addition mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.appendChild: fragment addition mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.appendChild: fragment removal mutations]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.appendChild: addition outside document tree mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.replaceChild: replacement mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.replaceChild: removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.replaceChild: internal replacement mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.removeChild: removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.deleteContents: child removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.deleteContents: child and data removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.extractContents: child removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.extractContents: child and data removal mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.insertNode: child insertion mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.insertNode: children insertion mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Range.surroundContents: children removal and addition mutation]
- expected: FAIL
+ expected: TIMEOUT
[childList Node.replaceChild: self internal replacement mutation]
- expected: FAIL
+ expected: TIMEOUT
diff --git a/tests/wpt/metadata/dom/nodes/MutationObserver-inner-outer.html.ini b/tests/wpt/metadata/dom/nodes/MutationObserver-inner-outer.html.ini
index 110e26e..5c86ae9 100644
--- a/tests/wpt/metadata/dom/nodes/MutationObserver-inner-outer.html.ini
+++ b/tests/wpt/metadata/dom/nodes/MutationObserver-inner-outer.html.ini
@@ -5,8 +5,8 @@
expected: FAIL
[innerHTML with 2 children mutation]
- expected: FAIL
+ expected: TIMEOUT
[outerHTML mutation]
- expected: FAIL
+ expected: TIMEOUT
diff --git a/tests/wpt/metadata/dom/nodes/Node-isEqualNode-xhtml.xhtml.ini b/tests/wpt/metadata/dom/nodes/Node-isEqualNode-xhtml.xhtml.ini
index 1c8bde4..0690b87 100644
--- a/tests/wpt/metadata/dom/nodes/Node-isEqualNode-xhtml.xhtml.ini
+++ b/tests/wpt/metadata/dom/nodes/Node-isEqualNode-xhtml.xhtml.ini
@@ -1,6 +1,5 @@
[Node-isEqualNode-xhtml.xhtml]
type: testharness
- expected: TIMEOUT
[isEqualNode should return true when only the internal subsets of DocumentTypes differ.]
expected: FAIL
diff --git a/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All-xht.xht.ini b/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All-xht.xht.ini
index 8120e48..4ab75dc 100644
--- a/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All-xht.xht.ini
+++ b/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All-xht.xht.ini
@@ -1,6 +1,5 @@
[ParentNode-querySelector-All-xht.xht]
type: testharness
- expected: TIMEOUT
[Document.querySelectorAll: :first-line pseudo-element (one-colon syntax) selector, not matching any elements: #pseudo-element:first-line]
expected: FAIL
diff --git a/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini b/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini
index 00073a6..f40be05 100644
--- a/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini
+++ b/tests/wpt/metadata/dom/nodes/ParentNode-querySelector-All.html.ini
@@ -1,6 +1,5 @@
[ParentNode-querySelector-All.html]
type: testharness
- expected: TIMEOUT
[Document.querySelectorAll: :first-line pseudo-element (one-colon syntax) selector, not matching any elements: #pseudo-element:first-line]
expected: FAIL
|
bors-servo
pushed a commit
that referenced
this pull request
May 17, 2017
Mutation Observer API Rebased from #16668. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix (partially) #6633 - [X] There are tests for these changes <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16883) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This contains the changes for the subsequent steps of Implementing the Mutation Observer API as described in the Mutation Observer Project:
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is