The default bug view has changed. See this FAQ.

Eliminate IWeaveCrypto

RESOLVED FIXED in mozilla6

Status

Cloud Services
Firefox Sync: Crypto
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

({dev-doc-needed})

unspecified
mozilla6
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [prune][qa-])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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);
Whiteboard: [prune]
(Assignee)

Comment 1

6 years ago
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.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #527375 - Flags: review?(philipp)
Attachment #527375 - Attachment is patch: true
Attachment #527375 - Attachment mime type: application/octet-stream → text/plain
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.
Attachment #527375 - Flags: review?(philipp) → review+
(Assignee)

Comment 3

6 years ago
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
Whiteboard: [prune] → [prune][fixed in services][qa-]
http://hg.mozilla.org/mozilla-central/rev/d0cab65b9d4e
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [prune][fixed in services][qa-] → [prune][qa-]
Target Milestone: --- → mozilla6
Blocks: 657252
This is being used by at least one extension. Flagging for developer documentation.
Keywords: dev-doc-needed
(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.
Keywords: dev-doc-needed
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.
Keywords: dev-doc-complete
Keywords: dev-doc-complete → dev-doc-needed
You need to log in before you can comment on or make changes to this bug.