Last Comment Bug 684083 - crash nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject calling Components.utils.getWeakReference(undefined)
: crash nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject calling Components.ut...
Status: RESOLVED FIXED
[good first bug] [mentor=jdm]
: crash
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Hari R
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-01 16:18 PDT by Mook (as)
Modified: 2011-09-15 07:41 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (757 bytes, patch)
2011-09-14 01:29 PDT, Hari R
no flags Details | Diff | Splinter Review
Proposed patch (pass 2) (894 bytes, patch)
2011-09-14 22:22 PDT, Hari R
no flags Details | Diff | Splinter Review
Proposed patch (pass 3) (1002 bytes, patch)
2011-09-14 23:13 PDT, Hari R
mrbkap: review+
Details | Diff | Splinter Review

Description Mook (as) 2011-09-01 16:18:05 PDT
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...
Comment 1 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-09-02 06:38:32 PDT
The first step here for a new contributor taking this bug on would be to reproduce the crash under gdb.
Comment 2 Hari R 2011-09-13 22:56:51 PDT
Taking this up and working on it.
Comment 3 Hari R 2011-09-14 01:29:41 PDT
Created attachment 560115 [details] [diff] [review]
Proposed patch

Proposed patch to fix this bug. Please review.
Comment 4 Blake Kaplan (:mrbkap) 2011-09-14 15:27:06 PDT
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]).
Comment 5 Hari R 2011-09-14 22:22:01 PDT
Created attachment 560303 [details] [diff] [review]
Proposed patch (pass 2)

Patch with review points from Comment 4 included. Should be applied after the previous patch.
Comment 6 Hari R 2011-09-14 23:13:03 PDT
Created attachment 560313 [details] [diff] [review]
Proposed patch (pass 3)

Patch that consolidates previous two patches.

Please review.
Comment 7 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-09-14 23:19:44 PDT
Comment on attachment 560313 [details] [diff] [review]
Proposed patch (pass 3)

This is all in Blake's hands.
Comment 8 Blake Kaplan (:mrbkap) 2011-09-14 23:24:48 PDT
Comment on attachment 560313 [details] [diff] [review]
Proposed patch (pass 3)

Looks great! Thanks again.
Comment 9 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-09-15 01:15:14 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/2b458be3c7b7
Comment 10 :Ehsan Akhgari 2011-09-15 07:41:14 PDT
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.  :-)

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