diff --git a/core/src/avm1.rs b/core/src/avm1.rs index ff542a25d8a3..a79651554188 100644 --- a/core/src/avm1.rs +++ b/core/src/avm1.rs @@ -16,7 +16,7 @@ mod flv; mod fscommand; pub(crate) mod globals; mod object; -mod object_reference; +pub(crate) mod object_reference; mod parameters; mod property; mod property_map; diff --git a/core/src/avm1/activation.rs b/core/src/avm1/activation.rs index 1fd540d04d99..b7a2ab6d93be 100644 --- a/core/src/avm1/activation.rs +++ b/core/src/avm1/activation.rs @@ -551,14 +551,6 @@ impl<'a, 'gc> Activation<'a, 'gc> { fn stack_push(&mut self, mut value: Value<'gc>) { if let Value::Object(obj) = value { - // Note that there currently exists a subtle issue with this logic: - // If the cached `Object` in a `MovieClipReference` becomes invalidated, causing it to switch back to path-based object resolution, - // it should *never* switch back to cache-based resolution - // However, currently if a `MovieClipReference` in this invalidated-cache state is converted back to an `Object`, such as when passed as an argument to a function, - // if it pushed back onto the stack then it will be converted into a new `MovieClipReference`, causing it to switch back to cache-based resolution - // Fixing this will require a thorough refactor of AVM1 to store `Either - // can refer to a MovieClip - // There is a ignored test for this issue of "reference laundering" at "avm1/string_paths_reference_launder" if let Some(mcr) = MovieClipReference::try_from_stage_object(self, obj) { value = Value::MovieClip(mcr); } diff --git a/core/src/avm1/object_reference.rs b/core/src/avm1/object_reference.rs index 6149fac1cb68..00d22313c1a2 100644 --- a/core/src/avm1/object_reference.rs +++ b/core/src/avm1/object_reference.rs @@ -1,4 +1,4 @@ -use super::{Activation, Object, object::ObjectWeak}; +use super::{Activation, NativeObject, Object, object::ObjectWeak}; use crate::{ display_object::{DisplayObject, TDisplayObject, TDisplayObjectContainer}, string::{AvmString, WStr, WString}, @@ -71,13 +71,20 @@ impl<'gc> MovieClipReference<'gc> { activation: &mut Activation<'_, 'gc>, object: Object<'gc>, ) -> Option { + // If we are creating a reference to a clip which itself was formed by resolving a reference, + // then return that original reference. + if let NativeObject::MovieClip(mc) = object.native() + && let Some(original_ref) = mc.original_reference() + { + return Some(original_ref); + } + // We can't use as_display_object here as we explicitly don't want to convert `SuperObjects` let display_object = object.as_display_object_no_super()?; let (path, cached) = if let DisplayObject::MovieClip(mc) = display_object { (mc.path(), display_object.object1()?) } else if activation.swf_version() <= 5 { let display_object = Self::process_swf5_references(activation, display_object)?; - (display_object.path(), display_object.object1()?) } else { return None; @@ -177,7 +184,12 @@ impl<'gc> MovieClipReference<'gc> { /// Convert this reference to an `Object` pub fn coerce_to_object(self, activation: &mut Activation<'_, 'gc>) -> Option> { - let (_, object, _) = self.resolve_reference(activation)?; + let (cached, object, display_object) = self.resolve_reference(activation)?; + // Track the original reference used to produce this clip, so we can get it back later. + // Only do this if the cache was invalid, as that's the state we need to preserve. + if !cached && let Some(mc) = display_object.as_movie_clip() { + mc.set_original_reference(activation.gc(), self); + } Some(object) } diff --git a/core/src/display_object/movie_clip.rs b/core/src/display_object/movie_clip.rs index ae5ac53bf7b6..e80f969af83f 100644 --- a/core/src/display_object/movie_clip.rs +++ b/core/src/display_object/movie_clip.rs @@ -1,6 +1,7 @@ //! `MovieClip` display object and support code. use crate::avm1::Avm1; use crate::avm1::globals::AVM_DEPTH_BIAS; +use crate::avm1::object_reference::MovieClipReference; use crate::avm1::{Activation as Avm1Activation, ActivationIdentifier}; use crate::avm1::{NativeObject as Avm1NativeObject, Object as Avm1Object}; use crate::avm2::Activation as Avm2Activation; @@ -175,6 +176,13 @@ pub struct MovieClipData<'gc> { object1: Lock>>, object2: Lock>>, + /// When a MovieClip is passed around in AVM1, it is usually passed as a `MovieClipReference`, + /// however we need to resolve this reference into a concrete `Object` in various places. + /// Because references hold observable state, we need to ensure that when we convert this clip + /// back into a Reference that we generate the *original* reference, rather than a fresh one. + /// This stores the original reference that produced this clip. + original_reference: Lock>>, + drop_target: Lock>>, /// A DisplayObject (doesn't need to be visible) to use for hit tests instead of this clip. @@ -244,6 +252,7 @@ impl<'gc> MovieClipData<'gc> { audio_stream: Cell::new(None), object1: Lock::new(None), object2: Lock::new(None), + original_reference: Lock::new(None), clip_event_handlers: OnceCell::new(), clip_event_flags: Cell::new(ClipEventFlag::empty()), flags: Cell::new(MovieClipFlags::empty()), @@ -2417,6 +2426,17 @@ impl<'gc> MovieClip<'gc> { let has_pending_script = self.has_frame_script(self.0.current_frame.get()); self.0.has_pending_script.set(has_pending_script); } + + /// Mark this `MovieClip` as being originated from resolving the given `MovieClipReference`. + pub fn set_original_reference(self, mc: &Mutation<'gc>, reference: MovieClipReference<'gc>) { + let write = Gc::write(mc, self.0); + unlock!(write, MovieClipData, original_reference).set(Some(reference)); + } + + /// Get the originating `MovieClipReference` of this `MovieClip`, if it exists. + pub fn original_reference(self) -> Option> { + self.0.original_reference.get() + } } impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> { diff --git a/tests/tests/swfs/avm1/string_paths_reference_launder/output.ruffle.txt b/tests/tests/swfs/avm1/string_paths_reference_launder/output.ruffle.txt deleted file mode 100644 index e58e9764b398..000000000000 --- a/tests/tests/swfs/avm1/string_paths_reference_launder/output.ruffle.txt +++ /dev/null @@ -1,2 +0,0 @@ -100 -100 diff --git a/tests/tests/swfs/avm1/string_paths_reference_launder/test.toml b/tests/tests/swfs/avm1/string_paths_reference_launder/test.toml index 66f080ddb2b6..dbee897f5863 100644 --- a/tests/tests/swfs/avm1/string_paths_reference_launder/test.toml +++ b/tests/tests/swfs/avm1/string_paths_reference_launder/test.toml @@ -1,2 +1 @@ num_frames = 1 -known_failure = true # See the comment in `stack_push` in avm1/activiation.rs for details