Last Comment Bug 760131 - Quickstub argument unwrapping fails for security-wrapped list proxy and paris binding objects
: Quickstub argument unwrapping fails for security-wrapped list proxy and paris...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Peter Van der Beken [:peterv]
:
Mentors:
Depends on: 769464
Blocks: 734503 750297
  Show dependency treegraph
 
Reported: 2012-05-31 08:18 PDT by Peter Van der Beken [:peterv]
Modified: 2012-07-05 13:57 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (11.47 KB, patch)
2012-06-07 13:24 PDT, Peter Van der Beken [:peterv]
bzbarsky: review+
Details | Diff | Splinter Review

Description Peter Van der Beken [:peterv] 2012-05-31 08:18:35 PDT
This was the cause of bug 743325.
Comment 1 Peter Van der Beken [:peterv] 2012-06-07 13:24:29 PDT
Created attachment 631104 [details] [diff] [review]
v1

The main issue was that xpc_qsUnwrapArgImpl didn't unwrap security wrappers before checking mozilla::dom::binding::instanceIsProxy.
Comment 2 Boris Zbarsky [:bz] 2012-06-07 13:31:27 PDT
Comment on attachment 631104 [details] [diff] [review]
v1

Would it make sense to test for IsDOMClass before InstanceIsProxy in both getWrapper and castNative?  Which do we think will be more common?

r=me in either case.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2012-06-07 23:28:27 PDT
Comment on attachment 631104 [details] [diff] [review]
v1

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

::: js/xpconnect/src/XPCQuickStubs.cpp
@@ +697,5 @@
>      *tearoff = nsnull;
>  
> +    js::Class* clasp = js::GetObjectClass(obj);
> +    if (mozilla::dom::binding::instanceIsProxy(obj) ||
> +        mozilla::dom::IsDOMClass(clasp)) {

You can drop the 'mozilla::' here

@@ +754,5 @@
> +        if (mozilla::dom::binding::instanceIsProxy(cur)) {
> +            native = static_cast<nsISupports*>(js::GetProxyPrivate(cur).toPrivate());
> +            entries = nsnull;
> +        } else if (mozilla::dom::IsDOMClass(clasp)) {
> +            native = mozilla::dom::UnwrapDOMObject<nsISupports>(cur);

And here

@@ +761,4 @@
>              native = static_cast<nsISupports*>(xpc_GetJSPrivate(cur));
>              entries = GetOffsetsFromSlimWrapper(cur);
>          } else {
> +            NS_NOTREACHED("what kind of wrapper is this?");

MOZ_NOT_REACHED?
Comment 4 Peter Van der Beken [:peterv] 2012-06-14 06:21:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fdee4a75df4
Comment 5 Ed Morley [:emorley] 2012-06-15 06:00:34 PDT
https://hg.mozilla.org/mozilla-central/rev/2fdee4a75df4

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