Commit 1a982041 authored by Nicolas Chevobbe's avatar Nicolas Chevobbe
Browse files

Bug 1662054 - Add a destroy function to targetList. r=jdescottes, a=RyanVM

At the moment, we don't have any guards in the targetList to _not_ execute the
creation/destruction listeners once the toolbox gets destroyed.
We only have a stopListening function on the targetList that is called when we
close the toolbox, but we can't rely only on that since it's also called when
doing a target switch (and working around that is very racy).

One solution would be to follow the common pattern we have everywhere by having
a destroy method that we would check before trying to call the listeners callback.

This might help with intermittent test failures.

Differential Revision: https://phabricator.services.mozilla.com/D88765
parent 81258220
......@@ -3643,7 +3643,7 @@ Toolbox.prototype = {
this._onTargetDestroyed
);
this.targetList.stopListening();
this.targetList.destroy();
// Unregister buttons listeners
this.toolbarButtons.forEach(button => {
......
......@@ -294,7 +294,7 @@ class ResponsiveUI {
[this.targetList.TYPES.FRAME],
this.onTargetAvailable
);
this.targetList.stopListening();
this.targetList.destroy();
}
this.tab.removeEventListener("TabClose", this);
......
......@@ -113,7 +113,7 @@ class BrowserConsole extends WebConsole {
// toolbox session id.
this._telemetry.toolClosed("browserconsole", -1, this);
await this.targetList.stopListening();
this.targetList.destroy();
// Wait for any pending connection initialization.
await Promise.all(
this.ui.getAllProxies().map(proxy => proxy.getConnectionPromise())
......
......@@ -101,7 +101,7 @@ async function generatePlatformMessagesStubs() {
stubs.set(key, getCleanedPacket(key, packet));
}
resourceWatcher.targetList.stopListening();
resourceWatcher.targetList.destroy();
await client.close();
return stubs;
......
......@@ -57,7 +57,7 @@ registerCleanupFunction(async function() {
});
const browserConsole = BrowserConsoleManager.getBrowserConsole();
if (browserConsole) {
browserConsole.targetList.stopListening();
browserConsole.targetList.destroy();
await clearOutput(browserConsole);
await waitForAllTargetsToBeAttached(browserConsole);
await BrowserConsoleManager.closeBrowserConsole();
......
......@@ -149,7 +149,7 @@ class TargetList extends EventEmitter {
return;
}
if (targetFront.isDestroyedOrBeingDestroyed()) {
if (this.isDestroyed() || targetFront.isDestroyedOrBeingDestroyed()) {
return;
}
......@@ -559,6 +559,17 @@ class TargetList extends EventEmitter {
isTargetRegistered(targetFront) {
return this._targets.has(targetFront);
}
isDestroyed() {
return this._isDestroyed;
}
destroy() {
this.stopListening();
this._createListeners.off();
this._destroyListeners.off();
this._isDestroyed = true;
}
}
/**
......
......@@ -47,7 +47,7 @@ add_task(async function() {
assertContents(cachedResources1, messages);
assertResources(cachedResources2, cachedResources1);
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......@@ -97,7 +97,7 @@ add_task(async function() {
assertContents(availableResources, allMessages);
assertResources(cachedResources, availableResources);
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......@@ -140,7 +140,7 @@ add_task(async function() {
is(cachedResources.length, 0, "The cache in ResourceWatcher is cleared");
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......@@ -200,7 +200,7 @@ add_task(async function() {
assertResources(cachedResources, availableResources);
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......@@ -271,7 +271,7 @@ async function testIgnoreExistingResources(isFirstListenerIgnoreExisting) {
assertContents(cachedResourcesWithFlag, additionalMessages);
assertContents(cachedResourcesWithoutFlag, allMessages);
await targetList.stopListening();
await targetList.destroy();
await client.close();
}
......
......@@ -78,7 +78,7 @@ async function testConsoleMessagesResources() {
"Got the expected number of runtime messages"
);
targetList.stopListening();
targetList.destroy();
await client.close();
}
......@@ -129,7 +129,7 @@ async function testConsoleMessagesResourcesWithIgnoreExistingResources() {
checkConsoleAPICall(message, expected);
}
await targetList.stopListening();
await targetList.destroy();
await client.close();
}
......
......@@ -91,7 +91,7 @@ add_task(async function() {
is(availableResources[2], existingResources[2], "3rd resource is correct");
is(availableResources[3], existingResources[3], "4th resource is correct");
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......
......@@ -70,7 +70,7 @@ async function testWatchingCssMessages() {
ok(true, "All the expected CSS messages were received");
Services.console.reset();
targetList.stopListening();
targetList.destroy();
await client.close();
}
......@@ -119,7 +119,7 @@ async function testWatchingCachedCssMessages() {
is(receivedMessages.length, 3, "Cached messages were retrieved as expected");
Services.console.reset();
targetList.stopListening();
targetList.destroy();
await client.close();
}
......
......@@ -59,7 +59,7 @@ async function testDocumentEventResources() {
);
ok(true, "Document events are fired after reloading");
await targetList.stopListening();
await targetList.destroy();
await client.close();
}
......@@ -88,7 +88,7 @@ async function testDocumentEventResourcesWithIgnoreExistingResources() {
await waitUntil(() => documentEvents.length === 3);
assertEvents(...documentEvents);
await targetList.stopListening();
await targetList.destroy();
await client.close();
}
......
......@@ -96,7 +96,7 @@ async function testErrorMessagesResources() {
ok(true, "All the expected errors were received");
Services.console.reset();
targetList.stopListening();
targetList.destroy();
await client.close();
}
......@@ -140,7 +140,7 @@ async function testErrorMessagesResourcesWithIgnoreExistingResources() {
}
Services.console.reset();
await targetList.stopListening();
await targetList.destroy();
await client.close();
}
......
......@@ -77,7 +77,7 @@ add_task(async function() {
[]
);
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......
......@@ -286,7 +286,7 @@ async function testNetworkEventResources(options) {
onUpdated: onResourceUpdated,
}
);
await targetList.stopListening();
await targetList.destroy();
await client.close();
BrowserTestUtils.removeTab(tab);
}
......
......@@ -88,7 +88,7 @@ async function testPlatformMessagesResources() {
ok(true, "All the expected messages were received");
Services.console.reset();
targetList.stopListening();
targetList.destroy();
await client.close();
}
......@@ -140,6 +140,6 @@ async function testPlatformMessagesResourcesWithIgnoreExistingResources() {
}
Services.console.reset();
await targetList.stopListening();
await targetList.destroy();
await client.close();
}
......@@ -70,7 +70,7 @@ add_task(async function() {
);
// Cleanup
targetList.stopListening();
targetList.destroy();
await client.close();
});
......@@ -131,6 +131,6 @@ add_task(async function testRootNodeFrontIsCorrect() {
resourceWatcher.unwatchResources([ResourceWatcher.TYPES.ROOT_NODE], {
onAvailable,
});
targetList.stopListening();
targetList.destroy();
await client.close();
});
......@@ -107,7 +107,7 @@ add_task(async function() {
);
// Cleanup
targetList.stopListening();
targetList.destroy();
await client.close();
});
......
......@@ -132,7 +132,7 @@ add_task(async function() {
Object.assign(expectedResource, { styleText: "" })
);
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......
......@@ -56,7 +56,7 @@ add_task(async function() {
});
info("Close the client, which will destroy the target");
targetList.stopListening();
targetList.destroy();
await client.close();
info(
......
......@@ -117,7 +117,7 @@ add_task(async function() {
);
}
await targetList.stopListening();
await targetList.destroy();
await client.close();
});
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment