core: Clean up ExternalInterface a little

core/src/avm1/globals/external_interface.rs

@@ -4,7 +4,7 @@ use crate::avm1::activation::Activation;
 use crate::avm1::error::Error;
 use crate::avm1::property_decl::{define_properties_on, Declaration};
 use crate::avm1::{Object, ScriptObject, Value};
-use crate::external::{Callback, Value as ExternalValue};
+use crate::external::{Callback, ExternalInterface, Value as ExternalValue};
 use crate::string::StringContext;
 
 const OBJECT_DECLS: &[Declaration] = declare_properties! {
@@ -64,23 +64,16 @@ pub fn call<'gc>(
     };
     let name = name.coerce_to_string(activation)?;
 
-    if let Some(method) = activation
-        .context
-        .external_interface
-        .get_method_for(&name.to_utf8_lossy())
-    {
-        let mut external_args = Vec::with_capacity(external_args_len);
-        if external_args_len > 0 {
-            for arg in &args[1..] {
-                external_args.push(ExternalValue::from_avm1(activation, arg.to_owned())?);
-            }
+    let mut external_args = Vec::with_capacity(external_args_len);
+    if external_args_len > 0 {
+        for arg in &args[1..] {
+            external_args.push(ExternalValue::from_avm1(activation, arg.to_owned())?);
         }
-        Ok(method
-            .call(activation.context, &external_args)
-            .into_avm1(activation))
-    } else {
-        Ok(Value::Null)
     }
+    Ok(
+        ExternalInterface::call_method(activation.context, &name.to_utf8_lossy(), &external_args)
+            .into_avm1(activation),
+    )
 }
 
 pub fn create_external_interface_object<'gc>(

core/src/avm2/globals/flash/external/external_interface.rs

@@ -1,7 +1,7 @@
 use crate::avm2::error::error;
 use crate::avm2::parameters::ParametersExt;
 use crate::avm2::{Activation, Error, Object, Value};
-use crate::external::{Callback, Value as ExternalValue};
+use crate::external::{Callback, ExternalInterface, Value as ExternalValue};
 use crate::string::AvmString;
 
 pub fn call<'gc>(
@@ -12,21 +12,14 @@ pub fn call<'gc>(
     let name = args.get_string(activation, 0)?;
     check_available(activation)?;
 
-    if let Some(method) = activation
-        .context
-        .external_interface
-        .get_method_for(&name.to_utf8_lossy())
-    {
-        let mut external_args = Vec::with_capacity(args.len() - 1);
-        for arg in &args[1..] {
-            external_args.push(ExternalValue::from_avm2(arg.to_owned()));
-        }
-        Ok(method
-            .call(activation.context, &external_args)
-            .into_avm2(activation))
-    } else {
-        Ok(Value::Null)
+    let mut external_args = Vec::with_capacity(args.len() - 1);
+    for arg in &args[1..] {
+        external_args.push(ExternalValue::from_avm2(arg.to_owned()));
     }
+    Ok(
+        ExternalInterface::call_method(activation.context, &name.to_utf8_lossy(), &external_args)
+            .into_avm2(activation),
+    )
 }
 
 fn check_available<'gc>(activation: &mut Activation<'_, 'gc>) -> Result<(), Error<'gc>> {

core/src/external.rs

@@ -13,6 +13,7 @@ use crate::context::UpdateContext;
 use crate::string::AvmString;
 use gc_arena::Collect;
 use std::collections::BTreeMap;
+use std::rc::Rc;
 
 /// An intermediate format of representing shared data between ActionScript and elsewhere.
 ///
@@ -333,55 +334,44 @@ impl FsCommandProvider for NullFsCommandProvider {
 }
 
 pub trait ExternalInterfaceProvider {
-    fn get_method(&self, name: &str) -> Option<Box<dyn ExternalInterfaceMethod>>;
+    fn call_method(&self, context: &mut UpdateContext<'_>, name: &str, args: &[Value]) -> Value;
 
     fn on_callback_available(&self, name: &str);
 
     fn get_id(&self) -> Option<String>;
 }
 
-pub trait ExternalInterfaceMethod {
-    fn call(&self, context: &mut UpdateContext<'_>, args: &[Value]) -> Value;
-}
-
-impl<F> ExternalInterfaceMethod for F
-where
-    F: Fn(&mut UpdateContext<'_>, &[Value]) -> Value,
-{
-    fn call(&self, context: &mut UpdateContext<'_>, args: &[Value]) -> Value {
-        self(context, args)
-    }
-}
+pub struct NullExternalInterfaceProvider;
 
 #[derive(Collect)]
 #[collect(no_drop)]
 pub struct ExternalInterface<'gc> {
     #[collect(require_static)]
-    providers: Vec<Box<dyn ExternalInterfaceProvider>>,
+    provider: Option<Rc<Box<dyn ExternalInterfaceProvider>>>,
     callbacks: BTreeMap<String, Callback<'gc>>,
     #[collect(require_static)]
     fs_commands: Box<dyn FsCommandProvider>,
 }
 
 impl<'gc> ExternalInterface<'gc> {
     pub fn new(
-        providers: Vec<Box<dyn ExternalInterfaceProvider>>,
+        provider: Option<Box<dyn ExternalInterfaceProvider>>,
         fs_commands: Box<dyn FsCommandProvider>,
     ) -> Self {
         Self {
-            providers,
+            provider: provider.map(Rc::new),
             callbacks: Default::default(),
             fs_commands,
         }
     }
 
-    pub fn add_provider(&mut self, provider: Box<dyn ExternalInterfaceProvider>) {
-        self.providers.push(provider);
+    pub fn set_provider(&mut self, provider: Option<Box<dyn ExternalInterfaceProvider>>) {
+        self.provider = provider.map(Rc::new);
     }
 
     pub fn add_callback(&mut self, name: String, callback: Callback<'gc>) {
         self.callbacks.insert(name.clone(), callback);
-        for provider in &self.providers {
+        if let Some(provider) = &self.provider {
             provider.on_callback_available(&name);
         }
     }
@@ -390,26 +380,21 @@ impl<'gc> ExternalInterface<'gc> {
         self.callbacks.get(name).cloned()
     }
 
-    pub fn get_method_for(&self, name: &str) -> Option<Box<dyn ExternalInterfaceMethod>> {
-        for provider in &self.providers {
-            if let Some(method) = provider.get_method(name) {
-                return Some(method);
-            }
+    pub fn call_method(context: &mut UpdateContext<'gc>, name: &str, args: &[Value]) -> Value {
+        let provider = context.external_interface.provider.clone();
+        if let Some(provider) = &provider {
+            provider.call_method(context, name, args)
+        } else {
+            Value::Undefined
         }
-        None
     }
 
     pub fn available(&self) -> bool {
-        !self.providers.is_empty()
+        self.provider.is_some()
     }
 
     pub fn any_id(&self) -> Option<String> {
-        for provider in &self.providers {
-            if let Some(id) = provider.get_id() {
-                return Some(id);
-            }
-        }
-        None
+        self.provider.as_ref().and_then(|p| p.get_id())
     }
 
     pub fn invoke_fs_command(&self, command: &str, args: &str) -> bool {

core/src/player.rs

@@ -2368,9 +2368,12 @@ impl Player {
         self.mutate_with_update_context(|context| context.avm1.has_mouse_listener())
     }
 
-    pub fn add_external_interface(&mut self, provider: Box<dyn ExternalInterfaceProvider>) {
+    pub fn set_external_interface_provider(
+        &mut self,
+        provider: Option<Box<dyn ExternalInterfaceProvider>>,
+    ) {
         self.mutate_with_update_context(|context| {
-            context.external_interface.add_provider(provider)
+            context.external_interface.set_provider(provider)
         });
     }
 
@@ -2467,7 +2470,7 @@ pub struct PlayerBuilder {
     quality: StageQuality,
     page_url: Option<String>,
     frame_rate: Option<f64>,
-    external_interface_providers: Vec<Box<dyn ExternalInterfaceProvider>>,
+    external_interface_provider: Option<Box<dyn ExternalInterfaceProvider>>,
     fs_command_provider: Box<dyn FsCommandProvider>,
     #[cfg(feature = "known_stubs")]
     stub_report_output: Option<std::path::PathBuf>,
@@ -2518,7 +2521,7 @@ impl PlayerBuilder {
             quality: StageQuality::High,
             page_url: None,
             frame_rate: None,
-            external_interface_providers: vec![],
+            external_interface_provider: None,
             fs_command_provider: Box::new(NullFsCommandProvider),
             #[cfg(feature = "known_stubs")]
             stub_report_output: None,
@@ -2703,7 +2706,7 @@ impl PlayerBuilder {
 
     /// Adds an External Interface provider for movies to communicate with
     pub fn with_external_interface(mut self, provider: Box<dyn ExternalInterfaceProvider>) -> Self {
-        self.external_interface_providers.push(provider);
+        self.external_interface_provider = Some(provider);
         self
     }
 
@@ -2737,7 +2740,7 @@ impl PlayerBuilder {
         player_runtime: PlayerRuntime,
         fullscreen: bool,
         fake_movie: Arc<SwfMovie>,
-        external_interface_providers: Vec<Box<dyn ExternalInterfaceProvider>>,
+        external_interface_provider: Option<Box<dyn ExternalInterfaceProvider>>,
         fs_command_provider: Box<dyn FsCommandProvider>,
     ) -> GcRoot<'gc> {
         let mut interner = AvmStringInterner::new(gc_context);
@@ -2761,7 +2764,7 @@ impl PlayerBuilder {
             current_context_menu: None,
             drag_object: None,
             external_interface: ExternalInterface::new(
-                external_interface_providers,
+                external_interface_provider,
                 fs_command_provider,
             ),
             library: Library::empty(),
@@ -2888,7 +2891,7 @@ impl PlayerBuilder {
                         self.player_runtime,
                         self.fullscreen,
                         fake_movie.clone(),
-                        self.external_interface_providers,
+                        self.external_interface_provider,
                         self.fs_command_provider,
                     )
                 }))),

desktop/src/backends/external_interface.rs

@@ -1,62 +1,49 @@
 use ruffle_core::context::UpdateContext;
-use ruffle_core::external::{
-    ExternalInterfaceMethod, ExternalInterfaceProvider, Value as ExternalValue,
-};
+use ruffle_core::external::{ExternalInterfaceProvider, Value as ExternalValue};
 use url::Url;
 
 pub struct DesktopExternalInterfaceProvider {
     pub spoof_url: Option<Url>,
 }
 
-struct FakeLocationHrefToString(Url);
-
-impl ExternalInterfaceMethod for FakeLocationHrefToString {
-    fn call(&self, _context: &mut UpdateContext<'_>, _args: &[ExternalValue]) -> ExternalValue {
-        ExternalValue::String(self.0.to_string())
-    }
-}
-
 fn is_location_href(code: &str) -> bool {
     matches!(
         code,
         "document.location.href" | "window.location.href" | "top.location.href"
     )
 }
 
-struct FakeEval(Option<Url>);
-
-impl ExternalInterfaceMethod for FakeEval {
-    fn call(&self, _context: &mut UpdateContext<'_>, args: &[ExternalValue]) -> ExternalValue {
-        if let Some(ref url) = self.0 {
-            if let [ExternalValue::String(ref code)] = args {
-                if is_location_href(code) {
-                    return ExternalValue::String(url.to_string());
-                }
-            }
-        }
-
-        tracing::warn!("Trying to call eval with ExternalInterface: {args:?}");
-        ExternalValue::Undefined
-    }
-}
-
 impl ExternalInterfaceProvider for DesktopExternalInterfaceProvider {
-    fn get_method(&self, name: &str) -> Option<Box<dyn ExternalInterfaceMethod>> {
+    fn call_method(
+        &self,
+        _context: &mut UpdateContext<'_>,
+        name: &str,
+        args: &[ExternalValue],
+    ) -> ExternalValue {
         if let Some(ref url) = self.spoof_url {
             // Check for e.g. "window.location.href.toString"
             if let Some(name) = name.strip_suffix(".toString") {
                 if is_location_href(name) {
-                    return Some(Box::new(FakeLocationHrefToString(url.clone())));
+                    return url.to_string().into();
                 }
             }
         }
 
         if name == "eval" {
-            return Some(Box::new(FakeEval(self.spoof_url.clone())));
+            if let Some(ref url) = self.spoof_url {
+                if let [ExternalValue::String(ref code)] = args {
+                    if is_location_href(code) {
+                        return ExternalValue::String(url.to_string());
+                    }
+                }
+            }
+
+            tracing::warn!("Trying to call eval with ExternalInterface: {args:?}");
+            return ExternalValue::Undefined;
         }
 
         tracing::warn!("Trying to call unknown ExternalInterface method: {name}");
-        None
+        ExternalValue::Undefined
     }
 
     fn on_callback_available(&self, _name: &str) {}

tests/tests/external_interface/mod.rs

@@ -3,8 +3,8 @@
 #![allow(clippy::needless_pass_by_ref_mut)]
 
 use ruffle_core::context::UpdateContext;
-use ruffle_core::external::Value as ExternalValue;
-use ruffle_core::external::{ExternalInterfaceMethod, ExternalInterfaceProvider};
+use ruffle_core::external::ExternalInterfaceProvider;
+use ruffle_core::external::{Value as ExternalValue, Value};
 
 pub mod tests;
 
@@ -41,12 +41,17 @@ fn do_reentry(context: &mut UpdateContext<'_>, _args: &[ExternalValue]) -> Exter
 }
 
 impl ExternalInterfaceProvider for ExternalInterfaceTestProvider {
-    fn get_method(&self, name: &str) -> Option<Box<dyn ExternalInterfaceMethod>> {
+    fn call_method(
+        &self,
+        context: &mut UpdateContext<'_>,
+        name: &str,
+        args: &[ExternalValue],
+    ) -> ExternalValue {
         match name {
-            "trace" => Some(Box::new(do_trace)),
-            "ping" => Some(Box::new(do_ping)),
-            "reentry" => Some(Box::new(do_reentry)),
-            _ => None,
+            "trace" => do_trace(context, args),
+            "ping" => do_ping(context, args),
+            "reentry" => do_reentry(context, args),
+            _ => Value::Null,
         }
     }
 

tests/tests/external_interface/tests.rs

@@ -25,7 +25,7 @@ pub fn external_interface_avm1(
         .player()
         .lock()
         .unwrap()
-        .add_external_interface(Box::new(ExternalInterfaceTestProvider::new()));
+        .set_external_interface_provider(Some(Box::new(ExternalInterfaceTestProvider::new())));
 
     let mut first = true;
 
@@ -93,7 +93,7 @@ pub fn external_interface_avm2(
         .player()
         .lock()
         .unwrap()
-        .add_external_interface(Box::new(ExternalInterfaceTestProvider::new()));
+        .set_external_interface_provider(Some(Box::new(ExternalInterfaceTestProvider::new())));
 
     let mut first = true;