core/avm2: Make all uncaught errors go through Avm2::uncaught_error

core/src/avm2.rs

@@ -19,7 +19,7 @@ use crate::avm2::script::{Script, TranslationUnit};
 use crate::avm2::stack::Stack;
 use crate::character::Character;
 use crate::context::UpdateContext;
-use crate::display_object::{MovieClip, TDisplayObject};
+use crate::display_object::{DisplayObject, MovieClip, TDisplayObject};
 use crate::string::{AvmString, StringContext};
 use crate::tag_utils::SwfMovie;
 use crate::PlayerRuntime;
@@ -361,20 +361,8 @@ impl<'gc> Avm2<'gc> {
         simulate_dispatch: bool,
     ) -> bool {
         let mut activation = Activation::from_nothing(context);
-        match events::dispatch_event(&mut activation, target, event, simulate_dispatch) {
-            Err(err) => {
-                let event_name = event.event().event_type();
-
-                tracing::error!(
-                    "Encountered AVM2 error when dispatching `{}` event: {:?}",
-                    event_name,
-                    err,
-                );
-                // TODO: push the error onto `loaderInfo.uncaughtErrorEvents`
-                false
-            }
-            Ok(handled) => handled,
-        }
+
+        events::dispatch_event(&mut activation, target, event, simulate_dispatch)
     }
 
     /// Add an object to the broadcast list.
@@ -447,17 +435,10 @@ impl<'gc> Avm2<'gc> {
                 .copied();
 
             if let Some(object) = object.and_then(|obj| obj.upgrade(context.gc())) {
-                let mut activation = Activation::from_nothing(context);
-
                 if object.is_of_type(on_type.inner_class_definition()) {
-                    if let Err(err) = events::broadcast_event(&mut activation, object, event) {
-                        tracing::error!(
-                            "Encountered AVM2 error when broadcasting `{}` event: {:?}",
-                            event_name,
-                            err,
-                        );
-                        // TODO: push the error onto `loaderInfo.uncaughtErrorEvents`
-                    }
+                    let mut activation = Activation::from_nothing(context);
+
+                    events::broadcast_event(&mut activation, object, event);
                 }
             }
         }
@@ -470,20 +451,6 @@ impl<'gc> Avm2<'gc> {
             .retain(|x| x.upgrade(context.gc_context).is_some());
     }
 
-    pub fn run_stack_frame_for_callable(
-        callable: Object<'gc>,
-        receiver: Value<'gc>,
-        domain: Domain<'gc>,
-        context: &mut UpdateContext<'gc>,
-    ) -> Result<(), String> {
-        let mut evt_activation = Activation::from_domain(context, domain);
-        Value::from(callable)
-            .call(&mut evt_activation, receiver, FunctionArgs::empty())
-            .map_err(|e| format!("{e:?}"))?;
-
-        Ok(())
-    }
-
     pub fn lookup_class_for_character(
         activation: &mut Activation<'_, 'gc>,
         movie_clip: MovieClip<'gc>,
@@ -703,4 +670,19 @@ impl<'gc> Avm2<'gc> {
     pub fn set_optimizer_enabled(&mut self, value: bool) {
         self.optimizer_enabled = value;
     }
+
+    // Report an uncaught AVM2 error.
+    // TODO should the `display_object` parameter be optional or not?
+    #[cold]
+    #[inline(never)]
+    pub fn uncaught_error(
+        _activation: &mut Activation<'_, 'gc>,
+        _display_object: Option<DisplayObject<'gc>>,
+        error: Error<'gc>,
+        info: &str,
+    ) {
+        tracing::error!("{}: {:?}", info, error);
+
+        // TODO: push the error onto `loaderInfo.uncaughtErrorEvents`
+    }
 }

core/src/avm2/events.rs

@@ -4,7 +4,7 @@ use crate::avm2::activation::Activation;
 use crate::avm2::function::FunctionArgs;
 use crate::avm2::globals::slots::flash_events_event_dispatcher as slots;
 use crate::avm2::object::{EventObject, FunctionObject, Object, TObject as _};
-use crate::avm2::Error;
+use crate::avm2::Avm2;
 use crate::display_object::TDisplayObject;
 use crate::string::AvmString;
 use fnv::FnvHashMap;
@@ -364,7 +364,7 @@ fn dispatch_event_to_target<'gc>(
     current_target: Object<'gc>,
     event: EventObject<'gc>,
     simulate_dispatch: bool,
-) -> Result<(), Error<'gc>> {
+) {
     avm_debug!(
         activation.context.avm2,
         "Event dispatch: {} to {current_target:?}",
@@ -375,7 +375,7 @@ fn dispatch_event_to_target<'gc>(
 
     if dispatch_list.is_none() {
         // Objects with no dispatch list act as if they had an empty one
-        return Ok(());
+        return;
     }
 
     let dispatch_list = dispatch_list.unwrap();
@@ -398,7 +398,7 @@ fn dispatch_event_to_target<'gc>(
     drop(evtmut);
 
     if simulate_dispatch {
-        return Ok(());
+        return;
     }
 
     for handler in handlers.iter() {
@@ -411,24 +411,24 @@ fn dispatch_event_to_target<'gc>(
         let args = &[event.into()];
         let result = handler.call(activation, global.into(), FunctionArgs::from_slice(args));
         if let Err(err) = result {
-            tracing::error!(
-                "Error dispatching event {:?} to handler {:?} : {:?}",
-                event,
-                handler,
+            let event_name = event.event().event_type();
+
+            Avm2::uncaught_error(
+                activation,
+                None, // TODO we need to set this, but how?
                 err,
+                &format!("Error dispatching event \"{}\"", event_name),
             );
         }
     }
-
-    Ok(())
 }
 
 pub fn dispatch_event<'gc>(
     activation: &mut Activation<'_, 'gc>,
     this: Object<'gc>,
     event: EventObject<'gc>,
     simulate_dispatch: bool,
-) -> Result<bool, Error<'gc>> {
+) -> bool {
     let target = this.get_slot(slots::TARGET).as_object().unwrap_or(this);
 
     let mut ancestor_list = Vec::new();
@@ -460,15 +460,15 @@ pub fn dispatch_event<'gc>(
             *ancestor,
             event,
             simulate_dispatch,
-        )?;
+        );
     }
 
     event
         .event_mut(activation.gc())
         .set_phase(EventPhase::AtTarget);
 
     if !event.event().is_propagation_stopped() {
-        dispatch_event_to_target(activation, this, target, target, event, simulate_dispatch)?;
+        dispatch_event_to_target(activation, this, target, target, event, simulate_dispatch);
     }
 
     event
