nsScriptError::Init should assert for sourceName

RESOLVED INACTIVE

Status

()

RESOLVED INACTIVE
13 years ago
3 months ago

People

(Reporter: WeirdAl, Unassigned)

Tracking

(Depends on: 3 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 1

13 years ago
Created attachment 199745 [details] [diff] [review]
patch

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

Updated

13 years ago
Depends on: 312634
(Reporter)

Updated

13 years ago
Depends on: 312648
(Reporter)

Updated

13 years ago
Depends on: 312651
(Reporter)

Comment 2

13 years ago
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.
(Reporter)

Updated

13 years ago
Depends on: 209701
(Reporter)

Updated

13 years ago
Depends on: 313094
(Reporter)

Comment 3

13 years ago
Comment on attachment 199745 [details] [diff] [review]
patch

:( Too many bites on the second assertion.
Attachment #199745 - Attachment is obsolete: true
(Reporter)

Comment 4

13 years ago
Created attachment 200174 [details] [diff] [review]
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.
Attachment #200174 - Flags: superreview?(bzbarsky)
Attachment #200174 - Flags: review?(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...
(Reporter)

Updated

13 years ago
Attachment #200174 - Attachment is obsolete: true
Attachment #200174 - Flags: superreview?(bzbarsky)
Attachment #200174 - Flags: review?(bzbarsky)
(Reporter)

Updated

13 years ago
Summary: nsScriptError::Init should assert for sourceName, sourceLine → nsScriptError::Init should assert for sourceName
(Reporter)

Comment 6

13 years ago
Created attachment 200204 [details] [diff] [review]
patch

Just add the assertion, then, for source name.
Attachment #200204 - Flags: superreview?
Attachment #200204 - Flags: review?(bzbarsky)
(Reporter)

Updated

13 years ago
Attachment #200204 - Flags: superreview? → superreview?(bzbarsky)
Comment on attachment 200204 [details] [diff] [review]
patch

Need to test *sourceName too.
Attachment #200204 - Flags: superreview?(bzbarsky)
Attachment #200204 - Flags: superreview-
Attachment #200204 - Flags: review?(bzbarsky)
Attachment #200204 - Flags: review-
(Reporter)

Comment 8

13 years ago
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
...
Do we have a bug filed on that?
(Assignee)

Updated

10 years ago
Product: Core → SeaMonkey

Comment 10

8 years ago
Moving to Core::XPConnect
Assignee: error-console → nobody
Component: Error Console → XPConnect
Product: SeaMonkey → Core
QA Contact: jrgmorrison → xpconnect

Comment 11

3 months ago
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.
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.