Closed Bug 674597 Opened 9 years ago Closed 9 years ago
abort if attempting to create an xpcom proxy for wrapped JS
With bug 667535 and bug 674571 fixed (and maybe a few more), we shouldn't be creating xpcom proxies for wrapped JS. Doing so violates a lot of preconditions. This bug is to create a (release mode) check and abort if this actually occurs in the wild (extensions). bsmedberg is trying to remove xpcom proxies altogether, so this is really just a safety check in the interim.
On second thought, abort() may be a bit severe.
Attachment #548917 - Flags: review?(benjamin)
Try server has revealed two more cases of proxied wrapped JS (one an nsIPrompt, I forget the other), but both were from main thread to main thread, so I just weakened the check.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Version: unspecified → Trunk
If I understand this correctly, add-ons shouldn't be using GetProxyForObject at all, right?
They might, but xpcom/proxy's days are numbered; bsmedberg wants to remove it entirely.
Some add-ons are using it right now. The question is whether we should flag this in the compatibility bump code, and under which circumstances. It would also be good to know how that code can be changed so that it doesn't use proxies.
The abort will hit if the GetProxyForObject is called to proxy wrapped JS (that is, an IDL interface implemented in JS) to some other thread. This has always been unsafe since it runs the wrapped JS's scripted QI immediately (instead of on the destination thread); this bug just catches it eagerly.
Backed out to fix bug 678440 and a few other intermittent crashes: http://hg.mozilla.org/integration/mozilla-inbound/rev/5f0596a0b81e See bug 674571 comment 11 for details.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Shouldn't this also get backed out of aurora (once landed on central)? As these events were either side of the merge?
(In reply to Mark Banner (:standard8) from comment #10) Already done.
AFAICT the backout in comment #9 above fixed bug 678637 too. See also bug 678637 comment #17 for one easy test ("can I start ChatZilla over SSL?") to run the next time a patch lands on this bug.
Thanks for pointing that out Tony. The fix that will enable the patches to reland (bug 675221) should nuke all possibility of this problem, but we'll make sure to retest.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 675221
You need to log in before you can comment on or make changes to this bug.