From 716c90bca4c8c83aa9b9615e1ecd2b8f1e69d512 Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Mon, 7 Dec 2020 12:40:56 -0800 Subject: [PATCH] fix: wrap `onRequestFinished` to use promises Fixes #249. This updates `browser.devtools.network.onRequestFinished` to emit an object with a promisified `getContent()` property. This brings the polyfill implementation in line with Firefox's implementation, although MDN documentation is still inaccurate at the moment. Also updates some out of date documentation with `makeCallback()` and `wrapAsyncFunction()`. --- src/browser-polyfill.js | 51 ++++++++++++-- test/test-onRequestFinished.js | 119 +++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 test/test-onRequestFinished.js diff --git a/src/browser-polyfill.js b/src/browser-polyfill.js index a081edd..a1d43ea 100644 --- a/src/browser-polyfill.js +++ b/src/browser-polyfill.js @@ -77,13 +77,17 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object. * promise. * @param {function} promise.resolve * The promise's resolution function. - * @param {function} promise.rejection + * @param {function} promise.reject * The promise's rejection function. * @param {object} metadata * Metadata about the wrapped method which has created the callback. - * @param {integer} metadata.maxResolvedArgs - * The maximum number of arguments which may be passed to the - * callback created by the wrapped async function. + * @param {boolean} metadata.singleCallbackArg + * Whether or not the promise is resolved with only the first + * argument of the callback, alternatively an array of all the + * callback arguments is resolved. By default, if the callback + * function is invoked with only a single argument, that will be + * resolved to the promise, while all arguments will be resolved as + * an array if multiple are given. * * @returns {function} * The generated callback function. @@ -118,9 +122,13 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object. * The maximum number of arguments which may be passed to the * function. If called with more than this number of arguments, the * wrapper will raise an exception. - * @param {integer} metadata.maxResolvedArgs - * The maximum number of arguments which may be passed to the - * callback created by the wrapped async function. + * @param {boolean} metadata.singleCallbackArg + * Whether or not the promise is resolved with only the first + * argument of the callback, alternatively an array of all the + * callback arguments is resolved. By default, if the callback + * function is invoked with only a single argument, that will be + * resolved to the promise, while all arguments will be resolved as + * an array if multiple are given. * * @returns {function(object, ...*)} * The generated wrapper function. @@ -345,6 +353,30 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object. }, }); + const onRequestFinishedWrappers = new DefaultWeakMap(listener => { + if (typeof listener !== "function") { + return listener; + } + + /** + * Wraps an onRequestFinished listener function so that it will return a + * `getContent()` property which returns a `Promise` rather than using a + * callback API. + * + * @param {object} req + * The HAR entry object representing the network request. + */ + return function onRequestFinished(req) { + const wrappedReq = wrapObject(req, {} /* wrappers */, { + getContent: { + minArgs: 0, + maxArgs: 0, + }, + }); + listener(wrappedReq); + }; + }); + // Keep track if the deprecation warning has been logged at least once. let loggedSendResponseDeprecationWarning = false; @@ -480,6 +512,11 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object. }; const staticWrappers = { + devtools: { + network: { + onRequestFinished: wrapEvent(onRequestFinishedWrappers), + }, + }, runtime: { onMessage: wrapEvent(onMessageWrappers), onMessageExternal: wrapEvent(onMessageWrappers), diff --git a/test/test-onRequestFinished.js b/test/test-onRequestFinished.js new file mode 100644 index 0000000..41805b2 --- /dev/null +++ b/test/test-onRequestFinished.js @@ -0,0 +1,119 @@ +"use strict"; + +const {deepEqual, equal, ok} = require("chai").assert; +const sinon = require("sinon"); + +const {setupTestDOMWindow} = require("./setup"); + +describe("browser-polyfill", () => { + describe("wrapped devtools.network.onRequestFinished listener", () => { + it("does not wrap the listener if it is not a function", () => { + const fakeChrome = { + devtools: { + network: { + onRequestFinished: { + addListener: sinon.spy(), + }, + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + const fakeNonFunctionListener = {fake: "non function listener"}; + + const browserOnRequestFinished = window.browser.devtools.network.onRequestFinished; + browserOnRequestFinished.addListener(fakeNonFunctionListener); + + const fakeChromeOnRequestFinished = fakeChrome.devtools.network.onRequestFinished; + deepEqual( + fakeChromeOnRequestFinished.addListener.firstCall.args[0], + fakeNonFunctionListener, + "The non-function listener has not been wrapped" + ); + }); + }); + + it("promisifies the result", () => { + const fakeChrome = { + devtools: { + network: { + onRequestFinished: { + addListener: sinon.spy(), + hasListener: sinon.stub(), + removeListener: sinon.spy(), + }, + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + const listener = sinon.spy(); + + const browserOnRequestFinished = window.browser.devtools.network.onRequestFinished; + browserOnRequestFinished.addListener(listener); + + const fakeChromeOnRequestFinished = fakeChrome.devtools.network.onRequestFinished; + ok(fakeChromeOnRequestFinished.addListener.calledOnce, + "devtools.network.onRequestFinished.addListener has been called once"); + + const wrappedListener = fakeChromeOnRequestFinished.addListener.firstCall.args[0]; + wrappedListener({ + getContent(cb) { + cb("...", "text/html; charset=utf8"); + }, + }); + + ok(listener.calledOnce, "listener has been called once"); + + const req = listener.firstCall.args[0]; + return req.getContent().then(([content, encodingOrMimeType]) => { + equal(content, "..."); + // On Chrome this is the encoding ('' or 'base64') while on Firefox + // this is the MIME type of the resource. + // See: https://github.com/mozilla/webextension-polyfill/issues/249#issuecomment-740000461 + equal(encodingOrMimeType, "text/html; charset=utf8"); + }); + }); + }); + + it("promisifies the result with a wrapped Request object", () => { + const fakeChrome = { + devtools: { + network: { + onRequestFinished: { + addListener: sinon.spy(), + hasListener: sinon.stub(), + removeListener: sinon.spy(), + }, + }, + }, + }; + + return setupTestDOMWindow(fakeChrome).then(window => { + const listener = sinon.spy(); + + const browserOnRequestFinished = window.browser.devtools.network.onRequestFinished; + browserOnRequestFinished.addListener(listener); + + const fakeChromeOnRequestFinished = fakeChrome.devtools.network.onRequestFinished; + ok(fakeChromeOnRequestFinished.addListener.calledOnce, + "devtools.network.onRequestFinished.addListener has been called once"); + + const request = Object.create({ + inheritedProp: true, + getContent(cb) { + cb("", ""); + }, + }); + + const wrappedListener = fakeChromeOnRequestFinished.addListener.firstCall.args[0]; + wrappedListener(request); + + ok(listener.calledOnce, "listener has been called once"); + + const req = listener.firstCall.args[0]; + ok(req.inheritedProp, "Wrapped request inherited prototype properties"); + }); + }); + }); +});