-
Notifications
You must be signed in to change notification settings - Fork 397
Add stub implementation of ExternalInterface to shell and inspector player #2293
base: master
Are you sure you want to change the base?
Add stub implementation of ExternalInterface to shell and inspector player #2293
Conversation
tschneidereit
commented
Jun 17, 2015
In many cases, getting one message per run per unique warning is enough and everything above that is just spam.
…layer. r=yury Many SWFs are thrown off by exceptions caused by missing ExternalInterface, because they just expect that to be available. With this patch, they get to continue running, at least up until the point where they do an `ExternalInterface.call` and expect something to be returned other than `undefined`.
…line The file got renamed, but apparently the old version persisted.
/botio test |
From: Bot.io (Main)ReceivedCommand cmd_test from @tschneidereit received. Current queue size: 0 Live output at: http://areweflashyet.com:8081/466f0d9968fbdd8/output.txt |
From: Bot.io (Main)SuccessFull output at http://areweflashyet.com:8081/466f0d9968fbdd8/output.txt Total script time: 11.85 mins
|
I don't see why this needed -- the SWF movie in most of the cases check for presence of ExternalInterface and properly handle the exception. |
If it's for mocking the ExternalInterface, could you add it as pref'd off options? |
That's the thing: this is just not true, sadly. In many, many cases, SWFs just assume that ExternalInterface is available.
What's the advantage of having it pref'd off? When do you ever want the inspector or shell to die because of this instead of just logging a warning? |
In default environment, ExternalInterface.available returns false and fails on methods calls. So FP will fail as well (you can notice that when debug build of FP is used). Mocking the external interface, e.g. returning undefined on |
That is a good point to some extent. However, we have 127 exceptions in the ads report that're caused by SWFs not checking for availability of ExternalInterface. I'd bet a serious sum on this causing more problems than the scenario you're worried about. Do you have any idea for how to get the best of both worlds? If not, I think we should make the behavior introduced in this PR the default, and I'll add an option for turning it off. |
Looks good with option to turn it on added -- test runner might turn that option from command line. Reviewed 7 of 7 files at r1. Comments from the review on Reviewable.io |
@tschneidereit please rebase and land if we are still interesting in having this. |