avm1: Value::coerce_to_string returns an Avm1String, as it can avoid a clone-and-reallocate

core/src/avm1.rs

@@ -394,7 +394,7 @@ pub fn root_error_handler<'gc>(
     if let Error::ThrownValue(error) = &error {
         let string = error
             .coerce_to_string(activation, context)
-            .unwrap_or_else(|_| Cow::Borrowed("undefined"));
+            .unwrap_or_else(|_| "undefined".into());
         log::info!(target: "avm_trace", "{}", string);
     } else {
         log::error!("{}", error);

core/src/avm1/activation.rs

@@ -1299,7 +1299,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
                     .coerce_to_object(self, context);
                 let (url, opts) = self.locals_into_request_options(
                     context,
-                    url,
+                    Cow::Borrowed(&url),
                     NavigationMethod::from_send_vars_method(swf_method),
                 );
                 let fetch = context.navigator.fetch(&url, opts);
@@ -1317,7 +1317,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
             if let Some(clip_target) = clip_target {
                 let (url, opts) = self.locals_into_request_options(
                     context,
-                    url,
+                    Cow::Borrowed(&url),
                     NavigationMethod::from_send_vars_method(swf_method),
                 );
                 let fetch = context.navigator.fetch(&url, opts);
@@ -2277,7 +2277,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
         // trace always prints "undefined" even though SWF6 and below normally
         // coerce undefined to "".
         let out = if val == Value::Undefined {
-            Cow::Borrowed("undefined")
+            "undefined".into()
         } else {
             val.coerce_to_string(self, context)?
         };
@@ -2480,7 +2480,7 @@ impl<'a, 'gc: 'a> Activation<'a, 'gc> {
                 v.ok()
                     .unwrap_or_else(|| Value::Undefined)
                     .coerce_to_string(self, context)
-                    .unwrap_or_else(|_| Cow::Borrowed("undefined"))
+                    .unwrap_or_else(|_| "undefined".into())
                     .to_string(),
             );
         }

core/src/avm1/debug.rs

@@ -198,7 +198,7 @@ mod tests {
     use super::*;
     use crate::avm1::error::Error;
     use crate::avm1::test_utils::with_avm;
-    use crate::avm1::{Avm1String, ScriptObject};
+    use crate::avm1::ScriptObject;
 
     #[test]
     fn dump_undefined() {

core/src/avm1/globals.rs

@@ -167,7 +167,7 @@ pub fn create_timer<'a, 'gc>(
                     .get(1)
                     .unwrap_or(&Value::Undefined)
                     .coerce_to_string(activation, context)?
-                    .into_owned(),
+                    .to_string(),
             },
             2,
         ),

core/src/avm1/globals/array.rs

@@ -8,7 +8,6 @@ use crate::avm1::property::Attribute;
 use crate::avm1::{Avm1String, Object, ScriptObject, TObject, UpdateContext, Value};
 use enumset::EnumSet;
 use gc_arena::MutationContext;
-use smallvec::alloc::borrow::Cow;
 use std::cmp::Ordering;
 
 // Flags used by `Array.sort` and `sortOn`.
@@ -244,7 +243,7 @@ pub fn join<'gc>(
     let separator = args
         .get(0)
         .and_then(|v| v.coerce_to_string(activation, context).ok())
-        .unwrap_or_else(|| Cow::Borrowed(","));
+        .unwrap_or_else(|| ",".into());
     let values: Vec<Value<'gc>> = this.array();
 
     Ok(Avm1String::new(
@@ -253,9 +252,10 @@ pub fn join<'gc>(
             .iter()
             .map(|v| {
                 v.coerce_to_string(activation, context)
-                    .unwrap_or_else(|_| Cow::Borrowed("undefined"))
+                    .unwrap_or_else(|_| "undefined".into())
+                    .to_string()
             })
-            .collect::<Vec<Cow<str>>>()
+            .collect::<Vec<String>>()
             .join(&separator),
     )
     .into())

core/src/avm1/globals/movie_clip.rs

@@ -12,6 +12,7 @@ use crate::prelude::*;
 use crate::shape_utils::DrawCommand;
 use crate::tag_utils::SwfSlice;
 use gc_arena::MutationContext;
+use std::borrow::Cow;
 use swf::{
     FillStyle, Gradient, GradientInterpolation, GradientRecord, GradientSpread, LineCapStyle,
     LineJoinStyle, LineStyle, Twips,
@@ -1047,7 +1048,7 @@ fn load_movie<'gc>(
     let url = url_val.coerce_to_string(activation, context)?;
     let method = args.get(1).cloned().unwrap_or(Value::Undefined);
     let method = NavigationMethod::from_method_str(&method.coerce_to_string(activation, context)?);
-    let (url, opts) = activation.locals_into_request_options(context, url, method);
+    let (url, opts) = activation.locals_into_request_options(context, Cow::Borrowed(&url), method);
     let fetch = context.navigator.fetch(&url, opts);
     let process = context.load_manager.load_movie_into_clip(
         context.player.clone().unwrap(),
@@ -1071,7 +1072,7 @@ fn load_variables<'gc>(
     let url = url_val.coerce_to_string(activation, context)?;
     let method = args.get(1).cloned().unwrap_or(Value::Undefined);
     let method = NavigationMethod::from_method_str(&method.coerce_to_string(activation, context)?);
-    let (url, opts) = activation.locals_into_request_options(context, url, method);
+    let (url, opts) = activation.locals_into_request_options(context, Cow::Borrowed(&url), method);
     let fetch = context.navigator.fetch(&url, opts);
     let target = target.object().coerce_to_object(activation, context);
     let process =

core/src/avm1/globals/object.rs

@@ -29,7 +29,7 @@ pub fn add_property<'gc>(
     let name = args
         .get(0)
         .and_then(|v| v.coerce_to_string(activation, context).ok())
-        .unwrap_or_else(|| Cow::Borrowed("undefined"));
+        .unwrap_or_else(|| "undefined".into());
     let getter = args.get(1).unwrap_or(&Value::Undefined);
     let setter = args.get(2).unwrap_or(&Value::Undefined);
 
@@ -188,7 +188,13 @@ fn watch<'gc>(
     };
     let user_data = args.get(2).cloned().unwrap_or(Value::Undefined);
 
-    this.set_watcher(activation, context.gc_context, name, callback, user_data);
+    this.set_watcher(
+        activation,
+        context.gc_context,
+        Cow::Borrowed(&name),
+        callback,
+        user_data,
+    );
 
     Ok(true.into())
 }
@@ -206,7 +212,7 @@ fn unwatch<'gc>(
         return Ok(false.into());
     };
 