@@ -488,12 +488,12 @@ pub fn dispatch_event<'gc>(
                 *ancestor,
                 event,
                 simulate_dispatch,
-            )?;
+            );
         }
     }
 
-    let handled = event.event().target.is_some();
-    Ok(handled)
+    // If the target is set, the event was handled
+    event.event().target.is_some()
 }
 
 /// Like `dispatch_event`, but does not run the Capturing and Bubbling phases,
@@ -503,14 +503,12 @@ pub fn broadcast_event<'gc>(
     activation: &mut Activation<'_, 'gc>,
     this: Object<'gc>,
     event: EventObject<'gc>,
-) -> Result<(), Error<'gc>> {
+) {
     let target = this.get_slot(slots::TARGET).as_object().unwrap_or(this);
 
     event
         .event_mut(activation.gc())
         .set_phase(EventPhase::AtTarget);
 
-    dispatch_event_to_target(activation, this, target, target, event, false)?;
-
-    Ok(())
+    dispatch_event_to_target(activation, this, target, target, event, false);
 }

core/src/avm2/globals/flash/events/event_dispatcher.rs

@@ -131,7 +131,7 @@ pub fn dispatch_event_internal<'gc>(
     // AS3-side typing guarantees that the event is actually an Event
     let event = event.as_event_object().unwrap();
 
-    events::dispatch_event(activation, this, event, false)?;
+    events::dispatch_event(activation, this, event, false);
 
     let not_canceled = !event.event().is_cancelled();
     Ok(not_canceled.into())

core/src/avm2/globals/flash/media/sound.rs

@@ -40,7 +40,7 @@ pub fn init<'gc>(
                 .library_for_movie_mut(movie)
                 .character_by_id(symbol)
             {
-                sound_object.set_sound(activation.context, sound)?;
+                sound_object.set_sound(activation.context, sound);
             } else {
                 tracing::warn!("Attempted to construct subclass of Sound, {}, which is associated with non-Sound character {}", class_def.name().local_name(), symbol);
             }
@@ -178,7 +178,7 @@ pub fn play<'gc>(
             sound_transform,
             sound_channel,
         };
-        if sound_object.play(queued_play, activation)? {
+        if sound_object.play(queued_play, activation) {
             return Ok(sound_channel.into());
         }
         // If we start playing a loaded sound with an invalid position,
@@ -297,7 +297,7 @@ pub fn load_compressed_data_from_byte_array<'gc>(
     Avm2::dispatch_event(activation.context, progress_evt, this_object);
 
     this.read_and_call_id3_event(activation, bytes);
-    this.set_sound(activation.context, handle)?;
+    this.set_sound(activation.context, handle);
 
     Ok(Value::Undefined)
 }

core/src/avm2/object/responder_object.rs

@@ -2,7 +2,6 @@ use crate::avm2::function::FunctionArgs;
 use crate::avm2::object::script_object::ScriptObjectData;
 use crate::avm2::object::{ClassObject, FunctionObject, Object, TObject};
 use crate::avm2::{Activation, Error};
-use crate::context::UpdateContext;
 use crate::net_connection::ResponderCallback;
 use flash_lso::types::Value as AMFValue;
 use gc_arena::barrier::unlock;
