562431 - Rewrite WeaveCrypto as a .jsm

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[(dormant account)]

Having it as an xpcom component incurs needless overhead.

Comment 1

[Justin Dolske [:Dolske]]

Should definitely do this, probably for 2.0 (?) when support for Firefox 3.6 and its binary WeaveCrypto component is dropped.

Comment 2

[Mike Connor [:mconnor]]

2.0 is a planned add-on-based release around June (it might get renumbered).

Future for now, but definitely wanted in the Fx4 timeframe.

Comment 3

[Philipp von Weitershausen [:philikon]]

This is a pre requisite for bug 570619 (which already has a patch for it; I was unaware of this bug; will move patch over).

Comment 4

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1 (v1): Make WeaveCrypto.js a JSM

This is pretty much what we had in bug 570619 already. Copying comment 6:

  For some reason this breaks one test (test_crypto_crypt.js) because it
  used an improper base64 string for the 'badiv' variable (trailing =);
  the failure origins from bug 439276. Making this a proper base64 string
  makes the tests pass but fail for the binary component.

Comment 5

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (v1): Move the add-on test harness

Move add-on test harness to the 'addon' directory and make it reusable, in preparation of Part 3 where we move the WeaveCrypto related tests to services/crypto/

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: Part 3 (v1): Move tests for WeaveCrypto to services/crypto/

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: Build fixes on m-c

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 481747 [details] [diff] [review]
Part 1 (v1): Make WeaveCrypto.js a JSM


>-# WeaveCrypto.js
>+# IWeaveCrypto.xpt
> interfaces components/IWeaveCrypto.xpt
>-component {7fa20841-c90e-4432-a1a1-ba3b20cb6b37} components/WeaveCrypto.js
>-contract @labs.mozilla.com/Weave/Crypto;2 {7fa20841-c90e-4432-a1a1-ba3b20cb6b37}

If we're not using this in Gecko 2.0, do we need to register the xpt at all?  I don't see why we would, trunk doesn't have an impl, and we shouldn't hit this...

Comment 9

[Philipp von Weitershausen [:philikon]]

(In reply to comment #8)
> If we're not using this in Gecko 2.0, do we need to register the xpt at all?

We still refer to Ci.IWeaveCrypto in the code. But of course trunk comes with its own, already registered Ci.IWeaveCrypto. At least since 4.0b4. If we don't care about the add-on working on previous betas, then yes, we can get rid of this.

Comment 10

[Philipp von Weitershausen [:philikon]]

Part 1: http://hg.mozilla.org/services/fx-sync/rev/f62e573d9c35
Part 2: http://hg.mozilla.org/services/fx-sync/rev/cd0f2e5b5ea6
Part 3: http://hg.mozilla.org/services/fx-sync/rev/518f949b58d6

m-c build fixes will have to land together with the next merge (bug 603388)

Comment 11

[Philipp von Weitershausen [:philikon]]

Attachment: Build fixes on m-c (v2)

Update build fixes patch to include test move.

Comment 12

[Philipp von Weitershausen [:philikon]]

Merged to m-c, landed build fix: http://hg.mozilla.org/mozilla-central/rev/19cb42fa4554