Load attachment data before rendering

Prevent double rendering of attachments by multiple entries into
`MessageView::render` using promises.

js/views/message_view.js

@@ -6,6 +6,9 @@
     'use strict';
     window.Whisper = window.Whisper || {};
 
+    const { Attachment } = window.Signal.Types;
+    const { context: migrationContext } = window.Signal.Migrations;
+
     var URL_REGEX = /(^|[\s\n]|<br\/?>)((?:https?|ftp):\/\/[\-A-Z0-9\u00A0-\uD7FF\uE000-\uFDCF\uFDF0-\uFFFD+\u0026\u2019@#\/%?=()~_|!:,.;]*[\-A-Z0-9+\u0026@#\/%=~()_|])/gi;
 
     var ErrorIconView = Whisper.View.extend({
@@ -179,6 +182,9 @@
             return this.model.id;
         },
         initialize: function() {
+            //   loadedAttachmentViews :: Promise (Array AttachmentView) | null
+            this.loadedAttachmentViews = null;
+
             this.listenTo(this.model, 'change:errors', this.onErrorsChanged);
             this.listenTo(this.model, 'change:body', this.render);
             this.listenTo(this.model, 'change:delivered', this.renderDelivered);
@@ -224,7 +230,8 @@
             // Failsafe: if in the background, animation events don't fire
             setTimeout(this.remove.bind(this), 1000);
         },
-        onUnload: function() {
+        /* jshint ignore:start */
+        onUnload: async function() {
             if (this.avatarView) {
                 this.avatarView.remove();
             }
@@ -240,18 +247,16 @@
             if (this.timeStampView) {
                 this.timeStampView.remove();
             }
-            if (this.loadedAttachments && this.loadedAttachments.length) {
+
-                for (var i = 0, max = this.loadedAttachments.length; i < max; i += 1) {
+            const views = await this.loadedAttachmentViews;
-                    var view = this.loadedAttachments[i];
+            views.forEach(view => view.unload());
-                    view.unload();
-                }
-            }
 
             // No need to handle this one, since it listens to 'unload' itself:
             //   this.timerView
 
             this.remove();
         },
+        /* jshint ignore:end */
         onDestroy: function() {
             if (this.$el.hasClass('expired')) {
               return;
@@ -376,7 +381,12 @@
             this.renderErrors();
             this.renderExpiring();
 
-            this.loadAttachments();
+
+            // NOTE: We have to do this in the background (`then` instead of `await`)
+            // as our code / Backbone seems to rely on `render` synchronously returning
+            // `this` instead of `Promise MessageView` (this):
+            // eslint-disable-next-line more/no-then
+            this.loadAttachmentViews().then(views => this.renderAttachmentViews(views));
 
             return this;
         },
@@ -395,51 +405,62 @@
             }))();
             this.$('.avatar').replaceWith(this.avatarView.render().$('.avatar'));
         },
-        appendAttachmentView: function(view) {
+    /* eslint-enable */
-            // We check for a truthy 'updated' here to ensure that a race condition in a
+    /* jshint ignore:start */
-            //   multi-fetch() scenario doesn't add an AttachmentView to the DOM before
+    loadAttachmentViews() {
-            //   its 'update' event is triggered.
+      if (this.loadedAttachmentViews !== null) {
-            var parent = this.$('.attachments')[0];
+        return this.loadedAttachmentViews;
-            if (view.updated && parent !== view.el.parentNode) {
+      }
-                if (view.el.parentNode) {
-                    view.el.parentNode.removeChild(view.el);
-                }
 
-                this.trigger('beforeChangeHeight');
+      const loadData = Attachment.loadData(migrationContext.readAttachmentData);
-                this.$('.attachments').append(view.el);
+      const attachments = this.model.get('attachments');
-                view.setElement(view.el);
+      const loadedAttachmentViews = Promise.all(attachments.map(attachment =>
-                this.trigger('afterChangeHeight');
+        new Promise(async (resolve) => {
-            }
+          const attachmentWithData = await loadData(attachment);
-        },
+          const view = new Whisper.AttachmentView({
-        loadAttachments: function() {
+            model: attachmentWithData,
-            this.loadedAttachments = this.loadedAttachments || [];
+            timestamp: this.model.get('sent_at'),
+          });
 
-            // If we're called a second time, render() has replaced the DOM out from under
+          this.listenTo(view, 'update', () => {
-            //   us with $el.html(). We'll need to reattach our AttachmentViews to the new
+            // NOTE: Can we do without `updated` flag now that we use promises?
-            //   parent DOM nodes if the 'update' event has already fired.
+            view.updated = true;
-            if (this.loadedAttachments.length) {
+            resolve(view);
-                for (var i = 0, max = this.loadedAttachments.length; i < max; i += 1) {
+          });
-                    var view = this.loadedAttachments[i];
-                    this.appendAttachmentView(view);
-                }
-                return;
-            }
 
-            this.model.get('attachments').forEach(function(attachment) {
+          view.render();
-                var view = new Whisper.AttachmentView({
+        })));
-                  model: attachment,
-                  timestamp: this.model.get('sent_at')
-                });
-                this.loadedAttachments.push(view);
 
-                this.listenTo(view, 'update', function() {
+      // Memoize attachment views to avoid double loading:
-                    view.updated = true;
+      this.loadedAttachmentViews = loadedAttachmentViews;
-                    this.appendAttachmentView(view);
-                });
 
-                view.render();
+      return loadedAttachmentViews;
-            }.bind(this));
+    },
-        }
+    renderAttachmentViews(views) {
+      views.forEach(view => this.renderAttachmentView(view));
+    },
+    renderAttachmentView(view) {
+      if (!view.updated) {
+        throw new Error('Invariant violation:' +
+          ' Cannot render an attachment view that isn’t ready');
+      }
+
+      const parent = this.$('.attachments')[0];
+      const isViewAlreadyChild = parent === view.el.parentNode;
+      if (isViewAlreadyChild) {
+        return;
+      }
+
+      if (view.el.parentNode) {
+        view.el.parentNode.removeChild(view.el);
+      }
+
+      this.trigger('beforeChangeHeight');
+      this.$('.attachments').append(view.el);
+      view.setElement(view.el);
+      this.trigger('afterChangeHeight');
+    },
+    /* jshint ignore:end */
+    /* eslint-disable */
     });
-
 })();