651596 - Eliminate IWeaveCrypto

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Richard Newman [:rnewman]]

services/crypto/modules/WeaveCrypto.js
48:const ALGORITHM                 = Ci.IWeaveCrypto.AES_256_CBC;
58:    QueryInterface: XPCOMUtils.generateQI([Ci.IWeaveCrypto]),
383:    // IWeaveCrypto interfaces

services/crypto/tests/unit/test_crypto_crypt.js
8:                .getService(Ci.IWeaveCrypto);

services/crypto/tests/unit/test_crypto_random.js
8:                .getService(Ci.IWeaveCrypto);
54:  cryptoSvc.algorithm = Ci.IWeaveCrypto.AES_256_CBC;

services/sync/tests/unit/head_helpers.js
182: * Mock implementation of IWeaveCrypto.  It does not encrypt or

services/sync/tests/unit/test_utils_deriveKey.js
8:                .getService(Ci.IWeaveCrypto);

Comment 1

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

This builds, passes tests, and doesn't generate IWeaveCrypto.idl in output.

On r+ I'll throw it at try.

Comment 2

[Philipp von Weitershausen [:philikon]]

Comment on attachment 527375 [details] [diff] [review]
Proposed patch. v1

>diff --git a/services/sync/tests/unit/test_utils_deriveKey.js b/services/sync/tests/unit/test_utils_deriveKey.js
>--- a/services/sync/tests/unit/test_utils_deriveKey.js
>+++ b/services/sync/tests/unit/test_utils_deriveKey.js
>@@ -1,19 +1,12 @@
>-let cryptoSvc;
>-try {
>-  Components.utils.import("resource://services-crypto/WeaveCrypto.js");
>-  cryptoSvc = new WeaveCrypto();
>-} catch (ex) {
>-  // Fallback to binary WeaveCrypto
>-  cryptoSvc = Cc["@labs.mozilla.com/Weave/Crypto;1"]
>-                .getService(Ci.IWeaveCrypto);
>-}
>+Cu.import("resource://services-crypto/WeaveCrypto.js");
>+Cu.import("resource://services-sync/util.js");
> 
>-Cu.import("resource://services-sync/util.js");
>+cryptoSvc = new WeaveCrypto();

Where did the 'let' go? r=me with that added back.

N.B.: While this removes an interface that we once shipped, it's not one that was ever actually used or documented, so I don't think we need superreview or anything like that. WeaveCrypto isn't public API, anyway.

Comment 3

[Richard Newman [:rnewman]]

Landed like a Honda Accord that's been riced up to look like a spaceship.

https://hg.mozilla.org/services/services-central/rev/d0cab65b9d4e

http://www.jdmuniverse.com/forums/gallery/data/505/honda-ricer.jpg

Comment 4

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/d0cab65b9d4e

Comment 5

[Jorge Villalobos [:jorgev] (he/him)]

This is being used by at least one extension. Flagging for developer documentation.

Comment 6

[Philipp von Weitershausen [:philikon]]

(In reply to comment #5)
> This is being used by at least one extension. Flagging for developer
> documentation.

It was never documented in the first place, so why does the removal need to be documented.

Comment 7

[Jorge Villalobos [:jorgev] (he/him)]

Because it is a fact that this interface has been used in add-ons, and there's an MDC article dedicated to breaking compatibility changes, and that has nothing to do with the interface being documented before or not.