Fix race handling contact sync with verified info

[liliakai]

When processing a contact sync with embedded identity key verification info, we
were running overlapping async fetch/save operations on the same conversation
model, causing a race that tends to clobber updates to the contact info.

In this change we extend the application-level contact info handler to block on
a subsequent call to the verification handler, which effectively serializes the
fetch/save calls, and relieves the need for the message receiver to trigger a
seperate event concerning the verification info on contact sync messages.

Fixes #1408

[scottnonnenberg]

Ha. We both tackled this one. :0) Here's my fix: https://github.com/WhisperSystems/Signal-Desktop/commits/contact-updates

[liliakai]

@scottnonnenberg ha. But if findOrCreate is called the second time before the first fetch resolves and populates attributes (ie timestamp) on the conversation, you could still end up fetching twice, no?

[scottnonnenberg]

It turns out that for any contact with active_at set, the initial fetchActive call means that no fetch() needs to happen again for the lifetime of the ConversationController. We re-fetch a lot today. But you're right that contacts we don't already know about could run into the double-fetch problem with my solution. And as has been pointed out that's likely what's happening with #1324 (comment)

My ultimate preference here would be to remove the fetches sprinkled about, and put two rules in place:

1. If a conversation is in the ConversationController it doesn't need to be fetched, but its initial fetch/save might be in progress.
2. If a conversation is not already in the ConversationController, it's not yet in the database. It needs to be added to the ConversationController and saved to the database.

[scottnonnenberg]

Here's a PR implementing those two rules, since I got pretty deep into investigating where we fetch yesterday. It's a global, long-term solution which will avoid other race conditions regarding conversation.fetch() as well: #1420

The solution implemented in this PR will certainly work for this contact/verified race condition, but I think MessageReceiver should be able to emit events whenever. To me, making the change in the application instead of in libtextsecure.js is better separation of concerns.

[liliakai]

#1420 should suffice to fix the race, though I would still suggest merging this because I rather like the removal of the viaContactSync concern from MessageReceiver. Also, it looks like by firing two events for the same envelope, either handler could resolve and remove that envelope from the queue. That seems a little weird, or at least, not as intended. This change would ensure that contact syncs are only removed from the queue if both the contact update and verified update are processed successfully.

[scottnonnenberg]

You've convinced me. Merging now...