avm1: Lookup classes in global scope in primitive-to-object coercions...

...instead of using `SystemPrototypes`.

This is what FP does, as demonstrated by the added test (`avm1/coerce_to_object_monkeypatch`).
(we're not 100% correct yet, but this is fairly close)

Author: moulins

core/common/src/avm_string/common.rs

@@ -84,6 +84,7 @@ define_common_strings! {
     str_bold: b"bold",
     str_boldItalic: b"boldItalic",
     str_boolean: b"boolean",
+    str_Boolean: b"Boolean",
     str_broadcastMessage: b"broadcastMessage",
     str_builtInItems: b"builtInItems",
     str_bytesLoaded: b"bytesLoaded",
@@ -187,6 +188,7 @@ define_common_strings! {
     str_normal: b"normal",
     str_null: b"null",
     str_number: b"number",
+    str_Number: b"Number",
     str_object: b"object",
     str_onCancel: b"onCancel",
     str_onChanged: b"onChanged",
@@ -272,6 +274,7 @@ define_common_strings! {
     str_standardExtended: b"standardExtended",
     str_status: b"status",
     str_string: b"string",
+    str_String: b"String",
     str_subpixel: b"subpixel",
     str_subtract: b"subtract",
     str_success: b"success",

core/src/avm1/globals.rs

@@ -504,9 +504,6 @@ pub struct SystemPrototypes<'gc> {
     pub array_constructor: Object<'gc>,
     pub xml_node_constructor: Object<'gc>,
     pub xml_constructor: Object<'gc>,
-    pub string: Object<'gc>,
-    pub number: Object<'gc>,
-    pub boolean: Object<'gc>,
     pub matrix_constructor: Object<'gc>,
     pub point_constructor: Object<'gc>,
     pub rectangle: Object<'gc>,
@@ -744,9 +741,6 @@ pub fn create_globals<'gc>(
             array_constructor: array.constr,
             xml_node_constructor: xmlnode.constr,
             xml_constructor: xml.constr,
-            string: string.proto,
-            number: number.proto,
-            boolean: boolean.proto,
             matrix_constructor: matrix.constr,
             point_constructor: point.constr,
             rectangle: rectangle.proto,

core/src/avm1/value.rs

@@ -470,6 +470,22 @@ impl<'gc> Value<'gc> {
         }
     }
 
+    /// Coerce `self` to a script object, unconditionally.
+    ///
+    /// If `self` isn't coercible, returns a fresh bare object instead.
+    /// [MOULINS]: I suspect that most (if not all) usages of this method are incorrect,
+    /// but we have too much code already using it to remove it right now.
+    pub fn coerce_to_object_or_bare(
+        self,
+        activation: &mut Activation<'_, 'gc>,
+    ) -> Result<Object<'gc>, Error<'gc>> {
+        if let Some(obj) = self.coerce_to_object(activation)? {
+            Ok(obj)
+        } else {
+            Ok(Object::new_without_proto(activation.gc()))
+        }
+    }
+
     /// Coerce `self` to a script object, boxing primitives if necessary.
     ///
     /// This can fail in two different ways:
@@ -480,46 +496,36 @@ impl<'gc> Value<'gc> {
         self,
         activation: &mut Activation<'_, 'gc>,
     ) -> Result<Option<Object<'gc>>, Error<'gc>> {
-        let proto = match self {
+        let class_name = match self {
             // Object coerce to themselves...
             Value::Object(obj) => return Ok(Some(obj)),
             // ...and MovieClips coerce to their underlying object.
             Value::MovieClip(mcr) => return Ok(mcr.coerce_to_object(activation)),
             // `null` and `undefined` cannot be coerced.
             Value::Null | Value::Undefined => return Ok(None),
-            // Primitives need to be boxed, so select a suitable prototype.
-            Value::Bool(_) => activation.prototypes().boolean,
-            Value::Number(_) => activation.prototypes().number,
-            Value::String(_) => activation.prototypes().string,
+            // Primitives need to be boxed, so select a suitable class.
+            Value::Bool(_) => istr!("Boolean"),
+            Value::Number(_) => istr!("Number"),
+            Value::String(_) => istr!("String"),
         };
 
-        let obj = Object::new(&activation.context.strings, Some(proto));
-
-        // Constructor populates the boxed object with the value.
-        use crate::avm1::{function::NativeFunction, globals};
-        let constr: NativeFunction = match self {
-            Value::Bool(_) => globals::boolean::constructor,
-            Value::Number(_) => globals::number::constructor,
-            Value::String(_) => globals::string::constructor,
-            _ => |_, _, _| Ok(Value::Undefined),
+        // Fetch the constructor from the global scope...
+        let Some(Value::Object(class)) = activation
+            .global_object()
+            .get_opt(class_name, activation, false)?
+        else {
+            // No valid constructor, give up.
+            return Ok(None);
         };
 
-        constr(activation, obj, &[self]).map(|_| Some(obj))
-    }
-
-    /// Coerce `self` to a script object, unconditionally.
-    ///
-    /// If `self` isn't coercible, returns a fresh bare object instead.
-    /// [MOULINS]: I suspect that most (if not all) usages of this method are incorrect,
-    /// but we have too much code already using it to remove it right now.
-    pub fn coerce_to_object_or_bare(
-        self,
-        activation: &mut Activation<'_, 'gc>,
-    ) -> Result<Object<'gc>, Error<'gc>> {
-        if let Some(obj) = self.coerce_to_object(activation)? {
-            Ok(obj)
+        // ...and use it to box the primitive.
+        if let Value::Object(obj) = class.construct(activation, &[self])? {
+            // TODO(moulins): do MovieClips count as objects here?
+            Ok(Some(obj))
         } else {
-            Ok(Object::new_without_proto(activation.gc()))
+            // The constructor returned a non-object (this can happen
+            // with some native constructors), give up.
+            Ok(None)
         }
     }
 

tests/tests/swfs/avm1/coerce_to_object_monkeypatch/MyBoolean.as

@@ -0,0 +1,9 @@
+class MyBoolean extends Boolean {
+  var className;
+
+  function MyBoolean(val) {
+    super(val);
+    trace(this.className + "(" + val + ") constructor called!");
+    return "fake";
+  }
+}

tests/tests/swfs/avm1/coerce_to_object_monkeypatch/MyNumber.as

@@ -0,0 +1,9 @@
+class MyNumber extends Number {
+  var className;
+
+  function MyNumber(val) {
+    super(val);
+    trace(this.className + "(" + val + ") constructor called!");
+    return "fake";
+  }
+}

tests/tests/swfs/avm1/coerce_to_object_monkeypatch/MyString.as

@@ -0,0 +1,9 @@
+class MyString extends String {
+  var className;
+
+  function MyString(val) {
+    super(val);
+    trace(this.className + "(" + val + ") constructor called!");
+    return "fake";
+  }
+}

tests/tests/swfs/avm1/coerce_to_object_monkeypatch/Test.as

@@ -0,0 +1,128 @@
+// Compile with:
+//  mtasc -main -header 200:150:30 Test.as -swf test.swf -version 8
+class Test {
+
+
+  static function main(current) {
+    Number.prototype.className = "Number";
+    Boolean.prototype.className = "Boolean";
+    String.prototype.className = "String";
+    flash.display.BitmapData.prototype.className = "BitmapData";
+    _global.className = "<_global>";
+
+    MyNumber.prototype.className = "MyNumber";
+    MyBoolean.prototype.className = "MyBoolean";
+    MyString.prototype.className = "MyString";
+
+    trace("Monkey-patching-globals...");
+    // __proto__ should work, so let's test it.
+    var proto = _global.__proto__ = { __proto__: _global.__proto__ };
+    delete _global.Boolean;
+    delete _global.Number;
+    delete _global.String;
+
+    addGetter(proto, "Boolean", MyBoolean);
+    addGetter(proto, "Number", MyNumber);
+    addGetter(proto, "String", MyString);
+
+    trace("Testing primitive coercions...");
+    trace("");
+    
+    testCoerced(true);
+    testCoerced(42);
+    testCoerced("hello");
+
+    trace('// "world".length');
+    trace("result: " + "world".length);
+
+    var v = "callme";
+    trace('// "callme"()');
+    trace("result: " + v());
+    trace('// new "callme"()');
+    trace("result: " + new v());
+
+    trace("");
+    trace("Testing loookup logic...");
+    trace("");
+
+    testCoerced(undefined);
+    testCoerced(null);
+
+    trace("// delete Number; __resolve = () => MyNumber");
+    delete _global.Number;
+    delete proto.Number;
+    _global.__resolve = function(name) {
+      trace("__resolve(" + name + ") called!");
+      return MyNumber;
+    };
+
+    testCoerced(42);
+
+    trace("// Number = MyNumber");
+    _global.Number = MyNumber;
+    testCoerced(42);
+
+    trace("// Number.prototype = true");
+    _global.Number.prototype = true;
+    testCoerced(42);
+
+    trace('// Number = "some string"');
+    _global.Number = "some string";
+    testCoerced(42);
+
+    trace('// Number = BitmapData');
+    _global.Number = flash.display.BitmapData;
+    testCoerced(42);
+
+    trace('// Number = {}');
+    _global.Number = {};
+    testCoerced(42);
+
+    trace('// Number = Boolean');
+    _global.Number = Boolean;
+    testCoerced(42);
+
+    trace("Done!");
+    fscommand("quit");
+  }
+
+
+  static function addGetter(obj, name, val) {
+    var getter = function() {
+      trace(name + " getter called!");
+      return val;
+    };
+    Object.addProperty.call(obj, name, getter, null);
+  }
+
+  static var TEST_COERCED_INNER = function(val) {
+    trace("  coerced: " + this);
+    if (this === _global) {
+      trace("  is _global!");
+    } else {
+      trace("  typeof: " + typeof this);
+      trace("  typeof __proto__: " + typeof this.__proto__);
+      trace("  className: " + this.className);
+    }
+  };
+
+  static function testCoerced(val) {
+    trace("// " + val + " coerced to 'this'");
+    TEST_COERCED_INNER.call(val, val);
+    trace("// " + val + ".className");
+    trace("  className: " + val.className);
+    trace("");
+  }
+
+  static function monkeyPatch(obj, name, cls) {
+    // Set both as a property and a getter, for distinguishing what supports
+    // getters and what doesn't.
+    obj[name] = cls;
+    var getter = function() {
+      trace(name + " getter called!");
+      return cls;
+    };
+    Object.prototype.addProperty.call(obj, name, getter, null);
+  }
+
+}