-    let result = this.remove_watcher(activation, context.gc_context, name);
+    let result = this.remove_watcher(activation, context.gc_context, Cow::Borrowed(&name));
 
     Ok(result.into())
 }

core/src/avm1/globals/string.rs

@@ -20,10 +20,7 @@ pub fn string<'gc>(
 ) -> Result<Value<'gc>, Error<'gc>> {
     let value = match args.get(0).cloned() {
         Some(Value::String(s)) => s,
-        Some(v) => Avm1String::new(
+        Some(v) => v.coerce_to_string(activation, ac)?,
-            ac.gc_context,
-            v.coerce_to_string(activation, ac)?.to_string(),
-        ),
         _ => Avm1String::new(ac.gc_context, String::new()),
     };
 
@@ -182,13 +179,7 @@ fn char_at<'gc>(
     // TODO: Will return REPLACEMENT_CHAR if this indexes a character outside the BMP, losing info about the surrogate.
     // When we improve our string representation, the unpaired surrogate should be returned.
     let this_val = Value::from(this);
-    let string = match this_val {
+    let string = this_val.coerce_to_string(activation, context)?;
-        Value::String(string) => string,
-        other => Avm1String::new(
-            context.gc_context,
-            other.coerce_to_string(activation, context)?.to_string(),
-        ),
-    };
     let i = args
         .get(0)
         .unwrap_or(&Value::Undefined)
@@ -406,13 +397,7 @@ fn split<'gc>(
     args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     let this_val = Value::from(this);
-    let this = match this_val {
+    let this = this_val.coerce_to_string(activation, context)?;
-        Value::String(string) => string,
-        other => Avm1String::new(
-            context.gc_context,
-            other.coerce_to_string(activation, context)?.to_string(),
-        ),
-    };
     let delimiter_val = args.get(0).unwrap_or(&Value::Undefined);
     let delimiter = delimiter_val.coerce_to_string(activation, context)?;
     let limit = match args.get(1) {
@@ -454,13 +439,7 @@ fn substr<'gc>(
     }
 
     let this_val = Value::from(this);
-    let this = match this_val {
+    let this = this_val.coerce_to_string(activation, context)?;
-        Value::String(string) => string,
-        other => Avm1String::new(
-            context.gc_context,
-            other.coerce_to_string(activation, context)?.to_string(),
-        ),
-    };
     let this_len = this.encode_utf16().count();
     let start_index = string_wrapping_index(
         args.get(0).unwrap().coerce_to_i32(activation, context)?,
@@ -487,13 +466,7 @@ fn substring<'gc>(
     }
 
     let this_val = Value::from(this);
-    let this = match this_val {
+    let this = this_val.coerce_to_string(activation, context)?;
-        Value::String(string) => string,
-        other => Avm1String::new(
-            context.gc_context,
-            other.coerce_to_string(activation, context)?.to_string(),
-        ),
-    };
     let this_len = this.encode_utf16().count();
     let mut start_index = string_index(
         args.get(0).unwrap().coerce_to_i32(activation, context)?,

core/src/avm1/globals/text_field.rs

@@ -112,7 +112,7 @@ pub fn set_html_text<'gc>(
                 .get(0)
                 .unwrap_or(&Value::Undefined)
                 .coerce_to_string(activation, context)?;
-            let _ = text_field.set_html_text(text.into_owned(), context);
+            let _ = text_field.set_html_text(text.to_string(), context);
             // Changing the htmlText does NOT update variable bindings (does not call EditText::propagate_text_binding).
         }
     }
