Skip to content

Allow supercalled methods to access properties of this correctly #499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Apr 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions core/src/avm1.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::avm1::function::{Avm1Function, FunctionObject};
use crate::avm1::globals::create_globals;
use crate::avm1::object::search_prototype;
use crate::avm1::return_value::ReturnValue;
use crate::backend::navigator::{NavigationMethod, RequestOptions};
use crate::context::UpdateContext;
Expand Down Expand Up @@ -310,16 +311,15 @@ impl<'gc> Avm1<'gc> {
context.gc_context,
Activation::from_nothing(swf_version, self.globals, context.gc_context, active_clip),
));
let callback = obj
.get(name, self, context)
.and_then(|prop| prop.resolve(self, context));
let search_result = search_prototype(Some(obj), name, self, context, obj)
.and_then(|r| Ok((r.0.resolve(self, context)?, r.1)));
self.stack_frames.pop();

// Run the callback.
// The function exec pushes its own stack frame.
// The function is now ready to execute with `run_stack_till_empty`.
if let Ok(callback) = callback {
let _ = callback.call(self, context, obj, args);
if let Ok((callback, base_proto)) = search_result {
let _ = callback.call(self, context, obj, base_proto, args);
}
}

Expand Down Expand Up @@ -376,7 +376,7 @@ impl<'gc> Avm1<'gc> {
// Each callback exec pushes its own stack frame.
// The functions are now ready to execute with `run_stack_till_empty`.
for (listener, handler) in handlers.drain(..) {
let _ = handler.call(self, context, listener, &args);
let _ = handler.call(self, context, listener, None, &args);
}
}

