Commit 7c81f0a2 authored by Nicolas Chevobbe's avatar Nicolas Chevobbe
Browse files

Bug 1660466 - Fix attaching target/thread test intermittents. r=jdescottes, a=RyanVM

Move the function taking care of closing the Browser Console to shared-head so
we can close it before closing the tabs opened during the test.

While attaching the worker, check that the Worker Debugger isn't closed, and
if it is, reject the promise. On the target list, catch rejection while attaching
and simply bail out so we don't call the rest of the code in onTargetAvailable.

Those guards are not enough to prevent any failure while attaching, so we're
wrapping calls to `attachAndInitThread`/`_onThreadInitialized` in try/catch
blocks to avoid test failures.

Differential Revision: https://phabricator.services.mozilla.com/D88766
parent 1a982041
......@@ -635,8 +635,16 @@ function TargetMixin(parentClass) {
// Before taking any action, notify listeners that destruction is imminent.
this.emit("close");
// If the target is being attached, try to wait until it's done, to prevent having
// pending connection to the server when the toolbox is destroyed.
if (this._onThreadInitialized) {
await this._onThreadInitialized;
try {
await this._onThreadInitialized;
} catch (e) {
// We might still get into cases where attaching fails (e.g. the worker we're
// trying to attach to is already closed). Since the target is being destroyed,
// we don't need to do anything special here.
}
}
for (let [, front] of this.fronts) {
......
......@@ -290,7 +290,18 @@ registerCleanupFunction(() => {
);
});
var {
BrowserConsoleManager,
} = require("devtools/client/webconsole/browser-console-manager");
registerCleanupFunction(async function cleanup() {
// Closing the browser console if there's one
const browserConsole = BrowserConsoleManager.getBrowserConsole();
if (browserConsole) {
browserConsole.targetList.destroy();
await safeCloseBrowserConsole({ clearOutput: true });
}
// Close any tab opened by the test.
// There should be only one tab opened by default when firefox starts the test.
while (gBrowser.tabs.length > 1) {
......@@ -320,6 +331,50 @@ registerCleanupFunction(async function cleanup() {
gDevTools.clearIsFissionContentToolboxEnabledReferenceForTest();
});
async function safeCloseBrowserConsole({ clearOutput = false } = {}) {
const hud = BrowserConsoleManager.getBrowserConsole();
if (!hud) {
return;
}
if (clearOutput) {
info("Clear the browser console output");
const { ui } = hud;
const promises = [ui.once("messages-cleared")];
// If there's an object inspector, we need to wait for the actors to be released.
if (ui.outputNode.querySelector(".object-inspector")) {
promises.push(ui.once("fronts-released"));
}
ui.clearOutput(true);
await Promise.all(promises);
info("Browser console cleared");
}
info("Wait for all Browser Console targets to be attached");
// It might happen that waitForAllTargetsToBeAttached does not resolve, so we set a
// timeout of 1s before closing
await Promise.race([
waitForAllTargetsToBeAttached(hud.targetList),
wait(1000),
]);
info("Close the Browser Console");
await BrowserConsoleManager.closeBrowserConsole();
info("Browser Console closed");
}
/**
* Returns a Promise that resolves when all the targets are fully attached.
*
* @param {TargetList} targetList
*/
function waitForAllTargetsToBeAttached(targetList) {
return Promise.allSettled(
targetList
.getAllTargets(targetList.ALL_TYPES)
.map(target => target._onThreadInitialized)
);
}
/**
* Add a new test tab in the browser and load the given url.
* @param {String} url The url to be loaded in the new tab
......
......@@ -46,22 +46,4 @@ add_task(async function() {
`"${expectedMessage}" should be still visible`
);
}
// Wait until the sourcemap worker is fully initialized
await waitForSourceMapWorker(hud);
});
function waitForSourceMapWorker(hud) {
const { targetList } = hud;
return new Promise(resolve => {
const onAvailable = ({ targetFront }) => {
if (
targetFront.url.endsWith("devtools/client/shared/source-map/worker.js")
) {
targetList.unwatchTargets([targetList.TYPES.WORKER], onAvailable);
resolve();
}
};
targetList.watchTargets([targetList.TYPES.WORKER], onAvailable);
});
}
......@@ -32,7 +32,7 @@ add_task(async function() {
is(messages.length, 1, "There is only the new message in the output");
info("Close and re-open the browser console");
await BrowserConsoleManager.toggleBrowserConsole();
await safeCloseBrowserConsole();
hud = await BrowserConsoleManager.toggleBrowserConsole();
info("Log a smoke message in order to know that the console is ready");
......
......@@ -80,8 +80,6 @@ add_task(async function testFilter() {
await resetFilters(hud);
await clearOutput(hud);
await waitForAllTargetsToBeAttached(hud);
await BrowserConsoleManager.closeBrowserConsole();
});
// Test that console.profile / profileEnd trigger the right events
......
......@@ -30,7 +30,7 @@ add_task(async function() {
await testExpandObject(objectMessage);
info("Restart the Browser Console");
await BrowserConsoleManager.toggleBrowserConsole();
await safeCloseBrowserConsole();
hud = await BrowserConsoleManager.toggleBrowserConsole();
info("Wait until the content object is displayed");
......@@ -40,9 +40,6 @@ add_task(async function() {
ok(true, "Content object is displayed in the Browser Console after restart");
await testExpandObject(objectMessage);
info("Close the browser console");
await BrowserConsoleManager.toggleBrowserConsole();
});
async function testExpandObject(objectMessage) {
......
......@@ -55,9 +55,6 @@ add_task(async function() {
oi.querySelector(".objectBox-array"),
JSON.stringify(["hello", "world"], null, 2)
);
info("Close the browser console");
await BrowserConsoleManager.toggleBrowserConsole();
});
async function testCopyObject(hud, element, expected) {
......
......@@ -113,12 +113,6 @@ add_task(async function() {
);
await hideContextMenu(hud);
// Close the browser console.
try {
await BrowserConsoleManager.toggleBrowserConsole();
} catch (e) {
console.warn("Error when closing Browser Console", e);
}
});
function addPrefBasedEntries(expectedEntries) {
......
......@@ -11,6 +11,10 @@ const TEST_URI =
"<button onclick='foobar.explode()'>click!</button>";
add_task(async function() {
// Disable the preloaded process as it creates processes intermittently
// which forces the emission of RDP requests we aren't correctly waiting for.
await pushPref("dom.ipc.processPrelaunch.enabled", false);
await pushPref("devtools.browserconsole.contentMessages", true);
await addTab(TEST_URI);
......@@ -50,7 +54,4 @@ add_task(async function() {
locationNode.click();
await onTabOpen;
ok(true, "The view source tab was opened in response to clicking the link");
await waitForAllTargetsToBeAttached(hud);
await BrowserConsoleManager.closeBrowserConsole();
});
......@@ -45,8 +45,8 @@ add_task(async function() {
);
info("Close the Browser Console");
await waitForAllTargetsToBeAttached(hud);
await BrowserConsoleManager.closeBrowserConsole();
await safeCloseBrowserConsole();
hud = BrowserConsoleManager.getBrowserConsole();
ok(!hud, "Browser Console has been closed");
});
......@@ -30,8 +30,7 @@ add_task(async function() {
info(
"Closing the browser console and waiting for the session restore to reopen it"
);
await waitForAllTargetsToBeAttached(hud);
await BrowserConsoleManager.closeBrowserConsole();
await safeCloseBrowserConsole();
const opened = waitForBrowserConsole(hud);
await gDevTools.restoreDevToolsSession({
......
......@@ -146,5 +146,5 @@ async function checkContentConsoleApiMessages(nonPrimitiveVariablesDisplayed) {
// would occassionally fail because the transport is closed before pending server
// responses have been sent.
await waitForTick();
await BrowserConsoleManager.toggleBrowserConsole();
await safeCloseBrowserConsole();
}
......@@ -67,7 +67,7 @@ add_task(async function() {
}, "web-console-destroyed");
});
await waitForAllTargetsToBeAttached(hud);
await waitForAllTargetsToBeAttached(hud.targetList);
waitForFocus(() => {
EventUtils.synthesizeKey("w", { accelKey: true }, hud.iframeWindow);
}, hud.iframeWindow);
......
......@@ -30,9 +30,8 @@ add_task(async function() {
hud = await BrowserConsoleManager.toggleBrowserConsole();
await testBrowserConsole(hud);
// clear and close the browser console.
// clear the browser console.
await clearOutput(hud);
await BrowserConsoleManager.toggleBrowserConsole();
});
async function testMessages(hud) {
......
......@@ -116,7 +116,7 @@ add_task(async function() {
assertNoPrivateMessages(hud);
info("close the browser console");
await BrowserConsoleManager.toggleBrowserConsole();
await safeCloseBrowserConsole();
info("reopen the browser console");
hud = await BrowserConsoleManager.toggleBrowserConsole();
......
......@@ -54,9 +54,9 @@ add_task(async function() {
// while the connection is being destroyed.
await waitForSourceMapWorker(browserConsole);
await closeConsole(browserTab);
await BrowserConsoleManager.toggleBrowserConsole();
Services.prefs.setBoolPref("devtools.chrome.enabled", false);
await safeCloseBrowserConsole();
Services.prefs.setBoolPref("devtools.chrome.enabled", false);
browserConsole = await BrowserConsoleManager.toggleBrowserConsole();
objInspector = await logObject(browserConsole);
testInputRelatedElementsAreNotVisibile(browserConsole);
......@@ -72,7 +72,7 @@ add_task(async function() {
// while the connection is being destroyed.
await waitForSourceMapWorker(browserConsole);
await closeConsole(browserTab);
await BrowserConsoleManager.toggleBrowserConsole();
await safeCloseBrowserConsole();
});
async function logObject(hud) {
......
......@@ -32,6 +32,7 @@ Services.scriptloader.loadSubScript(
var {
BrowserConsoleManager,
} = require("devtools/client/webconsole/browser-console-manager");
var WCUL10n = require("devtools/client/webconsole/utils/l10n");
const DOCS_GA_PARAMS = `?${new URLSearchParams({
utm_source: "mozilla",
......@@ -55,13 +56,6 @@ registerCleanupFunction(async function() {
Services.prefs.getChildList("devtools.webconsole.filter").forEach(pref => {
Services.prefs.clearUserPref(pref);
});
const browserConsole = BrowserConsoleManager.getBrowserConsole();
if (browserConsole) {
browserConsole.targetList.destroy();
await clearOutput(browserConsole);
await waitForAllTargetsToBeAttached(browserConsole);
await BrowserConsoleManager.closeBrowserConsole();
}
});
/**
......@@ -142,19 +136,6 @@ async function openNewWindowAndConsole(url) {
return { win, hud, tab };
}
/**
* Returns a Promise that resolves when all the targets are fully attached.
*
* @param {WebConsole} hud
*/
function waitForAllTargetsToBeAttached(hud) {
return Promise.all(
hud.targetList
.getAllTargets(hud.targetList.ALL_TYPES)
.map(target => target.attachAndInitThread())
);
}
/**
* Subscribe to the store and log out stringinfied versions of messages.
* This is a helper function for debugging, to make is easier to see what
......
......@@ -19,6 +19,10 @@ loader.lazyRequireGetter(
*/
function connectToWorker(connection, dbg, id, options) {
return new Promise((resolve, reject) => {
if (dbg.isClosed) {
reject("closed");
}
// Step 1: Ensure the worker debugger is initialized.
if (!dbg.isInitialized) {
dbg.initialize("resource://devtools/server/startup/worker.js");
......@@ -81,6 +85,10 @@ function connectToWorker(connection, dbg, id, options) {
dbg.addListener(listener);
}
if (dbg.isClosed) {
reject("closed");
}
// Step 2: Send a connect request to the worker debugger.
dbg.postMessage(
JSON.stringify({
......
......@@ -18,6 +18,10 @@ class LegacyProcessesWatcher {
}
async _processListChanged() {
if (this.targetList.isDestroyed()) {
return;
}
const processes = await this.rootFront.listProcesses();
// Process the new list to detect the ones being destroyed
// Force destroyed the descriptor as well as the target
......
......@@ -163,6 +163,10 @@ class LegacyServiceWorkersWatcher extends LegacyWorkersWatcher {
}
async _onRegistrationListChanged() {
if (this.targetList.isDestroyed()) {
return;
}
await this._updateRegistrations();
// Everything after this point is not strictly necessary for sw support
......
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