@@ -64,7 +63,7 @@ impl<'gc> ResponderObject<'gc> {
 
     pub fn send_callback(
         self,
-        context: &mut UpdateContext<'gc>,
+        activation: &mut Activation<'_, 'gc>,
         callback: ResponderCallback,
         message: &AMFValue,
     ) -> Result<(), Error<'gc>> {
@@ -74,10 +73,9 @@ impl<'gc> ResponderObject<'gc> {
         };
 
         if let Some(function) = function {
-            let mut activation = Activation::from_nothing(context);
-            let value = crate::avm2::amf::deserialize_value(&mut activation, message)?;
+            let value = crate::avm2::amf::deserialize_value(activation, message)?;
             let args = &[value];
-            function.call(&mut activation, self.into(), FunctionArgs::from_slice(args))?;
+            function.call(activation, self.into(), FunctionArgs::from_slice(args))?;
         }
 
         Ok(())

core/src/avm2/object/sound_object.rs

@@ -140,11 +140,7 @@ impl<'gc> SoundObject<'gc> {
     }
 
     /// Returns `true` if a `SoundChannel` should be returned back to the AVM2 caller.
-    pub fn play(
-        self,
-        queued: QueuedPlay<'gc>,
-        activation: &mut Activation<'_, 'gc>,
-    ) -> Result<bool, Error<'gc>> {
+    pub fn play(self, queued: QueuedPlay<'gc>, activation: &mut Activation<'_, 'gc>) -> bool {
         let mut sound_data = unlock!(
             Gc::write(activation.gc(), self.0),
             SoundObjectData,
@@ -156,29 +152,26 @@ impl<'gc> SoundObject<'gc> {
                 // Avoid to enqueue more unloaded sounds than the maximum allowed to be played
                 if queued_plays.len() >= AudioManager::MAX_SOUNDS {
                     tracing::warn!("Sound.play: too many unloaded sounds queued");
-                    return Ok(false);
+                    return false;
                 }
 
                 queued_plays.push(queued);
+
                 // We don't know the length yet, so return the `SoundChannel`
-                Ok(true)
+                true
             }
             SoundData::Loaded { sound } => play_queued(queued, *sound, activation.context),
         }
     }
 
-    pub fn set_sound(
-        self,
-        context: &mut UpdateContext<'gc>,
-        sound: SoundHandle,
-    ) -> Result<(), Error<'gc>> {
+    pub fn set_sound(self, context: &mut UpdateContext<'gc>, sound: SoundHandle) {
         let mut sound_data =
             unlock!(Gc::write(context.gc(), self.0), SoundObjectData, sound_data).borrow_mut();
 
         match &mut *sound_data {
             SoundData::NotLoaded { queued_plays } => {
                 for queued in std::mem::take(queued_plays) {
-                    play_queued(queued, sound, context)?;
+                    play_queued(queued, sound, context);
                 }
                 *sound_data = SoundData::Loaded { sound };
             }
@@ -187,7 +180,6 @@ impl<'gc> SoundObject<'gc> {
             }
         }
         self.set_loading_state(SoundLoadingState::Loaded);
-        Ok(())
     }
 
     pub fn id3(self) -> Option<Object<'gc>> {
@@ -280,15 +272,15 @@ fn play_queued<'gc>(
     queued: QueuedPlay<'gc>,
     sound: SoundHandle,
     context: &mut UpdateContext<'gc>,
-) -> Result<bool, Error<'gc>> {
+) -> bool {
     if let Some(duration) = context.audio.get_sound_duration(sound) {
         if queued.position > duration {
             tracing::error!(
                 "Sound.play: position={} is greater than duration={}",
                 queued.position,
                 duration
             );
-            return Ok(false);
+            return false;
         }
     }
 
@@ -303,7 +295,8 @@ fn play_queued<'gc>(
 
         context.attach_avm2_sound_channel(instance, queued.sound_channel);
     }
-    Ok(true)
+
+    true
 }
 
 impl<'gc> TObject<'gc> for SoundObject<'gc> {

core/src/avm2/object/stage3d_object.rs

@@ -1,8 +1,8 @@
 //! Object representation for Stage3D objects
 
-use crate::avm2::activation::Activation;
 use crate::avm2::object::script_object::ScriptObjectData;
 use crate::avm2::object::{Object, TObject};
+use crate::context::UpdateContext;
 use core::fmt;
 use gc_arena::barrier::unlock;
 use gc_arena::lock::Lock;
@@ -27,10 +27,10 @@ impl fmt::Debug for Stage3DObject<'_> {
 }
 
 impl<'gc> Stage3DObject<'gc> {
-    pub fn new(activation: &mut Activation<'_, 'gc>) -> Self {
-        let class = activation.avm2().classes().stage3d;
+    pub fn new(context: &mut UpdateContext<'gc>) -> Self {
+        let class = context.avm2.classes().stage3d;
         Stage3DObject(Gc::new(
-            activation.gc(),
+            context.gc(),
             Stage3DObjectData {
                 base: ScriptObjectData::new(class),
                 context3d: Lock::new(None),