From 5b1fe60277bdfe95f5f81754966d8eeec9bcb5e9 Mon Sep 17 00:00:00 2001 From: Rodion Borovyk Date: Mon, 29 Sep 2025 16:15:07 +0200 Subject: [PATCH] script: Empty pending mutation observers when notifying mutation observers (#39456) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty the surrounding agent’s pending mutation observers when notifying mutation observers according to the spec. Also, the code in the method MutationObserver::queue_a_mutation_record and the corresponding specification have diverged over the years. These changes bring the code into conformity with the specification. Testing: Added a new crash test Fixes: #39434 #39531 --------- Signed-off-by: Rodion Borovyk --- components/script/dom/mutationobserver.rs | 84 ++++++++++--------- .../script/script_mutation_observers.rs | 12 ++- tests/wpt/meta/MANIFEST.json | 7 ++ .../nodes/MutationObserver-nested-crash.html | 13 +++ 4 files changed, 74 insertions(+), 42 deletions(-) create mode 100644 tests/wpt/tests/dom/nodes/MutationObserver-nested-crash.html diff --git a/components/script/dom/mutationobserver.rs b/components/script/dom/mutationobserver.rs index 1d3bf5eb29e..4d498541063 100644 --- a/components/script/dom/mutationobserver.rs +++ b/components/script/dom/mutationobserver.rs @@ -3,6 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::cell::LazyCell; +use std::collections::HashMap; use std::rc::Rc; use dom_struct::dom_struct; @@ -105,31 +106,38 @@ impl MutationObserver { if !target.global().as_window().get_exists_mut_observer() { return; } - // Step 1 - let mut interested_observers: Vec<(DomRoot, Option)> = vec![]; + // Step 1 Let interestedObservers be an empty map. + let mut interested_observers: HashMap, Option> = + HashMap::new(); - // Step 2 & 3 + // Step 2 Let nodes be the inclusive ancestors of target. + // Step 3 For each node in nodes ... for node in target.inclusive_ancestors(ShadowIncluding::No) { let registered = node.registered_mutation_observers(); if registered.is_none() { continue; } + // Step 3 ... and then for each registered of node’s registered observer list: for registered in &*registered.unwrap() { + // 3.2 "1": node is not target and options["subtree"] is false if &*node != target && !registered.options.subtree { continue; } match *attr_type { + // 3.2 "2", "3" Mutation::Attribute { ref name, ref namespace, ref old_value, } => { - // Step 3.1 + // 3.1.2 "2": type is "attributes" and options["attributes"] either does not exist or is false if !registered.options.attributes { continue; } + // 3.1.2 "3": type is "attributes", options["attributeFilter"] exists, + // and options["attributeFilter"] does not contain name or namespace is non-null if !registered.options.attribute_filter.is_empty() { if *namespace != ns!() { continue; @@ -143,57 +151,51 @@ impl MutationObserver { continue; } } - // Step 3.1.2 - let paired_string = if registered.options.attribute_old_value { - old_value.clone() + // 3.2.1 Let mo be registered’s observer. + let mo = registered.observer.clone(); + // 3.2.2 If interestedObservers[mo] does not exist, then set interestedObservers[mo] to null. + if registered.options.attribute_old_value { + // 3.2.3 ... type is "attributes" and options["attributeOldValue"] is true ... + interested_observers.insert(mo, old_value.clone()); } else { - None - }; - // Step 3.1.1 - let idx = interested_observers - .iter() - .position(|(o, _)| std::ptr::eq(&**o, &*registered.observer)); - if let Some(idx) = idx { - interested_observers[idx].1 = paired_string; - } else { - interested_observers - .push((DomRoot::from_ref(&*registered.observer), paired_string)); + // 3.2.2 If interestedObservers[mo] does not exist, then set interestedObservers[mo] to null. + interested_observers.entry(mo).or_insert(None); } }, + // 3.2 "4" Mutation::CharacterData { ref old_value } => { + // 3.2 "4": type is "characterData" and options["characterData"] either does not exist or is false if !registered.options.character_data { continue; } - // Step 3.1.2 - let paired_string = if registered.options.character_data_old_value { - Some(old_value.clone()) + // 3.2.1 Let mo be registered’s observer. + let mo = registered.observer.clone(); + if registered.options.character_data_old_value { + // 3.2.3 ... type is "characterData" and options["characterDataOldValue"] is true + interested_observers.insert(mo, Some(old_value.clone())); } else { - None - }; - // Step 3.1.1 - let idx = interested_observers - .iter() - .position(|(o, _)| std::ptr::eq(&**o, &*registered.observer)); - if let Some(idx) = idx { - interested_observers[idx].1 = paired_string; - } else { - interested_observers - .push((DomRoot::from_ref(&*registered.observer), paired_string)); + // 3.2.2 If interestedObservers[mo] does not exist, then set interestedObservers[mo] to null. + interested_observers.entry(mo).or_insert(None); } }, + // 3.2 "5" Mutation::ChildList { .. } => { + // 3.2 "5": type is "childList" and options["childList"] is false if !registered.options.child_list { continue; } - interested_observers.push((DomRoot::from_ref(&*registered.observer), None)); + // 3.2.1 Let mo be registered’s observer. + let mo = registered.observer.clone(); + // 3.2.2 If interestedObservers[mo] does not exist, then set interestedObservers[mo] to null. + interested_observers.entry(mo).or_insert(None); }, } } } - // Step 4 - for (observer, paired_string) in interested_observers { - // Steps 4.1-4.7 + // Step 4 For each observer → mappedOldValue of interestedObservers: + for (observer, mapped_old_value) in interested_observers { + // Step 4.1 Let record be a new MutationRecord object ... let record = match *attr_type { Mutation::Attribute { ref name, @@ -209,12 +211,12 @@ impl MutationObserver { target, name, namespace, - paired_string, + mapped_old_value, CanGc::note(), ) }, Mutation::CharacterData { .. } => { - MutationRecord::character_data_mutated(target, paired_string, CanGc::note()) + MutationRecord::character_data_mutated(target, mapped_old_value, CanGc::note()) }, Mutation::ChildList { ref added, @@ -230,11 +232,13 @@ impl MutationObserver { CanGc::note(), ), }; - // Step 4.8 + // Step 4.2 Enqueue record to observer’s record queue. observer.record_queue.borrow_mut().push(record); + // Step 4.3 Append observer to the surrounding agent’s pending mutation observers. + ScriptThread::mutation_observers().add_mutation_observer(&observer); } - // Step 5 + // Step 5 Queue a mutation observer microtask. let mutation_observers = ScriptThread::mutation_observers(); mutation_observers.queue_mutation_observer_microtask(ScriptThread::microtask_queue()); } diff --git a/components/script/script_mutation_observers.rs b/components/script/script_mutation_observers.rs index 5134712d05e..919f663fb2f 100644 --- a/components/script/script_mutation_observers.rs +++ b/components/script/script_mutation_observers.rs @@ -43,8 +43,8 @@ impl ScriptMutationObservers { self.mutation_observer_microtask_queued.set(false); // Step 2. Let notifySet be a clone of the surrounding agent’s pending mutation observers. - // TODO Step 3. Empty the surrounding agent’s pending mutation observers. - let notify_list = self.mutation_observers.borrow(); + // Step 3. Empty the surrounding agent’s pending mutation observers. + let notify_list = self.take_mutation_observers(); // Step 4. Let signalSet be a clone of the surrounding agent’s signal slots. // Step 5. Empty the surrounding agent’s signal slots. @@ -110,4 +110,12 @@ impl ScriptMutationObservers { .map(|slot| slot.as_rooted()) .collect() } + + pub(crate) fn take_mutation_observers(&self) -> Vec> { + self.mutation_observers + .take() + .iter() + .map(|mo| mo.as_rooted()) + .collect() + } } diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 30bae85f41e..af2bf5d5d13 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -6984,6 +6984,13 @@ {} ] ], + "MutationObserver-nested-crash.html": [ + "0648c2037c8b2602ac77abde62f60793f0855a45", + [ + null, + {} + ] + ], "Node-cloneNode-on-inactive-document-crash.html": [ "cbd7a1e6a500e5671c1e0a71c5dcb963cab727ba", [ diff --git a/tests/wpt/tests/dom/nodes/MutationObserver-nested-crash.html b/tests/wpt/tests/dom/nodes/MutationObserver-nested-crash.html new file mode 100644 index 00000000000..0648c2037c8 --- /dev/null +++ b/tests/wpt/tests/dom/nodes/MutationObserver-nested-crash.html @@ -0,0 +1,13 @@ + + +MutationObservers: observer inside another observer's callback + +
+