Merge pull request #19574 from jrburke/bug1010701-email-multi-attach-lmk

Bug 1010701 - [Tarako][Email][LMK] Email gets killed when attach multiple files to a mail r=asuth

apps/email/js/cards/compose.js

@@ -17,6 +17,7 @@ var templateNode = require('tmpl!./compose.html'),
     cmpSendingContainerNode = require('tmpl!./cmp/sending_container.html'),
     msgAttachConfirmNode = require('tmpl!./msg/attach_confirm.html'),
     common = require('mail_common'),
+    Toaster = common.Toaster,
     model = require('model'),
     iframeShims = require('iframe_shims'),
     Marquee = require('marquee'),
@@ -71,6 +72,12 @@ function ComposeCard(domNode, mode, args) {
   this.sending = false;
   this.wifiLock = null;
 
+  // Management of attachment work, to limit memory use
+  this._totalAttachmentsFinishing = 0;
+  this._totalAttachmentsDone = 0;
+  this._wantAttachment = false;
+  this._onAttachmentDone = this._onAttachmentDone.bind(this);
+
   domNode.getElementsByClassName('cmp-back-btn')[0]
     .addEventListener('click', this.onBack.bind(this), false);
   this.sendButton = domNode.getElementsByClassName('cmp-send-btn')[0];
@@ -573,6 +580,45 @@ ComposeCard.prototype = {
     );
   },
 
+  /**
+   * Used to count when an attachment has been fully processed by this.composer.
+   * Broken out as a separate member method to avoid inline closures in
+   * addAttachmentsSubjectToSizeLimits that may lead to holding on to too much
+   * memory.
+   */
+  _onAttachmentDone: function() {
+    this._totalAttachmentsDone += 1;
+    if (this._totalAttachmentsDone < this._totalAttachmentsFinishing) {
+      return;
+    }
+
+    // Give a bit of time for all the DB transactions to clean up.
+    // Unfortunately there are no good signals to do this decisively so just
+    // adding a bit of a buffer, just to be nice for super low memory
+    // devices. Not a catastrophe if work is still going on when the timeout
+    // fires.
+    setTimeout(function() {
+      var wantAttachment = this._wantAttachment;
+      this._totalAttachmentsFinishing = 0;
+      this._totalAttachmentsDone = 0;
+      this._wantAttachment = false;
+
+      // Close out the toaster if it was showing. While the toaster could
+      // be showing for some other reason, this is the most likely cause,
+      // and want to give the user the impression of fast action.
+      if (Toaster.isShowing()) {
+        Toaster.hide();
+      }
+
+      // If the user wanted to add something else, proceed, since in many
+      // cases, the user just had to wait a second or so before we could
+      // proceed anyway.
+      if (wantAttachment) {
+        this.onAttachmentAdd();
+      }
+    }.bind(this), 600);
+  },
+
   /**
    * Given a list of Blobs/Files that we want to attach, attach as many as
    * possible and generate an error message for any we can't attach.  This will
@@ -582,9 +628,11 @@ ComposeCard.prototype = {
     var totalSize = 0;
     // Tally the size of the already-attached attachments.
     if (this.composer.attachments) {
-      this.composer.attachments.forEach(function(attachment) {
-        totalSize += attachment.blob.size;
-      });
+      // Using a for loop to avoid any closures that may capture
+      // the large attachments.
+      for (var i = 0; i < this.composer.attachments.length; i++) {
+        totalSize += this.composer.attachments[i].blob.size;
+      }
     }
 
     // Keep attaching until we find one that puts us over the limit.  Then
@@ -605,7 +653,8 @@ ComposeCard.prototype = {
         break;
       }
 
-      this.composer.addAttachment(attachment);
+      this._totalAttachmentsFinishing += 1;
+      this.composer.addAttachment(attachment, this._onAttachmentDone);
       attachedAny = true;
     }
 
@@ -877,7 +926,20 @@ ComposeCard.prototype = {
   },
 
   onAttachmentAdd: function(event) {
-    event.stopPropagation();
+    if (event) {
+      event.stopPropagation();
+    }
+
+    // To be nice on memory consumption, wait for any previous attachment to
+    // finish attaching before triggering another attachment action.
+    if (this._totalAttachmentsFinishing > 0) {
+      // Use a separate flag than testing if the toaster is showing, in case the
+      // toaster is shown for some other reason. In that case, do not want to
+      // trigger activity after previous attachment completes.
+      this._wantAttachment = true;
+      Toaster.show('text', mozL10n.get('compose-attachment-still-working'));
+      return;
+    }
 
     try {
       console.log('compose: attach: triggering web activity');

apps/email/js/mail_common.js

@@ -1157,6 +1157,10 @@ Toaster = {
                                          this._timeout);
   },
 
+  isShowing: function() {
+    return !this.body.classList.contains('collapsed');
+  },
+
   hide: function() {
     this.body.classList.add('collapsed');
     this.body.classList.remove('fadeout');

apps/email/locales/email.en-US.properties

@@ -273,6 +273,10 @@ compose-attachments[few]   = {{ n }} attachments
 compose-attachments[many]  = {{ n }} attachments
 compose-attachments[other] = {{ n }} attachments
 
+# Shown when user taps on attachment icon in compose, but the app still
+# needs a bit more time to finishing the add of the previous attachment.
+compose-attachment-still-working=One moment, still working on the previous attachment...
+
 compose-subject=Subject
 compose-draft-save=Save to Local Drafts
 compose-discard-message=Discard email?