It exposes a somewhat obscure implementation-internal value, it induces special-case logic in the emitter, and wrappers possibly have to be specifically wary of it. It's also not so great for secure-JS variants like fbjs and cajmumble.
Created attachment 438420 [details] [diff] [review]
Created attachment 438775 [details] [diff] [review]
KILLPARENT^H^H^H^H^H^H^H^H^H^HEr, I mean, this patch tests well, doesn't seem to break anything.
Nice work, Oedipus! :-P
The ratio of boilerplate to substance in nsDOMWindowUtils::GetParent makes me sad.
In this code, I use __parent__ to get the window where a listener object is created; and to remove the listener when the window is to unload.
The entire concept of every object/function having a parent is problematic. It's very easy to get parentage wrong when constructing objects/functions, in ways that lead to security vulnerabilities. The only reason I added functions for accessing it is so testing code can verify assertions about it. I think it unlikely we'll add anything beyond what's already in the patch, or provide more publicly visible access to the removed functionality.
Your use case doesn't require parent information, though, just global object information. I think we might be willing to expose JS_GetGlobalForObject on Components.utils or somewhere else, it being less problematic and obscure than parentage, or so I understand. Thoughts from other SpiderMonkey hackers appreciated on this idea here, will file new bug if people think this sounds reasonable. (That said, you're aware that walking up the JS stack like that is an easy way to slow SpiderMonkey down, right? It throws you out of the JIT at the moment and back into the interpreter.)
That said, why do you need this information anyway? My understanding has always been that manual event listener addition/removal like this is only necessary in old browsers like IE7 (or 6?) and Firefox 2 (or 1.x?), and that it's a waste of time elsewhere.
To borrow from the Emperor in "Amadeus", too many words :-P.
I think Nanto has a point, independent of his use-case's particulars -- I wish I had seen it sooner. Components.utils is better than nsWindowUtils.
(In reply to comment #6)
> To borrow from the Emperor in "Amadeus", too many words :-P.
> I think Nanto has a point, independent of his use-case's particulars -- I wish
> I had seen it sooner. Components.utils is better than nsWindowUtils.
The sample code applies __parent__ to a function or a global object. So perhaps we should just provide access to __parent__ only for functions, global objects and DOM nodes that could also present on the scope chain for event listeners?
(In reply to comment #5)
> Your use case doesn't require parent information, though, just global object
> information. I think we might be willing to expose JS_GetGlobalForObject on
> Components.utils or somewhere else, it being less problematic and obscure than
> parentage, or so I understand.
You're right. What I really want is a way to get the global object from an object. It's better if we can get global directly.
> (That said, you're aware that walking up the JS stack like that is
> an easy way to slow SpiderMonkey down, right? It throws you out of the JIT at
> the moment and back into the interpreter.)
The code referred to in comment 4 is an older version. I'm writing newer version of that but it's not yet public. In newer version, I try to get global from a listener object, not from a function in which the listener is added, so arguments.callee.caller is no longer used.
> That said, why do you need this information anyway? My understanding has
> always been that manual event listener addition/removal like this is only
> necessary in old browsers like IE7 (or 6?) and Firefox 2 (or 1.x?), and that
> it's a waste of time elsewhere.
work, now that indirect eval is global eval? I haven't tested, I confess.
eval is no longer a property of Object.prototype, only of the global object. Or do I misunderstand your idea?
Nope, I forgot that we'd fixed that too.
One way to get an object's global object was |obj.valueOf.call(null)|. I think jorendorff has a patch though to make that return the caller's global instead valueOf's global.
(In reply to comment #12)
> One way to get an object's global object was |obj.valueOf.call(null)|. I think
> jorendorff has a patch though to make that return the caller's global instead
> valueOf's global.
What bug is that?
Why is caller's global the right answer?
It does kind of seem to me like valueOf should return "its" own global, actually, contorted tho the idea is to reason through, and ugly to boot.
From reading jorendorff's patch over in bug 556277, it appears I misread a comment he made somewhere... never mind me!
Created attachment 443485 [details] [diff] [review]
Followup test fixes since parent() is shell-only, not in browser
I ran jstests but not browser jstests, so missed these. If you follow the bug reports in, one requires a parent-alike and a with, but since neither are available in unprivileged script it's easiest to just remove the test entirely. Another is an assertion typo that probably (bug is unclear) depended on access of a "special" property, but parent() isn't, so it's useless too. A third relies on Block objects not being censored, but we've done that for ages. The fourth, I just if-guarded the parent checks -- and because those were comparing parents of distinct objects, a checkParent(obj, parent, msg) function wouldn't work anyway, only a checkParents(obj1, obj2, msg) -- but it seemed like overkill for only a single function, so not worth it.
The last change effectively disabled a chrome-workers ctypes test (new since the test was written? wasn't failing when I last ran tests), in the interest of quelling an orange to permit more careful consideration of how to fix the problem (namely, that there's no parent() in chrome workers at present -- which change seems like a bad idea for workers). Still deciding what to do here, exactly; will mark f-i-t once I decide that issue.
Created attachment 443729 [details] [diff] [review]
Potential patch for ctypes bustage
This reenables the ctypes test and makes parent checks conditional on the presence of parent(). I haven't tested yet -- need to rebuild my tree, which will take a litle time -- but I'm pretty sure it's good, so optimistically jumping the gun on the request.
Comment on attachment 443729 [details] [diff] [review]
Potential patch for ctypes bustage
for the ctypes parent-check-guarding fix -- so we're all set here now! \o/ Onward and upward...
I've marked it obsolete in documentation I've found, but it still needs a note in some sort of overview document.