avm1: Store AvmStrings in FileReference

This avoids extra `RefCell`s and back-and-forth conversions to `String`.

Author: moulins

core/src/avm1/globals/file_reference.rs

@@ -11,7 +11,8 @@ use crate::backend::ui::{FileDialogResult, FileFilter};
 use crate::string::{AvmString, StringContext};
 use gc_arena::barrier::unlock;
 use gc_arena::lock::Lock;
-use gc_arena::{Collect, Gc, Mutation};
+use gc_arena::{Collect, Gc};
+use ruffle_macros::istr;
 use url::Url;
 
 // There are two undocumented functions in FileReference: convertToPPT and deleteConvertedPPT.
@@ -32,6 +33,9 @@ impl<'gc> FileReferenceObject<'gc> {
         activation: &mut Activation<'_, 'gc>,
         dialog_result: &dyn FileDialogResult,
     ) {
+        let mc = activation.gc();
+        let write = Gc::write(mc, self.0);
+
         self.0.is_initialised.set(true);
 
         let date_proto = activation.context.avm1.prototypes().date_constructor;
@@ -40,7 +44,7 @@ impl<'gc> FileReferenceObject<'gc> {
                 activation,
                 &[(creation_time.timestamp_millis() as f64).into()],
             ) {
-                self.set_creation_date(activation.gc(), Some(obj));
+                unlock!(write, FileReferenceData, creation_date).set(Some(obj));
             }
         }
 
@@ -49,39 +53,41 @@ impl<'gc> FileReferenceObject<'gc> {
                 activation,
                 &[(modification_time.timestamp_millis() as f64).into()],
             ) {
-                self.set_modification_date(activation.gc(), Some(obj));
+                unlock!(write, FileReferenceData, modification_date).set(Some(obj));
             }
         }
 
-        self.0.file_type.replace(dialog_result.file_type());
-        self.0.name.replace(dialog_result.file_name());
-        self.0.size.replace(dialog_result.size());
-        self.0.creator.replace(dialog_result.creator());
-        self.0.data.replace(dialog_result.contents().to_vec());
-    }
+        let file_type = dialog_result
+            .file_type()
+            .map(|s| AvmString::new_utf8(mc, s));
+        unlock!(write, FileReferenceData, file_type).set(file_type);
 
-    fn set_creation_date(self, gc: &'gc Mutation<'gc>, creation_date: Option<Object<'gc>>) {
-        unlock!(Gc::write(gc, self.0), FileReferenceData, creation_date).set(creation_date);
-    }
+        let file_name = dialog_result
+            .file_name()
+            .map(|s| AvmString::new_utf8(mc, s));
+        unlock!(write, FileReferenceData, name).set(file_name);
+
+        let creator = dialog_result.creator().map(|s| AvmString::new_utf8(mc, s));
+        unlock!(write, FileReferenceData, creator).set(creator);
 
-    fn set_modification_date(self, gc: &'gc Mutation<'gc>, modification_date: Option<Object<'gc>>) {
-        unlock!(Gc::write(gc, self.0), FileReferenceData, modification_date).set(modification_date);
+        self.0.size.replace(dialog_result.size());
+        self.0.data.replace(dialog_result.contents().to_vec());
     }
 }
 
-#[derive(Default, Clone, Collect)]
+#[derive(Clone, Default, Collect)]
 #[collect(no_drop)]
 pub struct FileReferenceData<'gc> {
     /// Has this object been initialised from a dialog
     is_initialised: Cell<bool>,
 
     creation_date: Lock<Option<Object<'gc>>>,
-    creator: RefCell<Option<String>>,
+    creator: Lock<Option<AvmString<'gc>>>,
     modification_date: Lock<Option<Object<'gc>>>,
-    name: RefCell<Option<String>>,
-    post_data: RefCell<String>,
+    name: Lock<Option<AvmString<'gc>>>,
+    post_data: Lock<Option<AvmString<'gc>>>,
     size: Cell<Option<u64>>,
-    file_type: RefCell<Option<String>>,
+    file_type: Lock<Option<AvmString<'gc>>>,
 
     /// The contents of the referenced file
     /// We track this here so that it can be referenced in FileReference.upload
@@ -114,26 +120,23 @@ pub fn creation_date<'gc>(
             .0
             .creation_date
             .get()
