From aed573562018462fbacd8f2f715e9daeddcde0dd Mon Sep 17 00:00:00 2001 From: lilia Date: Sun, 7 May 2017 14:18:22 -0700 Subject: [PATCH] Improve keychange notice reliability/perf Bind a single listener to keychange events from the storage interface, which then looks up relevant conversations and adds notices to them, with tests. Previously we would need to instantiate a conversation model in order to start listening to its key change events. In practice this usually happens at startup but we shouldn't rely on it, and it incurs higher overhead since it creates a different listener for each conversation. // FREEBIE --- background.html | 1 + js/background.js | 1 + js/delivery_receipts.js | 18 +------- js/keychange_listener.js | 30 +++++++++++++ js/models/conversations.js | 24 +++++++---- js/signal_protocol_store.js | 2 +- test/index.html | 2 + test/keychange_listener_test.js | 74 +++++++++++++++++++++++++++++++++ 8 files changed, 127 insertions(+), 25 deletions(-) create mode 100644 js/keychange_listener.js create mode 100644 test/keychange_listener_test.js diff --git a/background.html b/background.html index fcff3b8220..dfdb8c6a74 100644 --- a/background.html +++ b/background.html @@ -659,6 +659,7 @@ + diff --git a/js/background.js b/js/background.js index 05052be415..9c3b594571 100644 --- a/js/background.js +++ b/js/background.js @@ -20,6 +20,7 @@ // start a background worker for ecc textsecure.startWorker('/js/libsignal-protocol-worker.js'); + Whisper.KeyChangeListener.init(textsecure.storage.protocol); extension.onLaunched(function() { console.log('extension launched'); diff --git a/js/delivery_receipts.js b/js/delivery_receipts.js index 327d2647fa..b74d8be742 100644 --- a/js/delivery_receipts.js +++ b/js/delivery_receipts.js @@ -5,22 +5,6 @@ 'use strict'; window.Whisper = window.Whisper || {}; - var GroupCollection = Backbone.Collection.extend({ - database: Whisper.Database, - storeName: 'conversations', - model: Backbone.Model, - fetchGroups: function(number) { - return new Promise(function(resolve) { - this.fetch({ - index: { - name: 'group', - only: number - } - }).always(resolve); - }.bind(this)); - } - }); - Whisper.DeliveryReceipts = new (Backbone.Collection.extend({ initialize: function() { this.on('add', this.onReceipt); @@ -48,7 +32,7 @@ }); if (message) { return message; } - var groups = new GroupCollection(); + var groups = new Whisper.GroupCollection(); return groups.fetchGroups(receipt.get('source')).then(function() { var ids = groups.pluck('id'); ids.push(receipt.get('source')); diff --git a/js/keychange_listener.js b/js/keychange_listener.js new file mode 100644 index 0000000000..b58272f726 --- /dev/null +++ b/js/keychange_listener.js @@ -0,0 +1,30 @@ +/* + * vim: ts=4:sw=4:expandtab + */ + +;(function () { + 'use strict'; + window.Whisper = window.Whisper || {}; + + Whisper.KeyChangeListener = { + init: function(signalProtocolStore) { + if (!(signalProtocolStore instanceof SignalProtocolStore)) { + throw new Error('KeyChangeListener requires a SignalProtocolStore'); + } + + signalProtocolStore.on('keychange', function(id) { + var conversation = ConversationController.add({id: id}); + conversation.fetch().then(function() { + conversation.addKeyChange(id); + }); + var groups = new Whisper.GroupCollection(); + return groups.fetchGroups(id).then(function() { + groups.each(function(conversation) { + conversation = ConversationController.add(conversation); + conversation.addKeyChange(id); + }); + }); + }); + } + }; +}()); diff --git a/js/models/conversations.js b/js/models/conversations.js index 107471f879..7a32e78352 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -46,13 +46,6 @@ this.on('change:avatar', this.updateAvatarUrl); this.on('destroy', this.revokeAvatarUrl); - this.fetchContacts().then(function() { - this.contactCollection.each(function(contact) { - textsecure.storage.protocol.on('keychange:' + contact.id, function() { - this.addKeyChange(contact.id); - }.bind(this)); - }.bind(this)); - }.bind(this)); }, addKeyChange: function(id) { @@ -631,4 +624,21 @@ }); Whisper.Conversation.COLORS = COLORS.concat(['grey', 'default']).join(' '); + + // Special collection for fetching all the groups a certain number appears in + Whisper.GroupCollection = Backbone.Collection.extend({ + database: Whisper.Database, + storeName: 'conversations', + model: Whisper.Conversation, + fetchGroups: function(number) { + return new Promise(function(resolve) { + this.fetch({ + index: { + name: 'group', + only: number + } + }).always(resolve); + }.bind(this)); + } + }); })(); diff --git a/js/signal_protocol_store.js b/js/signal_protocol_store.js index 48efffd1a7..5eb376d42a 100644 --- a/js/signal_protocol_store.js +++ b/js/signal_protocol_store.js @@ -297,7 +297,7 @@ this.removeIdentityKey(identifier).then(function() { this.saveIdentity(identifier, publicKey).then(function() { console.log('Key changed for', identifier); - this.trigger('keychange:' + identifier); + this.trigger('keychange', identifier); resolve(true); }.bind(this)); }.bind(this)); diff --git a/test/index.html b/test/index.html index f0a660c19e..4f72299d24 100644 --- a/test/index.html +++ b/test/index.html @@ -529,6 +529,7 @@ + @@ -574,6 +575,7 @@ + diff --git a/test/keychange_listener_test.js b/test/keychange_listener_test.js new file mode 100644 index 0000000000..de1f114e15 --- /dev/null +++ b/test/keychange_listener_test.js @@ -0,0 +1,74 @@ +describe('KeyChangeListener', function() { + var phone_number_with_keychange = '+13016886524'; // nsa + var oldkey = libsignal.crypto.getRandomBytes(33); + var newkey = libsignal.crypto.getRandomBytes(33); + var store; + + before(function() { + storage.put('safety-numbers-approval', false); + }); + + after(function() { + storage.remove('safety-numbers-approval'); + }); + + beforeEach(function() { + store = new SignalProtocolStore(); + Whisper.KeyChangeListener.init(store); + return store.saveIdentity(phone_number_with_keychange, oldkey); + }); + + afterEach(function() { + return store.removeIdentityKey(phone_number_with_keychange); + }); + + describe('When we have a conversation with this contact', function() { + var convo = new Whisper.Conversation({ id: phone_number_with_keychange, type: 'private'}); + before(function() { + ConversationController.add(convo); + return new Promise(function(resolve) { convo.save().then(resolve); }); + }); + + after(function() { + convo.destroyMessages(); + return convo.destroy(); + }); + + it('generates a key change notice in the private conversation with this contact', function(done) { + convo.on('newmessage', function() { + return convo.fetchMessages().then(function() { + var message = convo.messageCollection.at(0); + assert.strictEqual(message.get('type'), 'keychange'); + done(); + }); + }); + return store.isTrustedIdentity(phone_number_with_keychange, newkey); + }); + }); + + + describe('When we have a group with this contact', function() { + var convo = new Whisper.Conversation({ id: 'groupId', type: 'group', members: [phone_number_with_keychange] }); + before(function() { + ConversationController.add(convo); + return convo.save(); + }); + after(function() { + convo.destroyMessages(); + return convo.destroy(); + }); + + it('generates a key change notice in the group conversation with this contact', function(done) { + convo.on('newmessage', function() { + return convo.fetchMessages().then(function() { + var message = convo.messageCollection.at(0); + assert.strictEqual(message.get('type'), 'keychange'); + done(); + }); + }); + return store.isTrustedIdentity(phone_number_with_keychange, newkey); + }); + + }); + +});