Expand Down Expand Up @@ -1246,7 +1246,7 @@ impl<'gc> Avm1<'gc> {
.resolve(fn_name.as_string()?, self, context)?
.resolve(self, context)?;
let this = self.target_clip_or_root().object().as_object()?;
target_fn.call(self, context, this, &args)?.push(self);
target_fn.call(self, context, this, None, &args)?.push(self);

Ok(())
}
Expand All @@ -1268,7 +1268,7 @@ impl<'gc> Avm1<'gc> {
Value::Undefined | Value::Null => {
let this = self.target_clip_or_root().object();
if let Ok(this) = this.as_object() {
object.call(self, context, this, &args)?.push(self);
object.call(self, context, this, None, &args)?.push(self);
} else {
log::warn!(
"Attempted to call constructor of {:?} (missing method name)",
Expand All @@ -1279,16 +1279,9 @@ impl<'gc> Avm1<'gc> {
}
Value::String(name) => {
if name.is_empty() {
object.call(self, context, object, &args)?.push(self);
object.call(self, context, object, None, &args)?.push(self);
} else {
let callable = object.get(&name, self, context)?.resolve(self, context)?;

if let Value::Object(_) = callable {
} else {
log::warn!("Object method {} is not callable", name);
}

callable.call(self, context, object, &args)?.push(self);
object.call_method(&name, &args, self, context)?.push(self);
}
}
_ => {
Expand Down Expand Up @@ -1597,6 +1590,7 @@ impl<'gc> Avm1<'gc> {
ScriptObject::object(context.gc_context, Some(super_proto)).into();

sub_prototype.set("constructor", superclass.into(), self, context)?;
sub_prototype.set("__constructor__", superclass.into(), self, context)?;
subclass.set("prototype", sub_prototype.into(), self, context)?;

Ok(())
Expand Down Expand Up @@ -2084,9 +2078,14 @@ impl<'gc> Avm1<'gc> {

let this = prototype.new(self, context, prototype, &args)?;

this.set("__constructor__", constructor.into(), self, context)?;
if self.current_swf_version() < 7 {
this.set("constructor", constructor.into(), self, context)?;
}

//TODO: What happens if you `ActionNewMethod` without a method name?
constructor
.call(self, context, this, &args)?
.call(self, context, this, None, &args)?
.resolve(self, context)?;

self.push(this);
Expand Down Expand Up @@ -2121,8 +2120,14 @@ impl<'gc> Avm1<'gc> {
.as_object()
{
let this = prototype.new(self, context, prototype, &args)?;

this.set("__constructor__", constructor.into(), self, context)?;
if self.current_swf_version() < 7 {
this.set("constructor", constructor.into(), self, context)?;
}

constructor
.call(self, context, this, &args)?
.call(self, context, this, None, &args)?
.resolve(self, context)?;
ret = this.into();
} else {
Expand Down
41 changes: 32 additions & 9 deletions core/src/avm1/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ impl<'gc> Executable<'gc> {
avm: &mut Avm1<'gc>,
ac: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
base_proto: Option<Object<'gc>>,
args: &[Value<'gc>],
) -> Result<ReturnValue<'gc>, Error> {
match self {
Expand Down Expand Up @@ -254,18 +255,19 @@ impl<'gc> Executable<'gc> {

let argcell = arguments.into();
let super_object: Option<Object<'gc>> = if !af.suppress_super {
Some(SuperObject::from_child_object(this, avm, ac)?.into())
Some(
SuperObject::from_this_and_base_proto(
this,
base_proto.unwrap_or(this),
avm,
ac,
)?
.into(),
)
} else {
None
};

if let Some(super_object) = super_object {
super_object
.as_super_object()
.unwrap()
.bind_this(ac.gc_context, super_object);
}

let effective_ver = if avm.current_swf_version() > 5 {
af.swf_version()
} else {
Expand Down Expand Up @@ -474,15 +476,27 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
base_proto: Option<Object<'gc>>,
args: &[Value<'gc>],
) -> Result<ReturnValue<'gc>, Error> {
if let Some(exec) = self.as_executable() {
exec.exec(avm, context, this, args)
exec.exec(avm, context, this, base_proto, args)
} else {
Ok(Value::Undefined.into())
}
}

fn call_setter(
&self,
name: &str,
value: Value<'gc>,
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error> {
self.base.call_setter(name, value, avm, context, this)
}

#[allow(clippy::new_ret_no_self)]
fn new(
&self,
Expand Down Expand Up @@ -587,6 +601,15 @@ impl<'gc> TObject<'gc> for FunctionObject<'gc> {
self.base.has_own_property(avm, context, name)
}

fn has_own_virtual(
&self,
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
name: &str,
) -> bool {
self.base.has_own_virtual(avm, context, name)
}

fn is_property_overwritable(&self, avm: &mut Avm1<'gc>, name: &str) -> bool {
self.base.is_property_overwritable(avm, name)
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/avm1/globals/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ fn sort_compare_custom<'gc>(
// TODO: Handle errors.
let args = [a.clone(), b.clone()];
let ret = compare_fn
.call(avm, context, this, &args)
.call(avm, context, this, None, &args)
.and_then(|v| v.resolve(avm, context))
.unwrap_or(Value::Undefined);
match ret {
Expand Down
4 changes: 2 additions & 2 deletions core/src/avm1/globals/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub fn call<'gc>(
};

match func.as_executable() {
Some(exec) => exec.exec(avm, action_context, this, args),
Some(exec) => exec.exec(avm, action_context, this, None, args),
_ => Ok(Value::Undefined.into()),
}
}
Expand Down Expand Up @@ -70,7 +70,7 @@ pub fn apply<'gc>(
}

match func.as_executable() {
Some(exec) => exec.exec(avm, action_context, this, &child_args),
Some(exec) => exec.exec(avm, action_context, this, None, &child_args),
_ => Ok(Value::Undefined.into()),
}
}
Expand Down
7 changes: 2 additions & 5 deletions core/src/avm1/globals/movie_clip_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,8 @@ pub fn broadcast_message<'gc>(
.resolve(avm, context)?;

if let Value::Object(listener) = listener {
let handler = listener
.get(&event_name, avm, context)?
.resolve(avm, context)?;
handler
.call(avm, context, listener, call_args)?
listener
.call_method(&event_name, call_args, avm, context)?
.resolve(avm, context)?;
}
}
Expand Down
12 changes: 3 additions & 9 deletions core/src/avm1/globals/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,22 +825,16 @@ pub fn xml_on_data<'gc>(
let src = args.get(0).cloned().unwrap_or(Value::Undefined);

if let Value::Undefined = src {
let on_load = this.get("onLoad", avm, ac)?.resolve(avm, ac)?;
on_load
.call(avm, ac, this, &[false.into()])?
this.call_method("onLoad", &[false.into()], avm, ac)?
.resolve(avm, ac)?;
} else {
let src = src.coerce_to_string(avm, ac)?;
let parse_xml = this.get("parseXML", avm, ac)?.resolve(avm, ac)?;
parse_xml
.call(avm, ac, this, &[src.into()])?
this.call_method("parseXML", &[src.into()], avm, ac)?
.resolve(avm, ac)?;

this.set("loaded", true.into(), avm, ac)?;

let on_load = this.get("onLoad", avm, ac)?.resolve(avm, ac)?;
on_load
.call(avm, ac, this, &[true.into()])?
this.call_method("onLoad", &[true.into()], avm, ac)?
.resolve(avm, ac)?;
}

Expand Down
70 changes: 66 additions & 4 deletions core/src/avm1/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
if self.has_own_property(avm, context, name) {
self.get_local(name, avm, context, (*self).into())
} else {
search_prototype(self.proto(), name, avm, context, (*self).into())
Ok(search_prototype(self.proto(), name, avm, context, (*self).into())?.0)
}
}

Expand All @@ -83,9 +83,54 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
base_proto: Option<Object<'gc>>,
args: &[Value<'gc>],
) -> Result<ReturnValue<'gc>, Error>;

/// Call a method on the object.
///
/// It is highly recommended to use this convenience method to perform
/// method calls. It is morally equivalent to an AVM1 `ActionCallMethod`
/// opcode. It will take care of retrieving the method, calculating it's
/// base prototype for `super` calls, and providing it with the correct
/// `this` parameter.
fn call_method(
&self,
name: &str,
args: &[Value<'gc>],
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
) -> Result<ReturnValue<'gc>, Error> {
let (method, base_proto) =
search_prototype(Some((*self).into()), name, avm, context, (*self).into())?;
let method = method.resolve(avm, context)?;

if let Value::Object(_) = method {
} else {
log::warn!("Object method {} is not callable", name);
}

method.call(avm, context, (*self).into(), base_proto, args)
}

/// Call a setter defined in this object.
///
/// This function returns the `ReturnValue` of the called function; it
/// should be resolved and discarded. Attempts to call a non-virtual setter
/// or non-existent setter fail silently.
///
/// The setter will be invoked with the provided `this`. It is assumed that
/// this function is being called on the appropriate `base_proto` and
/// `super` will be invoked following said guidance.
fn call_setter(
&self,
name: &str,
value: Value<'gc>,
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error>;

/// Construct a host object of some kind and return it's cell.
///
/// As the first step in object construction, the `new` method is called on
Expand Down Expand Up @@ -215,6 +260,15 @@ pub trait TObject<'gc>: 'gc + Collect + Debug + Into<Object<'gc>> + Clone + Copy
name: &str,
) -> bool;

/// Checks if the object has a given named property on itself that is
/// virtual.
fn has_own_virtual(
&self,
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
name: &str,
) -> bool;

/// Checks if a named property can be overwritten.
fn is_property_overwritable(&self, avm: &mut Avm1<'gc>, name: &str) -> bool;

Expand Down Expand Up @@ -385,13 +439,21 @@ impl<'gc> Object<'gc> {
}
}

/// Perform a prototype lookup of a given object.
///
/// This function returns both the `ReturnValue` and the prototype that
/// generated the value. If the property did not resolve, then it returns
/// `undefined` and `None` for the prototype.
///
/// The second return value can and should be used to populate the `base_proto`
/// property necessary to make `super` work.
pub fn search_prototype<'gc>(
mut proto: Option<Object<'gc>>,
name: &str,
avm: &mut Avm1<'gc>,
context: &mut UpdateContext<'_, 'gc, '_>,
this: Object<'gc>,
) -> Result<ReturnValue<'gc>, Error> {
) -> Result<(ReturnValue<'gc>, Option<Object<'gc>>), Error> {
let mut depth = 0;

while proto.is_some() {
Expand All @@ -400,12 +462,12 @@ pub fn search_prototype<'gc>(
}

if proto.unwrap().has_own_property(avm, context, name) {
return proto.unwrap().get_local(name, avm, context, this);
return Ok((proto.unwrap().get_local(name, avm, context, this)?, proto));
}

proto = proto.unwrap().proto();
depth += 1;
}

Ok(Value::Undefined.into())
Ok((Value::Undefined.into(), None))
}
Loading