-            .map_or(Value::Undefined, |x| x.into()));
+            .map_or(Value::Undefined, Into::into));
     }
 
     Ok(Value::Undefined)
 }
 
 pub fn creator<'gc>(
-    activation: &mut Activation<'_, 'gc>,
+    _activation: &mut Activation<'_, 'gc>,
     this: Object<'gc>,
     _args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     if let NativeObject::FileReference(file_ref) = this.native() {
         return Ok(file_ref
             .0
             .creator
-            .borrow()
-            .as_ref()
-            .map_or(Value::Undefined, |x| {
-                AvmString::new_utf8(activation.gc(), x).into()
-            }));
+            .get()
+            .map_or(Value::Undefined, Into::into));
     }
 
     Ok(Value::Undefined)
@@ -149,26 +152,19 @@ pub fn modification_date<'gc>(
             .0
             .modification_date
             .get()
-            .map_or(Value::Undefined, |x| x.into()));
+            .map_or(Value::Undefined, Into::into));
     }
 
     Ok(Value::Undefined)
 }
 
 pub fn name<'gc>(
-    activation: &mut Activation<'_, 'gc>,
+    _activation: &mut Activation<'_, 'gc>,
     this: Object<'gc>,
     _args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     if let NativeObject::FileReference(file_ref) = this.native() {
-        return Ok(file_ref
-            .0
-            .name
-            .borrow()
-            .as_ref()
-            .map_or(Value::Undefined, |x| {
-                AvmString::new_utf8(activation.gc(), x).into()
-            }));
+        return Ok(file_ref.0.name.get().map_or(Value::Undefined, Into::into));
     }
 
     Ok(Value::Undefined)
@@ -180,9 +176,8 @@ pub fn post_data<'gc>(
     _args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     if let NativeObject::FileReference(file_ref) = this.native() {
-        return Ok(
-            AvmString::new_utf8(activation.gc(), file_ref.0.post_data.borrow().clone()).into(),
-        );
+        let post_data = file_ref.0.post_data.get();
+        return Ok(post_data.unwrap_or_else(|| istr!("")).into());
     }
 
     Ok(Value::Undefined)
@@ -199,7 +194,8 @@ pub fn set_post_data<'gc>(
         .coerce_to_string(activation)?;
 
     if let NativeObject::FileReference(file_ref) = this.native() {
-        file_ref.0.post_data.replace(post_data.to_string());
+        let write = Gc::write(activation.gc(), file_ref.0);
+        unlock!(write, FileReferenceData, post_data).set(Some(post_data));
     }
 
     Ok(Value::Undefined)
@@ -211,26 +207,21 @@ pub fn size<'gc>(
     _args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     if let NativeObject::FileReference(file_ref) = this.native() {
-        return Ok(file_ref.0.size.get().map_or(Value::Undefined, |x| x.into()));
+        let size = file_ref.0.size.get();
+        return Ok(size.map_or(Value::Undefined, Into::into));
     }
 
     Ok(Value::Undefined)
 }
 
 pub fn file_type<'gc>(
-    activation: &mut Activation<'_, 'gc>,
+    _activation: &mut Activation<'_, 'gc>,
     this: Object<'gc>,
     _args: &[Value<'gc>],
 ) -> Result<Value<'gc>, Error<'gc>> {
     if let NativeObject::FileReference(file_ref) = this.native() {
-        return Ok(file_ref
-            .0
-            .file_type
-            .borrow()
-            .as_ref()
-            .map_or(Value::Undefined, |x| {
-                AvmString::new_utf8(activation.gc(), x).into()
-            }));
+        let file_type = file_ref.0.file_type.get();
+        return Ok(file_type.map_or(Value::Undefined, Into::into));
     }
 
     Ok(Value::Undefined)
@@ -401,17 +392,17 @@ pub fn upload<'gc>(
                 _ => return Ok(false.into()),
             }
 
+            let file_name = match file_reference.0.name.get() {
+                Some(name) => name.to_string(),
+                None => "file".to_string(),
+            };
+
             let process = activation.context.load_manager.upload_file(
                 activation.context.player.clone(),
                 this,
                 url_string,
                 file_reference.0.data.borrow().clone(),
-                file_reference
-                    .0
-                    .name
-                    .borrow()
-                    .clone()
-                    .unwrap_or_else(|| "file".to_string()),
+                file_name,
             );
 
             activation.context.navigator.spawn_future(process);