The AIR runtime requires the existence of ScriptObject::isGlobalObject; we currently use the kIsScriptEnv bit on MethodEnv/ScriptEnv to implement this... which works, but is awkward and makes for extra bit-twiddling in the already-overloaded activationOrMCTable mess in MethodEnv. A much simpler approach would be to make isGlobalObject() a virtual function (defaulting to false), and make a new subclass of ScriptObject (used only when we allocate the ScriptEnv's global field) that returns true. Unlikely to be any meaningful net gain or loss in either code or performance, but resulting code should be cleaner IMHO. (Note that there is already an acceptance test: acceptance/misc/isGlobalObject.abc)
Created attachment 474896 [details] [diff] [review] Patch
Attachment #474896 - Flags: review?(edwsmith)
A bit on Traits would also do it, without using native subclassing; the bit could be set right in AbcParser.
Better yet, we can just check for Traits::posType() == TRAITSTYPE_SCRIPT
Created attachment 475573 [details] [diff] [review] Patch v2
Comment on attachment 475573 [details] [diff] [review] Patch v2 Much cleaner, R+ with nits below addressed, one way or another. MethodEnv::kIsScriptEnv looks dead now. remove? A few carefully placed asserts could check that ScriptObject::isGlobalObject() returns the correct value. I think we only allocate global objects in one place, and that place should return true, all other allocation sites. Other testing ideas welcome.
Attachment #475573 - Flags: review?(edwsmith) → review+
(In reply to comment #5) > MethodEnv::kIsScriptEnv looks dead now. remove? Oops. Yeah.
pushed with nits: http://hg.mozilla.org/tamarin-redux/rev/9f9d08440c3e
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.