Last Comment Bug 763341 - Chrome QI of content object fails for new bindings
: Chrome QI of content object fails for new bindings
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Bobby Holley (PTO through June 13)
:
Mentors:
Depends on: 818281
Blocks: 762492
  Show dependency treegraph
 
Reported: 2012-06-10 13:04 PDT by Bobby Holley (PTO through June 13)
Modified: 2012-12-04 16:53 PST (History)
1 user (show)
bobbyholley: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Handle wrappers in new dom binding QI. v1 (5.19 KB, patch)
2012-06-10 13:07 PDT, Bobby Holley (PTO through June 13)
bzbarsky: review+
Details | Diff | Review

Description Bobby Holley (PTO through June 13) 2012-06-10 13:04:36 PDT
A common thing that happens in our test suite is:

enablePrivilege('UniversalXPConnect');
someDOMObject.QueryInterface(Ci.foo).doSomething;

The easiest way to fix this is to use SpecialPowers.wrap(someDOMObject). But this fails for new bindings, because our QueryInterface implementation doesn't unwrap.

As long as we're going to support this pseudo-QI implementation for the new bindings, we should make sure it works from chrome too. Patch coming up.
Comment 1 Bobby Holley (PTO through June 13) 2012-06-10 13:07:25 PDT
Created attachment 631766 [details] [diff] [review]
Handle wrappers in new dom binding QI. v1

The |ok| parent check goes away because it's ill-conceived. All it's really checking is that proxies are parented to the global, and it's impossible to port to SpecialPowers, because SpecialPowers will think that the proxy is parented to the SpecialPowers global. I don't think this check is very important, so I removed it.
Comment 2 :Ms2ger 2012-06-11 00:34:32 PDT
Comment on attachment 631766 [details] [diff] [review]
Handle wrappers in new dom binding QI. v1

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

::: js/xpconnect/tests/chrome/test_wrappers.xul
@@ +70,5 @@
> +
> +   // Test QI on new dom bindings.
> +   var contentXHR = XPCNativeWrapper(win.wrappedJSObject.xhr);
> +   try {
> +     QIed = contentXHR.QueryInterface(Components.interfaces.nsIClassInfo);

var QIed?
Comment 3 Boris Zbarsky [:bz] 2012-06-11 19:32:10 PDT
Comment on attachment 631766 [details] [diff] [review]
Handle wrappers in new dom binding QI. v1

r=me
Comment 4 Bobby Holley (PTO through June 13) 2012-06-12 03:37:47 PDT
Pushed to try, along with another patch: https://tbpl.mozilla.org/?tree=Try&rev=29d25bdd5cc2
Comment 5 Bobby Holley (PTO through June 13) 2012-06-12 06:45:41 PDT
Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/b4dbb556a619
Comment 6 Matt Brubeck (:mbrubeck) 2012-06-12 18:33:42 PDT
https://hg.mozilla.org/mozilla-central/rev/b4dbb556a619

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