@@ -326,7 +326,7 @@ fn set_variable<'gc>(
         .as_display_object()
         .and_then(|dobj| dobj.as_edit_text())
     {
-        etext.set_variable(variable.map(|v| v.into_owned()), activation, context);
+        etext.set_variable(variable.map(|v| v.to_string()), activation, context);
     }
 
     Ok(Value::Undefined)
@@ -627,7 +627,7 @@ fn replace_text<'gc>(
         .cloned()
         .unwrap_or(Value::Undefined)
         .coerce_to_string(activation, context)?
-        .into_owned();
+        .to_string();
 
     text_field.replace_text(from as usize, to as usize, &text, context);
 

core/src/avm1/globals/xml.rs

@@ -13,7 +13,6 @@ use crate::xml::{XMLDocument, XMLNode};
 use enumset::EnumSet;
 use gc_arena::MutationContext;
 use quick_xml::Error as ParseError;
-use std::borrow::Cow;
 
 pub const XML_NO_ERROR: f64 = 0.0;
 #[allow(dead_code)]
@@ -791,7 +790,7 @@ pub fn xml_parse_xml<'gc>(
             if let Some(Ok(xmlstring)) = args.get(0).map(|s| s.coerce_to_string(activation, ac)) {
                 xmlstring
             } else {
-                Cow::Borrowed("")
+                "".into()
             };
 
         if let Some(children) = node.children() {

core/src/avm1/object/stage_object.rs

@@ -187,7 +187,7 @@ impl<'gc> TObject<'gc> for StageObject<'gc> {
             .filter(|binding| binding.variable_name == name)
         {
             let _ = binding.text_field.set_html_text(
-                value.coerce_to_string(activation, context)?.into_owned(),
+                value.coerce_to_string(activation, context)?.to_string(),
                 context,
             );
         }

core/src/avm1/string.rs

@@ -1,4 +1,5 @@
 use gc_arena::{Collect, Gc, MutationContext};
+use std::fmt;
 use std::ops::Deref;
 
 #[derive(Debug, Clone, Collect)]
@@ -26,6 +27,14 @@ impl<'gc> Avm1String<'gc> {
     }
 }
 
+impl Default for Avm1String<'_> {
+    fn default() -> Self {
+        Self {
+            source: Source::Static(""),
+        }
+    }
+}
+
 impl<'gc> From<&'static str> for Avm1String<'gc> {
     fn from(str: &'static str) -> Self {
         Self {
@@ -34,6 +43,12 @@ impl<'gc> From<&'static str> for Avm1String<'gc> {
     }
 }
 
+impl fmt::Display for Avm1String<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self)
+    }
+}
+
 impl Deref for Avm1String<'_> {
     type Target = str;
 
@@ -56,6 +71,13 @@ impl AsRef<str> for Avm1String<'_> {
     }
 }
 
+impl<'gc> PartialEq<Avm1String<'gc>> for Avm1String<'gc> {
+    #[inline]
+    fn eq(&self, other: &Avm1String<'gc>) -> bool {
+        PartialEq::eq(self.as_str(), other.as_str())
+    }
+}
+
 macro_rules! impl_eq {
     ($lhs:ty, $rhs: ty) => {
         #[allow(unused_lifetimes)]

core/src/avm1/value.rs

@@ -457,26 +457,29 @@ impl<'gc> Value<'gc> {
         &'a self,
         activation: &mut Activation<'_, 'gc>,
         context: &mut UpdateContext<'_, 'gc, '_>,
-    ) -> Result<Cow<'a, str>, Error<'gc>> {
+    ) -> Result<Avm1String<'gc>, Error<'gc>> {
         Ok(match self {
             Value::Object(object) => {
                 match object.call_method("toString", &[], activation, context)? {
-                    Value::String(s) => Cow::Owned(s.to_string()),
+                    Value::String(s) => s,
-                    _ => Cow::Borrowed("[type Object]"),
+                    _ => "[type Object]".into(),
                 }
             }
             Value::Undefined => {
                 if activation.current_swf_version() >= 7 {
-                    Cow::Borrowed("undefined")
+                    "undefined".into()
                 } else {
-                    Cow::Borrowed("")
+                    "".into()
                 }
             }
-            Value::Null => Cow::Borrowed("null"),
+            Value::Null => "null".into(),
-            Value::Bool(true) => Cow::Borrowed("true"),
+            Value::Bool(true) => "true".into(),
-            Value::Bool(false) => Cow::Borrowed("false"),
+            Value::Bool(false) => "false".into(),
-            Value::Number(v) => f64_to_string(*v),
+            Value::Number(v) => match f64_to_string(*v) {
-            Value::String(v) => Cow::Borrowed(v),
+                Cow::Borrowed(str) => str.into(),
+                Cow::Owned(str) => Avm1String::new(context.gc_context, str),
+            },
+            Value::String(v) => v.to_owned(),
         })
     }
 

core/src/display_object/edit_text.rs

@@ -733,7 +733,7 @@ impl<'gc> EditText<'gc> {
                                     value
                                         .coerce_to_string(activation, context)
                                         .unwrap_or_default()
-                                        .into_owned(),
+                                        .to_string(),
                                     context,
                                 );
                             } else {