763847 - SpecialPowers.wrap having a hard time wrapping an object

Status: NEW

(Testing :: Mochitest, defect)

Description

[[e-mail] [:onecyrenus]]

Basically a patch has tests failing. 

https://hg.mozilla.org/integration/mozilla-inbound/raw-rev/3eded0507a6e

The test can be run

TEST_PATH=dom/tests/mochitest/webapps/test_install_app.xul make -C obj-ff-dbg/ mochitest-chrome

inside webapps/jshelper ~ line 202.

We are using SpecialPowers.wrap on the return from navigator.mozApps.install. 
An example how to invoke that yourself would be on 

http://people.mozilla.com/~fdesre/openwebapps/test.html

If you look at the install function, and compare with what is happening on line 202 you should be able to come up with a fairly isolated example. 

I am imagining the object is of sufficient complexity that doesn't allow itself to be wrapped, and it blocks the test, and causes a failure?

Comment 1

[Bobby Holley (:bholley)]

If you generate a reduced testcase and file a bug blocking bug 762492, I'll take a look.

Comment 2

[[e-mail] [:onecyrenus]]

Bobby do you want me to attach the test case to the bug, your mozilla central would need to be patched as well.

Comment 3

[[:fabrice] Fabrice Desré]

Here's the interface implemented by the object that fails to wrap :

interface mozIDOMApplication  : nsISupports
{
  readonly attribute jsval manifest;
  readonly attribute DOMString manifestURL;
  readonly attribute nsIArray receipts; /* an array of strings */
  readonly attribute DOMString origin;
  readonly attribute DOMString installOrigin;
  readonly attribute unsigned long installTime;
  readonly attribute double progress;
  readonly attribute DOMString status;

  attribute nsIDOMEventListener onprogress;
  
  /* startPoint will be used when several launch_path exists for an app */
  nsIDOMDOMRequest launch([optional] in DOMString startPoint);
  nsIDOMDOMRequest uninstall(); 
};

Comment 4

[[e-mail] [:onecyrenus]]

Attachment: A simple TestCase example

Run this mochitest before / after you have patched moz-central
You have to click the install button when the test runs.

Comment 5

[Bobby Holley (:bholley)]

Why are you using SpecialPowers wrapping at all in chrome? The whole point of it is to proxy access to objects into chrome code (which runs with system principal). Doing it in the context of this test-case doesn't make sense.

Comment 6

[[e-mail] [:onecyrenus]]

How would you suggest it be done ? 

I think the person who added the SpecialPowers.wrap, may not have known this.

Comment 7

[Bobby Holley (:bholley)]

(In reply to [e-mail] from comment #6)
> How would you suggest it be done ? 

What, exactly? If you're writing code in a .xul file, it's automatically privileged. You don't need any SpecialPowers.wrap stuff at all.

Comment 8

[[e-mail] [:onecyrenus]]

changeset:   94844:bb8a34106b05
user:        Bobby Holley <[e-mail]>
date:        Thu May 24 13:04:57 2012 +0200
summary:     Bug 553102 - Fix open web apps. r=fabrice

I don't know why it is written this way ? Do you know why ?

Comment 9

[[:fabrice] Fabrice Desré]

So, I took a deeper look at the issue, and I think we have to refactor the tests. Here's why:

Calls to mozApps apis return there results on a DOMRequest.result property. Before the patch in bug 702369, the result was a JS object, which is why the tests could add a |status| property on it. Now it is a proper xpcom object, and you can't add a property to a wrapped native.

So I think we should close this bug as invalid and open a new one to fix the tests.

Comment 10

[Bobby Holley (:bholley)]

(In reply to [e-mail] from comment #8)
> I don't know why it is written this way ? Do you know why ?

Hm, that's a good point! I'm not sure why the SpecialPowers was needed there - the structure of that test is kind of a blur to me now.