567642 - Need to update "Using the SDK with XUL extensions" (harness service contract ID is different in cfx run and cfx xpi)

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Nickolay_Ponomarev]

To use Jetpack SDK with regular extensions <https://jetpack.mozillalabs.com/sdk/latest/docs/#guide/xul-extensions> it's important to be able to access the jetpack loader functionality from code not yet in jetpack modules.

It used to be possible to do that by accessing the harness service by contract id "@mozilla.org/harness-service;1?id=[e-mail]", where [e-mail] was the id in package.json.

I just updated to the current tip and apparently the id of the harness service is now hardcoded when using 'cfx run':
http://hg.mozilla.org/labs/jetpack-sdk/diff/20cce16d1041/python-lib/cuddlefish/__init__.py
https://bugzilla.mozilla.org/show_bug.cgi?id=566812

Why? How should the XUL extensions doc be updated?

This should block 0.4, unless I'm missing something obvious.

Comment 1

[Atul Varma [:atul]]

Oops, thanks for filing this, I mentioned it to warner when working on bug 553020 but forgot to make a bug on it.

warner, any idea what the new value is? I think it includes the jetpack ID in it now, but I'm not sure what its exact form is.

I agree with Nickolay--sounds like this should be a blocker, since it's completely incorrect documentation at this point. Since it's just a documentation change, the risk is low.

Comment 2

[Nickolay_Ponomarev]

I have to use @mozilla.org/harness-service;1?id=6724fc1b-3ec4-40e2-8583-8061088b3185 when running via 'cfx run', but @mozilla.org/harness-service;1?id=jid0-4g7AasBscUrADY8rYIbIJ5BmrUY in the packaged extension, so unless I'm missing something it's not only docs.

Comment 3

[Brian Warner [:warner :bwarner]]

so, the current behavior of the SDK (not saying this is ideal or correct) is that "cfx xpi" sets "harness_contract_id" equal to a value derived from the cryptographic JID (which is stored in package.json). "cfx run", on the other hand, uses a harness_contract_id derived from one of two static values (that 6724fc1b- value when "options.use_server" is false, and a 2974c5b5- value when use_server is true).

Atul and I talked about making "cfx run" and "cfx xpi" behave the same way here: both would do the preflight/JID-creation checks (which are currently only done for "cfx xpi"), and then the contract_id would *always* be derived from the JID.

The harness class ID would continue to be built in the old way (static value for cfx run, random valud for cfx xpi).

Does that approach seem reasonable? I have no idea what needs to be consistent with what.

Comment 4

[Atul Varma [:atul]]

I think that sounds reasonable--the only thing that needs to stay consistently the same is the contract ID, the class ID can vary all it wants to.

It would probably be helpful to add a blurb to the "Using the SDK with XUL extensions" appendix which mentions that you should really make a keypair for your addon before accessing it from traditional platform code, otherwise the harness contract ID will change on you.

Comment 5

[Brian Warner [:warner :bwarner]]

If the first call to "cfx run" creates the keypair, will that suffice? That is, is it possible to "access it from traditional platform code" through some mechanism other than "cfx run"/"cfx xpi"?

I'm coding up a patch now.

Comment 6

[Brian Warner [:warner :bwarner]]

Attachment: first patch: use JID for both 'cfx run' and 'cfx xpi'

not sure this works yet, "cfx testall" seems to hang

Comment 7

[Brian Warner [:warner :bwarner]]

hrm, the hang seemed to be unrelated to my patch.. not sure what's going on now (I restarted one copy of FF in the meantime), but now the tests aren't hanging anymore.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

I also just tested on most combinations of (Windows, Mac, Linux) - (Firefox, XULRunner) - (Trunk, 3.6) - (with patch, without patch), and I didn't experience a hang on any of them.

Comment 9

[Atul Varma [:atul]]

The patch looks good, btw, so feel free to land it.

Comment 10

[Brian Warner [:warner :bwarner]]

Landed, in ff4524778f33, with a tiny change to mention rerunning "cfx %s"%command instead of always saying to rerun "cfx xpi".

Comment 11

[Nickolay_Ponomarev]

The patch fixes the problem, but I fail to see where you landed it. It's not in the current tip (621:0dc264ba9017): http://hg.mozilla.org/labs/jetpack-sdk/file/0dc264ba9017/python-lib/cuddlefish/__init__.py#l407

Comment 12

[Brian Warner [:warner :bwarner]]

Oops, I'm an idiot, I pushed from an http-based repo and didn't notice the (tiny) error message. I've rebased the patch to tip and repushed. 
http://hg.mozilla.org/labs/jetpack-sdk/rev/b519a0848585

Sorry about that..

Comment 13

[Nickolay_Ponomarev]

OK, so run and xpi now use the correct ID, but 'cfx test' still uses a hardcoded one... Sorry for not thinking of it before!

The fix similar to above for 'test' would sprinkle 'id' properties over all package.json files, even those that are module-packages, not program-packages.

Perhaps we should generate the ID only if there's an invalid ID field or the user's running 'cfx xpi' (and maybe 'cfx run', not sure)? If there's no ID, default to a constant in the harness contract ID.

Comment 14

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".