commit 9792818fe62094984108b627cd6f0cce5cdf9f27
parent 2490e6d383b35ddf61643075de728d87030d1d04
Author: Julian Descottes <jdescottes@mozilla.com>
Date: Thu, 16 Oct 2025 17:48:42 +0000
Bug 1986704 - [devtools] Reduce markupview timeout to wait for selector on navigation to 1000 ms r=devtools-reviewers,nchevobbe
Differential Revision: https://phabricator.services.mozilla.com/D268685
Diffstat:
5 files changed, 125 insertions(+), 37 deletions(-)
diff --git a/devtools/client/inspector/test/browser_inspector_reload_iframe.js b/devtools/client/inspector/test/browser_inspector_reload_iframe.js
@@ -9,25 +9,34 @@
// We're loading an image that would take a few second to load so the iframe won't have
// its readyState to "complete" (it should be "interactive").
// That was causing some issue, see Bug 1733539.
-const IMG_URL = URL_ROOT_COM_SSL + "sjs_slow-loading-image.sjs";
-const FRAME_URI =
- "data:text/html;charset=utf-8," +
- encodeURI(`
- <div id="in-frame">div in the iframe</div>
- <img src="${IMG_URL}"></img>
- `);
-const HTML = `
- <iframe src="${FRAME_URI}"></iframe>
-`;
-
-const TEST_URI = "data:text/html;charset=utf-8," + encodeURI(HTML);
+
+function getTestURI(delay) {
+ const IMG_URL = `${URL_ROOT_COM_SSL}sjs_slow-loading-image.sjs?delay=${delay}`;
+ const FRAME_URI =
+ "data:text/html;charset=utf-8," +
+ encodeURI(`
+ <div id="in-frame">div in the iframe</div>
+ <img src="${IMG_URL}"></img>
+ `);
+ const HTML = `
+ <iframe src="${FRAME_URI}"></iframe>
+ `;
+
+ return "data:text/html;charset=utf-8," + encodeURI(HTML);
+}
add_task(async function () {
+ const { getSystemInfo } = require("resource://devtools/shared/system.js");
+
+ const INSPECTOR_FIND_NODE_TIMEOUT = 1000 * getSystemInfo().timeoutMultiplier;
+ const TEST_URI = getTestURI(INSPECTOR_FIND_NODE_TIMEOUT / 10);
+ const SLOW_TEST_URI = getTestURI(INSPECTOR_FIND_NODE_TIMEOUT + 5000);
+
const { inspector } = await openInspectorForURL(TEST_URI);
await selectNodeInFrames(["iframe", "#in-frame"], inspector);
- const markupLoaded = inspector.once("markuploaded");
+ let markupLoaded = inspector.once("markuploaded");
info("Reloading page.");
await navigateTo(TEST_URI);
@@ -45,4 +54,42 @@ add_task(async function () {
reloadedNodeFront,
"#in-frame selected after reload."
);
+
+ info("Check that markup view is not blocked for a page with a slow iframe");
+ await navigateTo(SLOW_TEST_URI);
+ await selectNodeInFrames(["iframe", "#in-frame"], inspector);
+
+ markupLoaded = inspector.once("markuploaded");
+
+ info("Reloading page.");
+ const onNavigate = navigateTo(SLOW_TEST_URI);
+
+ info("Waiting for markupview to load after reload.");
+ await markupLoaded;
+
+ info("Check that the navigation is not done yet");
+ ok(
+ !(await hasPromiseResolved(onNavigate)),
+ "Navigation to page with slow iframe is not done yet"
+ );
+
+ const bodyNodeFront = await getNodeFrontInFrames(["body"], inspector);
+ is(
+ inspector.selection.nodeFront,
+ bodyNodeFront,
+ "Iframe was too slow to load, the markup view selected body as fallback"
+ );
+
+ await onNavigate;
});
+
+async function hasPromiseResolved(promise) {
+ let resolved = false;
+
+ // Note that the catch() is only here to avoid leaking promise rejections.
+ // In all cases the resolved flag should be successfully flipped in finally().
+ promise.finally(() => (resolved = true)).catch(() => {});
+ // Make sure microtasks have time to run.
+ await new Promise(resolve => Services.tm.dispatchToMainThread(resolve));
+ return resolved;
+}
diff --git a/devtools/client/inspector/test/browser_inspector_reload_nested_iframe.js b/devtools/client/inspector/test/browser_inspector_reload_nested_iframe.js
@@ -8,7 +8,7 @@
const NESTED_IFRAME_URL = `https://example.com/document-builder.sjs?html=${encodeURIComponent(
"<h3>second level iframe</h3>"
-)}&delay=500`;
+)}&delay=100`;
const TEST_URI = `data:text/html;charset=utf-8,
<h1>Top-level</h1>
diff --git a/devtools/shared/commands/inspector/inspector-command.js b/devtools/shared/commands/inspector/inspector-command.js
@@ -17,6 +17,8 @@ loader.lazyRequireGetter(
true
);
+const { getSystemInfo } = require("resource://devtools/shared/system.js");
+
class InspectorCommand {
constructor({ commands }) {
this.commands = commands;
@@ -176,14 +178,14 @@ class InspectorCommand {
* Several selectors can be needed if the element is nested in frames
* and not directly in the root document.
* @param {Integer} timeoutInMs
- * The maximum number of ms the function should run (defaults to 5000).
+ * The maximum number of ms the function should run (defaults to 1000).
* If it exceeds this, the returned promise will resolve with `null`.
* @return {Promise<NodeFront|null>} a promise that resolves when the node front is found
* for selection using inspector tools. It resolves with the deepest frame document
* that could be retrieved when the "final" nodeFront couldn't be found in the page.
* It resolves with `null` when the function runs for more than timeoutInMs.
*/
- async findNodeFrontFromSelectors(nodeSelectors, timeoutInMs = 5000) {
+ async findNodeFrontFromSelectors(nodeSelectors, timeoutInMs = 1000) {
if (
!nodeSelectors ||
!Array.isArray(nodeSelectors) ||
@@ -251,9 +253,9 @@ class InspectorCommand {
// Since this is only used for re-setting a selection after a page reloads, we can
// put a timeout, in case there's an iframe that would take too much time to load,
// and prevent the markup view to be populated.
- const onTimeout = new Promise(res => setTimeout(res, timeoutInMs)).then(
- () => null
- );
+ const onTimeout = new Promise(res =>
+ setTimeout(res, timeoutInMs * getSystemInfo().timeoutMultiplier)
+ ).then(() => null);
const onQuerySelectors = querySelectors(rootNodeFront);
return Promise.race([onTimeout, onQuerySelectors]);
}
diff --git a/devtools/shared/commands/inspector/tests/browser_inspector_command_findNodeFrontFromSelectors.js b/devtools/shared/commands/inspector/tests/browser_inspector_command_findNodeFrontFromSelectors.js
@@ -4,6 +4,11 @@
"use strict";
add_task(async () => {
+ // This test istied to the legacy 5ms timeout for findNodeFrontFromSelectors.
+ // Restore the original timeout when calling findNodeFrontFromSelectors.
+ const { getSystemInfo } = require("resource://devtools/shared/system.js");
+ const legacyTimeout = 5000 / getSystemInfo().timeoutMultiplier;
+
// Build a simple test page with a remote iframe, using two distinct origins .com and .org
const iframeOrgHtml = encodeURIComponent(
`<h2 id="in-iframe">in org - same origin</h2>`
@@ -41,8 +46,10 @@ add_task(async () => {
);
info("Check that it returns null when a string is passed");
- nodeFront =
- await commands.inspectorCommand.findNodeFrontFromSelectors("body main");
+ nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors(
+ "body main",
+ legacyTimeout
+ );
is(
nodeFront,
null,
@@ -50,7 +57,10 @@ add_task(async () => {
);
info("Check it returns null when an empty array is passed");
- nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors([]);
+ nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors(
+ [],
+ legacyTimeout
+ );
is(
nodeFront,
null,
@@ -58,9 +68,10 @@ add_task(async () => {
);
info("Check that passing a selector for a non-matching element returns null");
- nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors([
- "h1",
- ]);
+ nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors(
+ ["h1"],
+ legacyTimeout
+ );
is(
nodeFront,
null,
@@ -68,9 +79,10 @@ add_task(async () => {
);
info("Check passing a selector for an element in the top document");
- nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors([
- "button",
- ]);
+ nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors(
+ ["button"],
+ legacyTimeout
+ );
is(
nodeFront.typeName,
"domnode",
@@ -83,10 +95,10 @@ add_task(async () => {
);
info("Check passing a selector for an element in a same origin iframe");
- nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors([
- "#iframe-org",
- "#in-iframe",
- ]);
+ nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors(
+ ["#iframe-org", "#in-iframe"],
+ legacyTimeout
+ );
is(
nodeFront.displayName,
"h2",
@@ -102,7 +114,7 @@ add_task(async () => {
// Default timeout for findNodeFrontFromSelectors is 5s.
// Iframe loads in 6s, with only 3s remaining when we reach this step.
// Use a bigger timeout to avoid intermittent failures.
- Services.appinfo.OS === "Darwin" ? 20000 : undefined
+ (Services.appinfo.OS === "Darwin" ? 4 : 1) * legacyTimeout
);
is(
nodeFront.displayName,
@@ -113,10 +125,10 @@ add_task(async () => {
info(
"Check passing a selector for an non-existing element in an existing iframe"
);
- nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors([
- "iframe",
- "#non-existant-id",
- ]);
+ nodeFront = await commands.inspectorCommand.findNodeFrontFromSelectors(
+ ["iframe", "#non-existant-id"],
+ legacyTimeout
+ );
is(
nodeFront.displayName,
"#document",
diff --git a/devtools/shared/system.js b/devtools/shared/system.js
@@ -9,6 +9,7 @@ loader.lazyRequireGetter(
"resource://devtools/server/devtools-server.js",
true
);
+loader.lazyRequireGetter(this, "flags", "resource://devtools/shared/flags.js");
const lazy = {};
ChromeUtils.defineESModuleGetters(
lazy,
@@ -157,6 +158,9 @@ function getSystemInfo() {
// Update channel
channel: lazy.AppConstants.MOZ_UPDATE_CHANNEL,
+ // Timeout multiplier for tests.
+ timeoutMultiplier: getTimeoutMultiplier(),
+
dpi,
useragent,
width,
@@ -199,4 +203,27 @@ function getProfileLocation() {
}
}
+/**
+ * Retrieve a timeout multiplier based on the platform. Slow platforms should
+ * use a bigger multiplier.
+ */
+function getTimeoutMultiplier() {
+ // Slow platform checks are only relevant to avoid issues in mochitests.
+ if (!flags.testing) {
+ return 1;
+ }
+
+ if (
+ lazy.AppConstants.DEBUG ||
+ lazy.AppConstants.MOZ_CODE_COVERAGE ||
+ lazy.AppConstants.ASAN
+ ) {
+ return 4;
+ }
+ if (lazy.AppConstants.TSAN) {
+ return 8;
+ }
+
+ return 1;
+}
exports.getSystemInfo = getSystemInfo;