312630 - nsScriptError::Init should assert for sourceName

Status: RESOLVED INACTIVE

(Core :: XPConnect, defect)

Description

[Alex Vincent [:WeirdAl]]

In a discussion over IRC, bz, biesi and I realized we should always check to
make sure nsScriptError has sourceName and sourceLine defined for it.  This bug
is to add assertions to that effect.

The goal is to fix any callers that pass in null for sourceName or sourceLine,
and to ensure nobody does it in the future.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: patch

FYI, the discussion for this was borne of bug 248801.

Comment 2

[Alex Vincent [:WeirdAl]]

Bug 312634 and bug 312651 are both a result of the second assertion, about
sourceLine.  I've hit no instances of the first assertion, but bug 312648 is a
case we're just begging to hit.

I would not be entirely opposed to making the second assertion a warning, but I
would still like to fix the callers.

Comment 3

[Alex Vincent [:WeirdAl]]

Comment on attachment 199745 [details] [diff] [review]
patch

:( Too many bites on the second assertion.

Comment 4

[Alex Vincent [:WeirdAl]]

Attachment: patch

I've been running with the initial patch for quite a while, and would often hit
the second assertion (which this patch now makes a warning).  However, I have
never hit the first assertion.	This patch is safe.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Please don't add the warning until we've at least fixed the known bugs on that
issue.  No point cluttering up the debug output with random crap...

Comment 6

[Alex Vincent [:WeirdAl]]

Attachment: patch

Just add the assertion, then, for source name.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 200204 [details] [diff] [review]
patch

Need to test *sourceName too.

Comment 8

[Alex Vincent [:WeirdAl]]

bz: any call to a javascript: url that has an error in it results in this
assertion being hit.

javascript:alert(Components.classes["FOO"])

00 ntdll!DbgBreakPoint
01 xpcom_core!nsDebugImpl::Break(char * aFile = 0x00e7e2e0
"m:/mozilla/js/src/xpconnect/src/nsScriptError.cpp", int aLine = 131)+0x7e
02 xpcom_core!nsDebugImpl::Assertion(char * aStr = 0x00e7e330 "Need a non-null
URI for the console service!", char * aExpr = 0x00e7e314 "sourceName &&
*sourceName", char * aFile = 0x00e7e2e0
"m:/mozilla/js/src/xpconnect/src/nsScriptError.cpp", int aLine = 131)+0x29e
03 xpcom_core!NSGlue_Assertion(char * aStr = 0x00e7e330 "Need a non-null URI for
the console service!", char * aExpr = 0x00e7e314 "sourceName && *sourceName",
char * aFile = 0x00e7e2e0 "m:/mozilla/js/src/xpconnect/src/nsScriptError.cpp",
int aLine = 131)+0x4d
04 xpc3250!nsScriptError::Init(unsigned short * message = 0x04358ed0, unsigned
short * sourceName = 0x100f6c50, unsigned short * sourceLine = 0x00000000,
unsigned int lineNumber = 0, unsigned int columnNumber = 0, unsigned int flags =
2, char * category = 0x01abad80 "content javascript")+0x2d
05 gklayout!NS_ScriptErrorReporter(struct JSContext * cx = 0x038595a8, char *
message = 0x03c57d20 "uncaught exception: Permission denied to get property
UnnamedClass.classes", struct JSErrorReport * report = 0x0012a104)+0x3c7
06 js3250!js_ReportErrorAgain(struct JSContext * cx = 0x038595a8, char * message
= 0x03c57ca8 "uncaught exception: Permission denied to get property
UnnamedClass.classes", struct JSErrorReport * reportp = 0x0012a104)+0xc9
07 js3250!ReportError(struct JSContext * cx = 0x038595a8, char * message =
0x03c57ca8 "uncaught exception: Permission denied to get property
UnnamedClass.classes", struct JSErrorReport * reportp = 0x0012a104)+0x4e
08 js3250!js_ReportErrorNumberVA(struct JSContext * cx = 0x038595a8, unsigned
int flags = 0, <function> * callback = 0x00cf1efb, void * userRef = 0x00000000,
unsigned int errorNumber = 0x93, int charArgs = 1, char * ap = 0x0012a16c
"???")+0xe3
09 js3250!JS_ReportErrorNumber(struct JSContext * cx = 0x038595a8, <function> *
errorCallback = 0x00cf1efb, void * userRef = 0x00000000, unsigned int
errorNumber = 0x93, char * ap = 0x0012a16c "???")+0x27
0a js3250!js_ReportUncaughtException(struct JSContext * cx = 0x038595a8)+0x308
0b js3250!JS_EvaluateUCScriptForPrincipals(struct JSContext * cx = 0x038595a8,
struct JSObject * obj = 0x03c78618, struct JSPrincipals * principals =
0x03ca8e0c, unsigned short * chars = 0x0012a30c, unsigned int length = 0x20,
char * filename = 0x0012a4b0 "javascript:alert(Components.classes["FOO"])",
unsigned int lineno = 1, long * rval = 0x0012a268)+0x95
0c gklayout!nsJSContext::EvaluateString(class nsAString_internal * aScript =
0x0012a2f4, void * aScopeObject = 0x03c78618, class nsIPrincipal * aPrincipal =
0x03ca8e08, char * aURL = 0x0012a4b0
"javascript:alert(Components.classes["FOO"])", unsigned int aLineNo = 1, char *
aVersion = 0x00000000 "", class nsAString_internal * aRetValue = 0x0012a4f4, int
* aIsUndefined = 0x0012a578)+0x2cf
0d gklayout!nsJSThunk::EvaluateScript(class nsIChannel * aChannel =
0x03ddaa20)+0xe1d
0e gklayout!nsJSChannel::InternalOpen(int aIsAsync = 1, class nsIStreamListener
* aListener = 0x03ddaaf0, class nsISupports * aContext = 0x00000000, class
nsIInputStream ** aResult = 0x00000000)+0xb3
0f gklayout!nsJSChannel::AsyncOpen(class nsIStreamListener * aListener =
0x03ddaaf0, class nsISupports * aContext = 0x00000000)+0x17
10 docshell!nsDocumentOpenInfo::Open(class nsIChannel * aChannel = 0x0432c730)+0x74
11 docshell!nsURILoader::OpenURI(class nsIChannel * channel = 0x0432c730, int
aIsContentPreferred = 0, class nsIInterfaceRequestor * aWindowContext =
0x03867e70)+0x498
...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Do we have a bug filed on that?

Comment 10

[Philip Chee]

Moving to Core::XPConnect

Comment 11

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.