Skip to content

Commit 7cb64f0

Browse files
committed
Bug 1760077 - Fix racy test_channel_close.js r=necko-reviewers,kershaw
ChannelListener throws an exception if the channel gets closed unexpectedly. Since this test was racing live_channels[1] vs local_channel, sometimes the server would get closed before local_channel completed, causing an exception. This patch introduces SimpleChannelListener which doesn't include so much logic and may be a better fit when a channel might succeed or fail depending on certain circumstances. Differential Revision: https://phabricator.services.mozilla.com/D202413
1 parent 1bf6a99 commit 7cb64f0

File tree

3 files changed

+48
-25
lines changed

3 files changed

+48
-25
lines changed

netwerk/test/unit/head_channels.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,3 +525,27 @@ function makeHTTPChannel(url, with_proxy) {
525525
loadUsingSystemPrincipal: true,
526526
}).QueryInterface(Ci.nsIHttpChannel);
527527
}
528+
529+
// Like ChannelListener but does not throw an exception if something
530+
// goes wrong. Callback is supposed to do all the work.
531+
class SimpleChannelListener {
532+
constructor(callback) {
533+
this._onStopCallback = callback;
534+
this._buffer = "";
535+
}
536+
get QueryInterface() {
537+
return ChromeUtils.generateQI(["nsIStreamListener", "nsIRequestObserver"]);
538+
}
539+
540+
onStartRequest(request) {}
541+
542+
onDataAvailable(request, stream, offset, count) {
543+
this._buffer = this._buffer.concat(read_stream(stream, count));
544+
}
545+
546+
onStopRequest(request, status) {
547+
if (this._onStopCallback) {
548+
this._onStopCallback(request, this._buffer);
549+
}
550+
}
551+
}

netwerk/test/unit/test_channel_close.js

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,41 @@ var httpbody = "0123456789";
1818

1919
var live_channels = [];
2020

21-
function run_test() {
21+
add_task(async function test() {
2222
httpserver.registerPathHandler(testpath, serverHandler);
2323
httpserver.start(-1);
24+
registerCleanupFunction(async () => {
25+
if (httpserver) {
26+
await httpserver.stop();
27+
}
28+
});
2429

25-
httpProtocolHandler.EnsureHSTSDataReady().then(function () {
26-
var local_channel;
30+
await httpProtocolHandler.EnsureHSTSDataReady();
2731

28-
// Opened channel that has no remaining references on shutdown
29-
local_channel = setupChannel(testpath);
30-
local_channel.asyncOpen(new ChannelListener(checkRequest, local_channel));
32+
// Opened channel that has no remaining references on shutdown
33+
let local_channel = setupChannel(testpath);
34+
local_channel.asyncOpen(new SimpleChannelListener());
3135

32-
// Opened channel that has no remaining references after being opened
33-
setupChannel(testpath).asyncOpen(new ChannelListener(function () {}, null));
36+
// Opened channel that has no remaining references after being opened
37+
setupChannel(testpath).asyncOpen(new SimpleChannelListener());
3438

35-
// Unopened channel that has remaining references on shutdown
36-
live_channels.push(setupChannel(testpath));
39+
// Unopened channel that has remaining references on shutdown
40+
live_channels.push(setupChannel(testpath));
3741

38-
// Opened channel that has remaining references on shutdown
39-
live_channels.push(setupChannel(testpath));
42+
// Opened channel that has remaining references on shutdown
43+
live_channels.push(setupChannel(testpath));
44+
await new Promise(resolve => {
4045
live_channels[1].asyncOpen(
41-
new ChannelListener(checkRequestFinish, live_channels[1])
46+
new SimpleChannelListener((req, data) => {
47+
Assert.equal(data, httpbody);
48+
resolve();
49+
})
4250
);
4351
});
4452

45-
do_test_pending();
46-
}
53+
await httpserver.stop();
54+
httpserver = null;
55+
});
4756

4857
function setupChannel(path) {
4958
var chan = NetUtil.newChannel({
@@ -59,12 +68,3 @@ function serverHandler(metadata, response) {
5968
response.setHeader("Content-Type", "text/plain", false);
6069
response.bodyOutputStream.write(httpbody, httpbody.length);
6170
}
62-
63-
function checkRequest(request, data, context) {
64-
Assert.equal(data, httpbody);
65-
}
66-
67-
function checkRequestFinish(request, data, context) {
68-
checkRequest(request, data, context);
69-
httpserver.stop(do_test_finished);
70-
}

netwerk/test/unit/xpcshell.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,6 @@ run-sequentially = "node server exceptions dont replay well"
434434
run-sequentially = "node server exceptions dont replay well"
435435

436436
["test_channel_close.js"]
437-
skip-if = ["socketprocess_networking"]
438437

439438
["test_channel_long_domain.js"]
440439

0 commit comments

Comments
 (0)