+++ This bug was initially created as a clone of Bug #1644027 +++

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

1. Get a MessageHeader via browser.messageDisplay.onMessageDisplayed for news message inside a nntp account.
2. I tried to get the content by calling message.getRaw(id)

Actual results:

The promises returned by getRaw() never get out of the pending state.

Expected results:

The promises should get resolved with the message content. Or at least get rejected with an error.

I have some questions concerning this patch. I do not insist, I just want to be sure that the choice was conscious.

1 CallbackStreamListener.onDataAvailable() implementation in
https://hg.mozilla.org/comm-central/file/tip/mailnews/db/gloda/modules/MimeMessage.jsm#l124
creates scriptableinputstream just ones and then reuses it.
2. Unsure concerning JavaScript and SpiderMonkey, but generally it could be better to push string fragments to array and join them as the final step. I would expect less heap reallocations with such strategy.
3. aStatus argument of onStopRequest is ignored. Should not an error be reported and and the promise be rejected on non-zero value? Unsure if urlListener argument should be added to check aStatusCode as well or such check is redundant.
4. Is it possible in any messageService implementation that streamMessage returns null but does not throw an exception?

I do not know what is better, pure JS implementation of nsIStreamListener or sync-stream-listener used earlier.

Consider the following code a workaround for older version of thunderbird that could be added to any extension using experiment API.

experiment.js

"use strict";
var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
ChromeUtils.defineModuleGetter(
  this,
  "NetUtil",
  "resource://gre/modules/NetUtil.jsm"
);

var gNoopUrlListener = {
	OnStartRunningUrl(_aUrl) {},
	OnStopRunningUrl(_aUrl, _aExitCode) {},
	QueryInterface: ChromeUtils.generateQI([Ci.nsIUrlListener]),
};

class PromiseStreamListenerRequestObserver {
	constructor() {
		const self = this;
		const promise = new Promise((resolve, reject) => {
			self._resolve = resolve;
			self._reject = reject;
		});
		this.then = promise.then.bind(promise);
		this.finally = promise.finally.bind(promise);
		this.catch = promise.catch.bind(promise);
		this._streamListener = Cc[
			"@mozilla.org/network/sync-stream-listener;1"
		].createInstance(Ci.nsISyncStreamListener);
		this.onDataAvailable
			= this._streamListener.onDataAvailable.bind(this._streamListener);
	}
	onStartRequest(aRequest) {
		if (this._streamListener.onStartRequest) {
			this._streamListener.onStartRequest(aRequest);
		}
	}
	onStopRequest(aRequest, aStatusCode) {
		try {
			if (this._streamListener.onStopRequest) {
				this._streamListener.onStopRequest(aRequest, aStatusCode);
			}
			if (aStatusCode === 0) {
				this._resolve(NetUtil.readInputStreamToString(
					this._streamListener.inputStream,
					this._streamListener.available()));
			} else {
				Cu.reportError(exitCode);
				this._reject(new Error("StreamListener error " + aStatusCode));
			}
		} catch (ex) {
			this._reject(ex);
		}
	}
}

PromiseStreamListenerRequestObserver.prototype.QueryInterface
	= ChromeUtils.generateQI([Ci.nsIStreamListener]);

var tst = class extends ExtensionCommon.ExtensionAPI {
	getAPI(context) {
		return { tst: {
			async getRaw(messageId) {
				let messenger = Cc["@mozilla.org/messenger;1"].createInstance(
					Ci.nsIMessenger
				);
				const msgHdr = context.extension.messageManager.get(messageId);
				let msgUri = msgHdr.folder.generateMessageURI(msgHdr.messageKey);
				let service = messenger.messageServiceFromURI(msgUri);
				const msgWindow = null;
				const streamListener = new PromiseStreamListenerRequestObserver();
				const convertData = false;
				const additionalHeader = "";
				const localOnly = false;
				const uri = service.streamMessage(
					msgUri,
					streamListener,
					msgWindow,
					gNoopUrlListener,
					convertData,
					additionalHeader,
					localOnly
				);
				if (uri == null) {
					throw new Error("streamMessage returned null");
				}
				return streamListener;
			},
		}, };
	}
	onShutdown(isAppShutdown) {
		if (!isAppShutdown) {
			Services.obs.notifyObservers(null, "startupcache-invalidate", null);
		}
	}
};

schema.hson

[ {
	"namespace": "tst",
	"functions": [ {
		"name": "getRaw",
		"type": "function",
		"async": true,
		"parameters": [ { "name": "messageId", "type": "integer" } ]
	} ]
} ]

If you ask me, async is better. But let us see what the reviewer is saying.

In general, performance of nsSyncStreamListener native code could be better, I am unsure however if it is so in reality. Taking into account callback API, it not clear for me why async should be better in this particular case. My curiosity is not strong enough to carefully inspect nsPipeOutputStream code.

Anyway, since for other message types it is enough to define callbacks for nsUrlHandler, I filed for Bug #1695235 to at least review C++ implementation of nsNntpService if its behaviour could be more uniform with other types.

Concerning collecting fragments in an array, I mean

{
  _chunks: [],
  onDataAvailable(aRequest, aInputStream, aOffset, aCount) {
    // ...
    this._chunks.push(/* this._ */ scriptStream.read(aCount));
  },
  onStopRequest(aRequest, aStatus) {
    try {
      // ... check aStatus
      resolve(this._chunks.join(""));
    } catch (ex) {
      reject(ex);
    }
  },
}

I will add a reject on bad status, as the old code had one as well, thanks for pointing that out.
Regarding sync vs async, I really have no idea. I was told async is always better, but in this case it could be wrong.
Regarding the array chunks: I was going for what I have seen in the code (), but if using the array chunk approach is better I am fine with that.

I do not think it is a reason for another revision of the patch, just for the case if additional modification will be requested:

• streamMessage has one more optional argument aLocalOnly. Fortunately default value does not cause a problem unlike the similar argument of MsgHdrToMimeMessage discussed in Bug #1644027 and Bug #1623685.
• Do not know if it really matters but implementation in MimeMessage.jsm explicitly deletes instance of scriptableinputstream.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/c97f2e748a0f
switch getRaw from sync to async streamlistener and no longer use urllistener to work with news as well. r=darktrojan