234703 - mClassInfoFlags is always 0 instead of using the XPCWrappedNativeProto(..., ClassInfoFlags, ...) value

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[timeless]

While working on bug 81879, i tripped over the fact that mClassInfoFlags is
always 0.

Testcase (for debug xpcshell):

var gES =
Components.classes["@mozilla.org/exceptionservice;1"].getService(Components.interfaces.nsIExceptionService);
print(gES.currentExceptionManager);
function MyThreadType() {}
MyThreadType.prototype = {Run: function() {var es =
Components.classes["@mozilla.org/exceptionservice;1"].getService(Components.interfaces.nsIExceptionService);
print(es.currentExceptionManager); }}
const tconstructor=new Components.Constructor("@mozilla.org/thread;1", "nsIThread");
var mythread, myrunnable;
mythread = new tconstructor(); myrunnable = new MyThreadType();
mythread.init(myrunnable, 8000, 1, 1, 1);

The code was introduced in 1.1.10.2 of xpcwrappednativeproto.cpp

Fixing this unfortunately requires significant testing as it means we'll be
exercising code which should have been exercised for years but hasn't.

Comment 1

[timeless]

Attachment: use the value passed to the constructor

Comment 2

[timeless]

Attachment: Current output for reference

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 141641 [details] [diff] [review]
use the value passed to the constructor

sr=jst, this for sure looks like what we want, but I'd *really* like to hear
dbradley's thoughts on this change.

Comment 4

[Johnny Stenback (:jst)]

er, that was r=jst, or whatever :-)

Comment 5

[David Bradley]

Judging from the GetNewOrUsed this was intended to be used. I assume it was some
annomoly left in when the code was created. That said, it's not without risk,
and would need testing. I'd get it in at the beginning of the upcomming release
cycle so it gets plenty of exposure as well as ofcourse a thorough run of the
smoke tests.

What's odd is that with the current code it sets the don't share flag but that's
really not going to have any effect by itself with the current code. So this
will be excersizing some interesting code that could have interesting
sideffects. If we're lucky all good ones.

so I'm fine with the change. I'll see if I can get some time and walk some of
the code and see if I can figure out any pitfalls.

Oddly enough I wonder if this might be the source of one of my recent IDispatch
issues. Probably not, but deals with similar stuff.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 141641 [details] [diff] [review]
use the value passed to the constructor

sr=bzbarsky

Comment 7

[timeless]

Checked in:
mozilla/js/src/xpconnect/src/xpcwrappednativeproto.cpp 	1.13

Phil: this needs significant testing of mozilla, derivatives, test apps, and
probably testing by a large userbase. I'd say that it needs baking for at least
two weeks, maybe a month or the milestone. Unfortunately I can't describe a
decent set of testcases.

DOM is probably the biggest user of the flags, so perhaps bc's testsuite...

Comment 8

[John Bandhauer]

doh! I didn't trace through all the usage cases to see what the impact of the
change would be, but this is *clearly* what I meant to write in the first place.
Sorry. John.