Skip to content

Commit 716c90b

Browse files
dgp1130Rob--W
authored andcommitted
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()`.
1 parent 3ba72c9 commit 716c90b

File tree

2 files changed

+163
-7
lines changed

2 files changed

+163
-7
lines changed

src/browser-polyfill.js

+44-7
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,17 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
7777
* promise.
7878
* @param {function} promise.resolve
7979
* The promise's resolution function.
80-
* @param {function} promise.rejection
80+
* @param {function} promise.reject
8181
* The promise's rejection function.
8282
* @param {object} metadata
8383
* Metadata about the wrapped method which has created the callback.
84-
* @param {integer} metadata.maxResolvedArgs
85-
* The maximum number of arguments which may be passed to the
86-
* callback created by the wrapped async function.
84+
* @param {boolean} metadata.singleCallbackArg
85+
* Whether or not the promise is resolved with only the first
86+
* argument of the callback, alternatively an array of all the
87+
* callback arguments is resolved. By default, if the callback
88+
* function is invoked with only a single argument, that will be
89+
* resolved to the promise, while all arguments will be resolved as
90+
* an array if multiple are given.
8791
*
8892
* @returns {function}
8993
* The generated callback function.
@@ -118,9 +122,13 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
118122
* The maximum number of arguments which may be passed to the
119123
* function. If called with more than this number of arguments, the
120124
* wrapper will raise an exception.
121-
* @param {integer} metadata.maxResolvedArgs
122-
* The maximum number of arguments which may be passed to the
123-
* callback created by the wrapped async function.
125+
* @param {boolean} metadata.singleCallbackArg
126+
* Whether or not the promise is resolved with only the first
127+
* argument of the callback, alternatively an array of all the
128+
* callback arguments is resolved. By default, if the callback
129+
* function is invoked with only a single argument, that will be
130+
* resolved to the promise, while all arguments will be resolved as
131+
* an array if multiple are given.
124132
*
125133
* @returns {function(object, ...*)}
126134
* The generated wrapper function.
@@ -345,6 +353,30 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
345353
},
346354
});
347355

356+
const onRequestFinishedWrappers = new DefaultWeakMap(listener => {
357+
if (typeof listener !== "function") {
358+
return listener;
359+
}
360+
361+
/**
362+
* Wraps an onRequestFinished listener function so that it will return a
363+
* `getContent()` property which returns a `Promise` rather than using a
364+
* callback API.
365+
*
366+
* @param {object} req
367+
* The HAR entry object representing the network request.
368+
*/
369+
return function onRequestFinished(req) {
370+
const wrappedReq = wrapObject(req, {} /* wrappers */, {
371+
getContent: {
372+
minArgs: 0,
373+
maxArgs: 0,
374+
},
375+
});
376+
listener(wrappedReq);
377+
};
378+
});
379+
348380
// Keep track if the deprecation warning has been logged at least once.
349381
let loggedSendResponseDeprecationWarning = false;
350382

@@ -480,6 +512,11 @@ if (typeof browser === "undefined" || Object.getPrototypeOf(browser) !== Object.
480512
};
481513

482514
const staticWrappers = {
515+
devtools: {
516+
network: {
517+
onRequestFinished: wrapEvent(onRequestFinishedWrappers),
518+
},
519+
},
483520
runtime: {
484521
onMessage: wrapEvent(onMessageWrappers),
485522
onMessageExternal: wrapEvent(onMessageWrappers),

test/test-onRequestFinished.js

+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
"use strict";
2+
3+
const {deepEqual, equal, ok} = require("chai").assert;
4+
const sinon = require("sinon");
5+
6+
const {setupTestDOMWindow} = require("./setup");
7+
8+
describe("browser-polyfill", () => {
9+
describe("wrapped devtools.network.onRequestFinished listener", () => {
10+
it("does not wrap the listener if it is not a function", () => {
11+
const fakeChrome = {
12+
devtools: {
13+
network: {
14+
onRequestFinished: {
15+
addListener: sinon.spy(),
16+
},
17+
},
18+
},
19+
};
20+
21+
return setupTestDOMWindow(fakeChrome).then(window => {
22+
const fakeNonFunctionListener = {fake: "non function listener"};
23+
24+
const browserOnRequestFinished = window.browser.devtools.network.onRequestFinished;
25+
browserOnRequestFinished.addListener(fakeNonFunctionListener);
26+
27+
const fakeChromeOnRequestFinished = fakeChrome.devtools.network.onRequestFinished;
28+
deepEqual(
29+
fakeChromeOnRequestFinished.addListener.firstCall.args[0],
30+
fakeNonFunctionListener,
31+
"The non-function listener has not been wrapped"
32+
);
33+
});
34+
});
35+
36+
it("promisifies the result", () => {
37+
const fakeChrome = {
38+
devtools: {
39+
network: {
40+
onRequestFinished: {
41+
addListener: sinon.spy(),
42+
hasListener: sinon.stub(),
43+
removeListener: sinon.spy(),
44+
},
45+
},
46+
},
47+
};
48+
49+
return setupTestDOMWindow(fakeChrome).then(window => {
50+
const listener = sinon.spy();
51+
52+
const browserOnRequestFinished = window.browser.devtools.network.onRequestFinished;
53+
browserOnRequestFinished.addListener(listener);
54+
55+
const fakeChromeOnRequestFinished = fakeChrome.devtools.network.onRequestFinished;
56+
ok(fakeChromeOnRequestFinished.addListener.calledOnce,
57+
"devtools.network.onRequestFinished.addListener has been called once");
58+
59+
const wrappedListener = fakeChromeOnRequestFinished.addListener.firstCall.args[0];
60+
wrappedListener({
61+
getContent(cb) {
62+
cb("<html>...</html>", "text/html; charset=utf8");
63+
},
64+
});
65+
66+
ok(listener.calledOnce, "listener has been called once");
67+
68+
const req = listener.firstCall.args[0];
69+
return req.getContent().then(([content, encodingOrMimeType]) => {
70+
equal(content, "<html>...</html>");
71+
// On Chrome this is the encoding ('' or 'base64') while on Firefox
72+
// this is the MIME type of the resource.
73+
// See: https://github.com/mozilla/webextension-polyfill/issues/249#issuecomment-740000461
74+
equal(encodingOrMimeType, "text/html; charset=utf8");
75+
});
76+
});
77+
});
78+
79+
it("promisifies the result with a wrapped Request object", () => {
80+
const fakeChrome = {
81+
devtools: {
82+
network: {
83+
onRequestFinished: {
84+
addListener: sinon.spy(),
85+
hasListener: sinon.stub(),
86+
removeListener: sinon.spy(),
87+
},
88+
},
89+
},
90+
};
91+
92+
return setupTestDOMWindow(fakeChrome).then(window => {
93+
const listener = sinon.spy();
94+
95+
const browserOnRequestFinished = window.browser.devtools.network.onRequestFinished;
96+
browserOnRequestFinished.addListener(listener);
97+
98+
const fakeChromeOnRequestFinished = fakeChrome.devtools.network.onRequestFinished;
99+
ok(fakeChromeOnRequestFinished.addListener.calledOnce,
100+
"devtools.network.onRequestFinished.addListener has been called once");
101+
102+
const request = Object.create({
103+
inheritedProp: true,
104+
getContent(cb) {
105+
cb("", "");
106+
},
107+
});
108+
109+
const wrappedListener = fakeChromeOnRequestFinished.addListener.firstCall.args[0];
110+
wrappedListener(request);
111+
112+
ok(listener.calledOnce, "listener has been called once");
113+
114+
const req = listener.firstCall.args[0];
115+
ok(req.inheritedProp, "Wrapped request inherited prototype properties");
116+
});
117+
});
118+
});
119+
});

0 commit comments

Comments
 (0)