Last Comment Bug 651596 - Eliminate IWeaveCrypto
: Eliminate IWeaveCrypto
Status: RESOLVED FIXED
[prune][qa-]
: dev-doc-needed
Product: Cloud Services
Classification: Client Software
Component: Firefox Sync: Crypto (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla6
Assigned To: Richard Newman [:rnewman]
:
:
Mentors:
Depends on:
Blocks: 657252
  Show dependency treegraph
 
Reported: 2011-04-20 12:49 PDT by Richard Newman [:rnewman]
Modified: 2011-05-27 17:09 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch. v1 (14.97 KB, patch)
2011-04-20 14:16 PDT, Richard Newman [:rnewman]
philipp: review+
Details | Diff | Splinter Review

Description Richard Newman [:rnewman] 2011-04-20 12:49:59 PDT
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] 2011-04-20 14:16:40 PDT
Created attachment 527375 [details] [diff] [review]
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] 2011-04-20 14:25:28 PDT
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] 2011-04-20 16:41:35 PDT
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] 2011-04-26 21:47:55 PDT
http://hg.mozilla.org/mozilla-central/rev/d0cab65b9d4e
Comment 5 Jorge Villalobos [:jorgev] 2011-05-27 16:12:27 PDT
This is being used by at least one extension. Flagging for developer documentation.
Comment 6 Philipp von Weitershausen [:philikon] 2011-05-27 16:14:58 PDT
(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] 2011-05-27 17:08:57 PDT
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.

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