Last Comment Bug 502959 - Permission denied for <> to create wrapper for object of class UnnamedClass
: Permission denied for <> to create wrapper for object of c...
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: 1.9.1 Branch
: x86 Linux
: -- normal with 5 votes (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Andrew Overholt [:overholt]
Depends on: 688416
Blocks: 501962 506249
  Show dependency treegraph
Reported: 2009-07-07 16:00 PDT by John J. Barton
Modified: 2011-09-22 05:08 PDT (History)
17 users (show)
mbeltzner: blocking1.9.1.1-
mbeltzner: wanted1.9.1.x+
mbeltzner: blocking1.9.0.19-
mbeltzner: wanted1.9.0.x-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

De-backout (9.31 KB, patch)
2009-07-22 15:15 PDT, Blake Kaplan (:mrbkap)
jst: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
For checkin (11.44 KB, patch)
2009-07-28 17:04 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
mrbkap: superreview+
dveditz: approval1.9.1.8+
Details | Diff | Splinter Review

Comment 1 John J. Barton 2009-07-07 16:03:33 PDT
Some info from Boris on the newsgroup:
John J. Barton wrote:
> Most of the loads on my machine work ok. But some (30%?) fail with the subject message.

The subject message typically means that script running with the permissions of attempted to access an object that it's not allowed to access.  Specifically, the "wrapper" in this case is the JS reflection of the underlying C++ object.  Permission to create that was denied.  UnnamedClass means that the C++ object did not indicate what kind of object it is (normal for objects not meant to be exposed to JavaScript) and that the error reporter therefore can't tell you what sort of object it was
John J. Barton wrote:
> Ok, maybe we are getting closer. The call stack points to this code in Firebug:
>         if (spy.onreadystatechange)
>             spy.onreadystatechange.handleEvent(event);

OK, this shouldn't obviously be a problem...  assuming |event| could be exposed to web script to start with.

> Here the property onreadystatechange is the request's onreadystatechange:
>      this.onreadystatechange = this.xhrRequest.onreadystatechange;

Right, which is where your safe JS object wrapper came from.

> But the missing bit here is that most of the time the code works. So the problem has to relate to some other condition, maybe a network error or something.

I'd still really like a testcase I could point my browser at to reproduce this.  At the very least I might be able to tell you what the C++ object involved is.
Comment 2 John J. Barton 2009-07-07 16:43:23 PDT
Bug 501962 is about the same issue I guess
Comment 3 Rob Campbell [:rc] (:robcee) 2009-07-07 20:38:49 PDT

*** This bug has been marked as a duplicate of bug 501962 ***
Comment 4 John J. Barton 2009-07-07 20:43:27 PDT
I would like this bug not to be marked duplicate to 501962.

Bug 501962 is about an ajax problem. This bug is about a specific error message with a specific test case that I have already investigated. They may be duplicates but we do not know that.
Comment 5 John J. Barton 2009-07-09 23:10:33 PDT
Any hints Boris? We are getting quite a lot of complaints about this. I can't see anything from the Firebug side that could be related. There is a workaround, but it disables a favorite feature of the very folks who have the problem (ajax traffic tracing, Console > ShowXMLHttpRequests).
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2009-07-10 23:37:28 PDT
John, what are the actual steps to reproduce here?  Install that Firebug version.  Load this site.  Then what?
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2009-07-11 00:02:57 PDT
Ok, I enabled the net panel and by reloading a bunch of times managed to reproduce the bug eventually...

So what's happening here is that onHTTPSpyReadyStateChange (in the firebug code) calls XPC_SJOW_Call (presumably for the safe js object wrapper around the content function), which calls XPC_SJOW_CallWrapper (via some js_Invoke stuff), which calls XPC_WN_CallMethod.  The js stack here is:

0 [native frame]
1 [native frame]
2 anonymous([function], [function], [object Event @ 0x1c144fb0 (native @ 0x1c4def70)]) ["XPCSafeJSObjectWrapper.cpp":812]
    i = 3
    args = [function]
    this = [xpconnect wrapped nsIDOMEventListener @ 0x1c480060 (native @ 0x183bbe60)]
3 [native frame]
4 onHTTPSpyReadyStateChange(event = [object Event @ 0x1c144fb0 (native @ 0x1c4def70)], spy = [object Object]) ["chrome://firebug/content/spy.js":509]
    rethrow = undefined
    error = undefined

Now XPC_WN_CallMethod does the usual xpconnectish stuff until we end up doing XPCWrappedNative::FindTearOff (called from XPCCallContext::CanCallNow). The aInterface passed to FindTearOff is nsIDOMEventListener.  FindTearOff lands us in InitTearOff, which does the CanCreateWrapper check.  In that check, aObj is the XPCWrappedJS for the content function, which is NOT a DOM object (I seem to recall mrbkap hitting that sort of issue in other circumstances too).  The subject principal is the one from the safe js object wrapper, so we fail out of CanCreateWrapper with a security error: only system code is allowed to create wrappers for non-DOM objects that are not security-checked components.

So one obvious issue here is that the XPCWrappedJS is not a DOM object.  As I said, that's come up before.  But more interesting to me is the fact that we end up in InitTearOff at all.  Presumably the tearoff got GCed, which is why this is not reliably reproducible.  But the nsIDOMEventListener interface is what got handed out from the onreadystatechange getter.  Did the chrome code somehow end up with the main jsobject instead of the tearoff jsobject there?  If so, was it supposed to?  It's really weird to me that this tearoff can just go away like that....
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2009-07-11 00:06:08 PDT
OK, I can confirm that XPC_SJOW_CallWrapper passes the mFlatJSObject of our XPCWrappedNative as the |obj| argument to CallWithoutStatics.  Presumably that means that the caller is in fact holding on to the mFlatJSObject....

Why do we have these tearoff jsobjects again?  :(
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2009-07-11 00:11:25 PDT
Sure isn't a DOM issue in any case.  Blake or Peter, thoughts?
Comment 10 John J. Barton 2009-07-11 08:51:05 PDT
(In reply to comment #6)
> John, what are the actual steps to reproduce here?  Install that Firebug
> version.  Load this site.  Then what?

For others:

statusbar icon context menu enable all panels
Console panel > minimenu > showxmlhttprequest On
Reload multiple times
Comment 11 John J. Barton 2009-07-12 22:29:51 PDT
Just before the |try| block that fails, I called jsdIDebuggerService.GC():
        spy.context.onReadySpy = spy; // maybe the handler will eval(), we want the URL.
        if (spy.onreadystatechange)
I get the subject error message every time I reload.
Comment 12 John J. Barton 2009-07-13 11:12:54 PDT
As I understand it, this seems like serious issue which may affect more than just Firebug.  Requesting blocking.
Comment 13 John J. Barton 2009-07-18 20:27:00 PDT
Just to make sure this is known to mozilla: this problem is the number one problem with Firebug on FF 3.5.
Comment 14 Blake Kaplan (:mrbkap) 2009-07-20 19:56:47 PDT
I can't reproduce this on trunk. I'll try 1.9.1 next.
Comment 15 Boris Zbarsky [:bz] (still a bit busy) 2009-07-20 20:16:57 PDT
Blake, I could repro on trunk.... You might want to add the GC() call per comment 11, or ask John to point you to an XPI of firebug with that added.
Comment 16 Mike Beltzner [:beltzner, not reading bugmail] 2009-07-21 20:37:50 PDT
Can't block on this, but would definitely take and are definitely interested in a fix.
Comment 17 Blake Kaplan (:mrbkap) 2009-07-22 15:14:12 PDT
(In reply to comment #8)
> OK, I can confirm that XPC_SJOW_CallWrapper passes the mFlatJSObject of our
> XPCWrappedNative as the |obj| argument to CallWithoutStatics.  Presumably that
> means that the caller is in fact holding on to the mFlatJSObject....

So, the tearoff objects are totally transient. As far as I can tell, they are all collected every time we GC. So it doesn't actually matter which JSObject pointer you have.

> Why do we have these tearoff jsobjects again?  :(

To store the offset C++ pointer. Why they have to be marked and swept, I don't know.

So, re-instating the code bug 493074 removed fixes this bug. Since the backout there didn't actually appear to help the perf regression it was trying to fix, I think (and jst agrees) that we should just check it back in.
Comment 18 Blake Kaplan (:mrbkap) 2009-07-22 15:14:37 PDT
(sorry, mid-aired with beltzner)
Comment 19 Blake Kaplan (:mrbkap) 2009-07-22 15:15:15 PDT
Created attachment 390087 [details] [diff] [review]
Comment 20 John J. Barton 2009-07-22 15:21:47 PDT
After badgering you all, we separately looked into alternatives on the Firebug side and found that we can offer the Firebug ShowXMLHttp request feature without intercepting the onreadystate call. So we don't trigger this problem in our code any more.  Our fix should come out in Firebug 1.4.1, Friday-ish.
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2009-07-22 20:20:49 PDT
Comment on attachment 390087 [details] [diff] [review]

Looks good.  Please add a test (can use XHR and gc() to reproduce reliably).
Comment 22 Blake Kaplan (:mrbkap) 2009-07-28 17:04:16 PDT
Created attachment 391234 [details] [diff] [review]
For checkin

This is ready to go!
Comment 23 Blake Kaplan (:mrbkap) 2009-08-06 20:42:26 PDT
Comment 24 Helder "Lthere" Magalhães 2009-11-21 01:30:19 PST
Firebug issue 2501 [1] notes that this wasn't back-ported. Although a Firebug-specific workaround was made, shouldn't this fix make it to the next 3.5.x release? More than Firebug might currently be affected. ;-)

Of course this should make it to 3.6.x as well, if it's not fixed there yet: I'm not sure about when the branching occurred, but I guess it was before this issue was fixed...?

Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2009-11-21 01:38:28 PST
> I'm not sure about when the branching occurred

A few weeks after this fix was checked in.  You can look that sort of thing up, by the way; the source repository is available and all....
Comment 26 Blake Kaplan (:mrbkap) 2010-01-13 11:45:39 PST
Comment on attachment 391234 [details] [diff] [review]
For checkin

This basically applies as-is (some tweaking to the xpconnect makefile is all).
Comment 27 Daniel Veditz [:dveditz] 2010-01-15 13:23:04 PST
Comment on attachment 391234 [details] [diff] [review]
For checkin

Approved for, a=dveditz for release-drivers
Comment 28 Blake Kaplan (:mrbkap) 2010-01-19 15:40:57 PST

Note You need to log in before you can comment on or make changes to this bug.