Bug 1715340 - use event for conveying openpgp processing state. r=kaie
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Fri, 11 Jun 2021 13:30:02 +0300
changeset 32789 0f6d8f014e16c06d536736ec549213ff627c7079
parent 32788 9473904d2047fc7d42476625eb8f4cf8d5a6f11f
child 32790 6b3c1e3f279210a20b2078f5b11a45db5dfb80d7
push id18866
push usermkmelin@iki.fi
push dateFri, 11 Jun 2021 10:41:09 +0000
treeherdercomm-central@a009a421cf65 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskaie
bugs1715340, 1681887
Bug 1715340 - use event for conveying openpgp processing state. r=kaie Use event based checking for when we're done (followup on bug 1681887). Get rid of some callbacks. Differential Revision: https://phabricator.services.mozilla.com/D117213
mail/extensions/openpgp/content/ui/enigmailMessengerOverlay.js
mail/test/browser/openpgp/browser_viewMessage.js
--- a/mail/extensions/openpgp/content/ui/enigmailMessengerOverlay.js
+++ b/mail/extensions/openpgp/content/ui/enigmailMessengerOverlay.js
@@ -520,26 +520,21 @@ Enigmail.msg = {
     EnigmailLog.DEBUG("enigmailMessengerOverlay.js: messageAutoDecrypt:\n");
     await Enigmail.msg.messageDecrypt(null, true);
   },
 
   async notifyMessageDecryptDone() {
     Enigmail.msg.messageDecryptDone = true;
     Enigmail.msg.processAfterAttachmentsAndDecrypt();
 
-    // To allow the automated tests to access the messageDecryptDone
-    // state, we set an attribute with value "true".
-    // TODO: Replace with an event based system, but that will require
-    //       to rewrite the message parsing, so that we can reliably
-    //       await the state of all pending processing, in particular
-    //       see the hack in msgDirectCallback which uses setTimeout(0).
-    let cryptoBox = document.getElementById("cryptoBox");
-    if (cryptoBox) {
-      cryptoBox.setAttribute("decryptDone", "true");
-    }
+    document.dispatchEvent(
+      new CustomEvent("openpgpprocessed", {
+        detail: { messageDecryptDone: true },
+      })
+    );
 
     // Show the partial inline encryption reminder only if the decryption action
     // came from a partially inline encrypted message.
     if (Enigmail.msg.showPartialDecryptionReminder) {
       Enigmail.msg.showPartialDecryptionReminder = false;
 
       this.notificationBox.appendNotification(
         await document.l10n.formatValue("openpgp-reminder-partial-display"),
@@ -1490,17 +1485,17 @@ Enigmail.msg = {
           isAuto,
           pbMessageIndex
         );
         return;
       } else if (retry == 2) {
         // Try to verify signature by accessing raw message text directly
         // (avoid recursion by setting retry parameter to false on callback)
         newSignature = "";
-        Enigmail.msg.msgDirectDecrypt(
+        await Enigmail.msg.msgDirectDecrypt(
           interactive,
           importOnly,
           contentEncoding,
           charset,
           newSignature,
           0,
           head,
           tail,
@@ -2184,17 +2179,17 @@ Enigmail.msg = {
       if (AppConstants.platform != "win") {
         contentData = contentData.replace(/\r\n/g, "\n");
       }
     }
 
     return contentData;
   },
 
-  msgDirectDecrypt(
+  async msgDirectDecrypt(
     interactive,
     importOnly,
     contentEncoding,
     charset,
     signature,
     bufferSize,
     head,
     tail,
@@ -2204,139 +2199,130 @@ Enigmail.msg = {
   ) {
     EnigmailLog.WRITE(
       "enigmailMessengerOverlay.js: msgDirectDecrypt: contentEncoding=" +
         contentEncoding +
         ", signature=" +
         signature +
         "\n"
     );
-    var mailNewsUrl = this.getCurrentMsgUrl();
+    let mailNewsUrl = this.getCurrentMsgUrl();
     if (!mailNewsUrl) {
       return;
     }
 
-    var callbackArg = {
-      interactive,
-      importOnly,
-      contentEncoding,
-      charset,
-      messageUrl: mailNewsUrl.spec,
-      msgUriSpec,
-      signature,
-      data: "",
-      head,
-      tail,
-      isAuto,
-      callbackFunction,
+    let PromiseStreamListener = function() {
+      this._promise = new Promise((resolve, reject) => {
+        this._resolve = resolve;
+        this._reject = reject;
+      });
+      this._data = null;
+      this._stream = null;
     };
 
-    var msgSvc = messenger.messageServiceFromURI(msgUriSpec);
-
-    var listener = {
+    PromiseStreamListener.prototype = {
       QueryInterface: ChromeUtils.generateQI(["nsIStreamListener"]),
-      onStartRequest() {
+
+      onStartRequest(request) {
         this.data = "";
         this.inStream = Cc[
           "@mozilla.org/scriptableinputstream;1"
         ].createInstance(Ci.nsIScriptableInputStream);
       },
-      onStopRequest() {
-        var start = this.data.indexOf("-----BEGIN PGP");
-        var end = this.data.indexOf("-----END PGP");
+
+      onStopRequest(request, statusCode) {
+        if (statusCode != Cr.NS_OK) {
+          this._reject(`Streaming failed: ${statusCode}`);
+          return;
+        }
+
+        let start = this.data.indexOf("-----BEGIN PGP");
+        let end = this.data.indexOf("-----END PGP");
 
         if (start >= 0 && end > start) {
-          var tStr = this.data.substr(end);
-          var n = tStr.indexOf("\n");
-          var r = tStr.indexOf("\r");
-          var lEnd = -1;
+          let tStr = this.data.substr(end);
+          let n = tStr.indexOf("\n");
+          let r = tStr.indexOf("\r");
+          let lEnd = -1;
           if (n >= 0 && r >= 0) {
             lEnd = Math.min(r, n);
           } else if (r >= 0) {
             lEnd = r;
           } else if (n >= 0) {
             lEnd = n;
           }
 
           if (lEnd >= 0) {
             end += lEnd;
           }
 
-          callbackArg.data = Enigmail.msg.trimIfEncrypted(
+          let data = Enigmail.msg.trimIfEncrypted(
             this.data.substring(start, end + 1)
           );
           EnigmailLog.DEBUG(
-            "enigmailMessengerOverlay.js: data: >" +
-              callbackArg.data.substr(0, 100) +
-              "<\n"
+            "enigmailMessengerOverlay.js: data: >" + data.substr(0, 100) + "<\n"
           );
-          Enigmail.msg.msgDirectCallback(callbackArg);
+
+          let currentMsgURL = Enigmail.msg.getCurrentMsgUrl();
+          let urlSpec = currentMsgURL ? currentMsgURL.spec : "";
+
+          let l = urlSpec.length;
+          if (urlSpec.substr(0, l) != mailNewsUrl.spec.substr(0, l)) {
+            EnigmailLog.ERROR(
+              "enigmailMessengerOverlay.js: Message URL mismatch " +
+                currentMsgURL +
+                " vs. " +
+                urlSpec +
+                "\n"
+            );
+            this._reject(`Msg url mismatch: ${currentMsgURL} vs ${urlSpec}`);
+            return;
+          }
+
+          Enigmail.msg
+            .messageParseCallback(
+              data,
+              contentEncoding,
+              charset,
+              interactive,
+              importOnly,
+              mailNewsUrl.spec,
+              signature,
+              3,
+              head,
+              tail,
+              msgUriSpec,
+              isAuto
+            )
+            .then(() => this._resolve(this.data));
         }
       },
+
+      onDataAvailable(request, stream, off, count) {
+        this.inStream.init(stream);
+        this.data += this.inStream.read(count);
+      },
+
+      get promise() {
+        return this._promise;
+      },
     };
 
-    listener.onDataAvailable = function(req, stream, offset, count) {
-      this.inStream.init(stream);
-      this.data += this.inStream.read(count);
-    };
-
+    let streamListener = new PromiseStreamListener();
+    let msgSvc = messenger.messageServiceFromURI(msgUriSpec);
     msgSvc.streamMessage(
       msgUriSpec,
-      listener,
+      streamListener,
       msgWindow,
       null,
       false,
       null,
       false
     );
-  },
-
-  msgDirectCallback(callbackArg) {
-    EnigmailLog.DEBUG("enigmailMessengerOverlay.js: msgDirectCallback: \n");
-
-    var mailNewsUrl = Enigmail.msg.getCurrentMsgUrl();
-    var urlSpec = mailNewsUrl ? mailNewsUrl.spec : "";
-
-    var l = urlSpec.length;
-
-    if (urlSpec.substr(0, l) != callbackArg.messageUrl.substr(0, l)) {
-      EnigmailLog.ERROR(
-        "enigmailMessengerOverlay.js: msgDirectCallback: Message URL mismatch " +
-          mailNewsUrl.spec +
-          " vs. " +
-          callbackArg.messageUrl +
-          "\n"
-      );
-      return;
-    }
-
-    var msgText = callbackArg.data;
-
-    EnigmailLog.DEBUG(
-      "enigmailMessengerOverlay.js: msgDirectCallback: msgText='" +
-        msgText.substr(0, 100) +
-        "'\n"
-    );
-
-    setTimeout(() => {
-      callbackArg.callbackFunction(
-        msgText,
-        callbackArg.contentEncoding,
-        callbackArg.charset,
-        callbackArg.interactive,
-        callbackArg.importOnly,
-        callbackArg.messageUrl,
-        callbackArg.signature,
-        3,
-        callbackArg.head,
-        callbackArg.tail,
-        callbackArg.msgUriSpec,
-        callbackArg.isAuto
-      );
-    }, 0);
+    await streamListener;
   },
 
   revealAttachments(index) {
     if (!index) {
       index = 0;
     }
 
     if (index < currentAttachments.length) {
--- a/mail/test/browser/openpgp/browser_viewMessage.js
+++ b/mail/test/browser/openpgp/browser_viewMessage.js
@@ -42,31 +42,33 @@ function getMsgBodyTxt(mc) {
   let msgPane = mc.window.document.getElementById("messagepane");
   return msgPane.contentDocument.firstChild.textContent;
 }
 
 var aliceAcct;
 var aliceIdentity;
 var initialKeyIdPref = "";
 
-async function waitForOpenPGPDone(doc) {
-  // We must wait until after OpenPGP processing is done.
-  // When testing an PGP/INLINE message, the message will initially
-  // be loaded into the message pane, and in a second step, the
-  // message will be decrypted or verified, the shown text will be
-  // updated, and the OpenPGP status icons will be set.
-  // Initially on loading a message, attribute decryptDone is missing.
-  // Once the OpenPGP code is done processing the message, the
-  // attribute will be set to true.
+/**
+ * When testing a scenario that should automatically process the OpenPGP
+ * contents (it's not suppressed e.g. because of a partial content),
+ * then we need to wait for the automatic processing to complete.
+ */
+async function openpgpProcessed() {
+  let [subject] = await TestUtils.topicObserved(
+    "document-element-inserted",
+    document => {
+      return (
+        document.ownerGlobal?.location ==
+        "chrome://messenger/content/messageWindow.xhtml"
+      );
+    }
+  );
 
-  await BrowserTestUtils.waitForCondition(
-    () =>
-      doc.getElementById("cryptoBox")?.getAttribute("decryptDone") == "true",
-    "Timeout waiting for decrypt processing completion"
-  );
+  return BrowserTestUtils.waitForEvent(subject, "openpgpprocessed");
 }
 
 /**
  * Set up the base account, identity and keys needed for the tests.
  */
 add_task(async function setupTest() {
   aliceAcct = MailServices.accounts.createAccount();
   aliceAcct.incomingServer = MailServices.accounts.createIncomingServer(
@@ -492,20 +494,21 @@ add_task(async function testUpdateMessag
 // After test testUpdateMessageSignature acceptance of Bob's key
 // has changed from verified to unverified.
 
 /**
  * Test that a signed (only) inline PGP message with UTF-8 characters
  * can be correctly verified.
  */
 add_task(async function testOpenSignedInlineWithUTF8() {
+  let opengpgprocessed = openpgpProcessed();
   let mc = await open_message_from_file(
     new FileUtils.File(getTestFilePath("data/eml/alice-utf.eml"))
   );
-  await waitForOpenPGPDone(mc.window.document);
+  await opengpgprocessed;
 
   Assert.ok(
     getMsgBodyTxt(mc).includes("£35.00"),
     "UTF-8 character found in message"
   );
   Assert.ok(
     OpenPGPTestUtils.hasSignedIconState(mc.window.document, "unverified"),
     "signed unverified icon is displayed"
@@ -517,20 +520,21 @@ add_task(async function testOpenSignedIn
   close_window(mc);
 });
 
 /**
  * Test that a signed (only) inline PGP message with leading whitespace
  * can be correctly verified.
  */
 add_task(async function testOpenSignedInlineWithLeadingWS() {
+  let opengpgprocessed = openpgpProcessed();
   let mc = await open_message_from_file(
     new FileUtils.File(getTestFilePath("data/eml/signed-inline-indented.eml"))
   );
-  await waitForOpenPGPDone(mc.window.document);
+  await opengpgprocessed;
 
   Assert.ok(
     getMsgBodyTxt(mc).includes("indent test with £"),
     "expected text should be found in message"
   );
   Assert.ok(
     OpenPGPTestUtils.hasSignedIconState(mc.window.document, "unverified"),
     "signed unverified icon is displayed"
@@ -542,20 +546,21 @@ add_task(async function testOpenSignedIn
   close_window(mc);
 });
 
 /**
  * Test that an encrypted inline message, with nbsp encoded as qp
  * in the PGP separator line, is trimmed and decrypted.
  */
 add_task(async function testDecryptInlineWithNBSPasQP() {
+  let opengpgprocessed = openpgpProcessed();
   let mc = await open_message_from_file(
     new FileUtils.File(getTestFilePath("data/eml/bob-enc-inline-nbsp-qp.eml"))
   );
-  await waitForOpenPGPDone(mc.window.document);
+  await opengpgprocessed;
 
   Assert.ok(
     getMsgBodyTxt(mc).includes("My real name is not Bob."),
     "Secret text should be contained in message"
   );
   Assert.ok(
     OpenPGPTestUtils.hasEncryptedIconState(mc.window.document, "ok"),
     "Encrypted icon should be displayed"
@@ -563,20 +568,21 @@ add_task(async function testDecryptInlin
   close_window(mc);
 });
 
 /**
  * Test that an inline message, encoded as html message, with nbsp
  * encoded as qp in the PGP separator line, is trimmed and decrypted.
  */
 add_task(async function testDecryptHtmlWithNBSP() {
+  let opengpgprocessed = openpgpProcessed();
   let mc = await open_message_from_file(
     new FileUtils.File(getTestFilePath("data/eml/bob-enc-html-nbsp.eml"))
   );
-  await waitForOpenPGPDone(mc.window.document);
+  await opengpgprocessed;
 
   Assert.ok(
     getMsgBodyTxt(mc).includes("My real name is not Bob."),
     "Secret text should be contained in message"
   );
   Assert.ok(
     OpenPGPTestUtils.hasEncryptedIconState(mc.window.document, "ok"),
     "Encrypted icon should be displayed"