-
-
Notifications
You must be signed in to change notification settings - Fork 894
core: Clean up ExternalInterface a little #18223
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
Conversation
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably write this using an iterator an map.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was old code moved up, so I didn't really touch it - but good idea, changed :)
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup 🎉
… we never use more than 1
…stead of get-then-call
…ultiple providers
a0b3097
to
16c86a0
Compare
Instead of getting a method then calling it, we just call things directly now. We also can now only have one Provider instead of many, but realistically there's no case in which multiple providers can peacefully coexist as they'll be conflicting with methods.