Closed Bug 684083 Opened 11 years ago Closed 11 years ago

crash nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject calling Components.utils.getWeakReference(undefined)

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: mark.yen, Assigned: innomotive)

Details

(Keywords: crash, Whiteboard: [good first bug] [mentor=jdm])

Crash Data

Attachments

(1 file, 2 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-5c51e855-6c33-4d53-9249-326872110901 .
============================================================= 

STR:
1) Open error console
2) Evaluate: Components.utils.getWeakReference(undefined)

Expected results:
"undefined", possibly, or "null".

Actual results: 
crash.

Additional information:
Calling Components.utils.getWeakReference() (with no arguments) doesn't crash; it's only if you supplied undefined as the argument.
Pretty sure you can't reach getWeakReference from unprivileged script...
The first step here for a new contributor taking this bug on would be to reproduce the crash under gdb.
Whiteboard: [good first bug] [mentor=jdm]
Taking this up and working on it.
Assignee: nobody → innomotive
Attached patch Proposed patch (obsolete) — Splinter Review
Proposed patch to fix this bug. Please review.
Attachment #560115 - Flags: review?(mrbkap)
Comment on attachment 560115 [details] [diff] [review]
Proposed patch

Review of attachment 560115 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch! It looks good, but there's a little more cleanup to be done here.

::: js/src/xpconnect/src/xpcJSWeakReference.cpp
@@ +75,3 @@
>      
>      JSObject *obj;
>      if (!JS_ValueToObject(cx, argv[0], &obj))

Since the if statement above excludes primitives, we know here that argv[0] is an object. So instead of calling JS_ValueToObject, this can just set obj to JSVAL_TO_OBJECT(argv[0]).
Attachment #560115 - Flags: review?(mrbkap)
Attached patch Proposed patch (pass 2) (obsolete) — Splinter Review
Patch with review points from Comment 4 included. Should be applied after the previous patch.
Attachment #560303 - Flags: review?
Patch that consolidates previous two patches.

Please review.
Attachment #560115 - Attachment is obsolete: true
Attachment #560303 - Attachment is obsolete: true
Attachment #560303 - Flags: review?
Attachment #560313 - Flags: review?(mrbkap)
Attachment #560313 - Flags: review?(josh)
Comment on attachment 560313 [details] [diff] [review]
Proposed patch (pass 3)

This is all in Blake's hands.
Attachment #560313 - Flags: review?(josh)
Comment on attachment 560313 [details] [diff] [review]
Proposed patch (pass 3)

Looks great! Thanks again.
Attachment #560313 - Flags: review?(mrbkap) → review+
https://hg.mozilla.org/mozilla-central/rev/2b458be3c7b7

Thanks a lot Hari for your patch!  This was a great first patch, and I do hope to see you around in Bugzilla more often.  :-)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.