Skip to content

Commit f86ba2f

Browse files
ochameauDonalMe
authored andcommitted
Bug 2003810 - [devtools] Nullify WindowGlobalTarget actor when the underlying WindowGlobal navigated to another origin and process. r=devtools-reviewers,jdescottes a=dmeehan
This prevents having to special case the "isRemoteProxy" edgecase in all actors. Differential Revision: https://phabricator.services.mozilla.com/D277446
1 parent d26d368 commit f86ba2f

File tree

4 files changed

+109
-1
lines changed

4 files changed

+109
-1
lines changed

devtools/client/inspector/test/browser.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ skip-if = [
154154

155155
["browser_inspector_picker-no-page-events.js"]
156156

157+
["browser_inspector_picker-page-reload.js"]
158+
157159
["browser_inspector_picker-reset-reference.js"]
158160

159161
["browser_inspector_picker-shift-key.js"]
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
"use strict";
6+
7+
// Test that the node picker works while reloading a page with
8+
// an iframe going through an intermediate about:blank document
9+
10+
const FRAME_URL = "https://example.com/document-builder.sjs?html=iframe";
11+
const TEST_URL =
12+
`https://example.org/document-builder.sjs?html=` +
13+
encodeURI(
14+
`<h1>Pick target</h1><h2>Other element</h2><iframe></iframe><script>window.onload=()=>{document.querySelector("iframe").src = "${FRAME_URL}";}</script>`
15+
);
16+
17+
async function waitForIframeLoad() {
18+
const iframeBC = await SpecialPowers.spawn(
19+
gBrowser.selectedBrowser,
20+
[],
21+
function () {
22+
return content.document.querySelector("iframe").browsingContext;
23+
}
24+
);
25+
await SpecialPowers.spawn(iframeBC, [FRAME_URL], async function (url) {
26+
if (
27+
content.document.readyState == "complete" &&
28+
content.location.href == url
29+
) {
30+
return null;
31+
}
32+
return new Promise(resolve => {
33+
content.addEventListener("load", resolve, { once: true });
34+
});
35+
});
36+
}
37+
38+
add_task(async () => {
39+
let { inspector, toolbox, highlighterTestFront } =
40+
await openInspectorForURL(TEST_URL);
41+
await waitForIframeLoad();
42+
43+
info(
44+
"Start the picker and hover an element to populate the picker hovered node reference"
45+
);
46+
await startPicker(toolbox);
47+
await hoverElement(inspector, "iframe");
48+
49+
const HIGHLIGHTER_TYPE = inspector.highlighters.TYPES.BOXMODEL;
50+
const { getActiveHighlighter } = getHighlighterTestHelpers(inspector);
51+
52+
ok(getActiveHighlighter(HIGHLIGHTER_TYPE), "Boxmodel highlighter is shown.");
53+
ok(
54+
await highlighterTestFront.assertHighlightedNode("iframe"),
55+
"The highlighter is shown on the expected node"
56+
);
57+
58+
// Do not wait for full reload as the faulty target for transient about:blank
59+
// document may already be destroyed and not cause troubles anymore
60+
const onReloaded = reloadSelectedTab();
61+
62+
// Bug 2003810: hovering during the navigation may throw error in the content process,
63+
// but this exception wouldn't make the test fail anyway.
64+
hoverElement(inspector, "h1");
65+
66+
await onReloaded;
67+
68+
// As the previous hover may not complete, we have to do another one once the page is loaded
69+
await hoverElement(inspector, "h1");
70+
ok(getActiveHighlighter(HIGHLIGHTER_TYPE), "Boxmodel highlighter is shown.");
71+
72+
highlighterTestFront = await getHighlighterTestFront(toolbox);
73+
ok(
74+
await highlighterTestFront.assertHighlightedNode("h1"),
75+
"The highlighter is shown on the expected node"
76+
);
77+
78+
await stopPickerWithEscapeKey(toolbox);
79+
80+
// Bug 2003810 highlighted that the toolbox couldn't be re-opened
81+
// as the broken, not correctly unregistered/disabled node-picker was keeping
82+
// its suppressedEventListener active and possibly disable all further key presses.
83+
// But there is no way to reproduce the exact same keypress event.
84+
// BrowserTestUtils.synthesizeKey/EventUtils.synthesizeNativeKey both seem to send the event only to the content.
85+
// And EventUtils.synthesizeKey doesn't, but avoids reproducing the regression.
86+
const onToolboxDestroyed = toolbox.once("destroyed");
87+
info("Try to close the toolbox via a key shortcut");
88+
if (Services.appinfo.OS == "Darwin") {
89+
EventUtils.synthesizeKey("i", { accelKey: true, altKey: true });
90+
} else {
91+
EventUtils.synthesizeKey("i", { accelKey: true, shiftKey: true });
92+
}
93+
await onToolboxDestroyed;
94+
});

devtools/server/actors/inspector/node-picker.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ class NodePicker {
389389
* @param callback The function to call with suppressed events, or null.
390390
*/
391391
_setSuppressedEventListener(callback) {
392+
// The related document may already have been destroyed, or moved to another origin/process.
392393
if (!this._targetActor?.window?.document) {
393394
return;
394395
}

devtools/server/actors/targets/window-global.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ class WindowGlobalTargetActor extends BaseTargetActor {
414414
});
415415
Object.defineProperty(this, "window", {
416416
value: this.window,
417-
configurable: false,
417+
configurable: true,
418418
writable: false,
419419
});
420420
Object.defineProperty(this, "chromeEventHandler", {
@@ -806,6 +806,17 @@ class WindowGlobalTargetActor extends BaseTargetActor {
806806
}
807807
this.destroying = true;
808808

809+
// In case the window already navigated to another origin,
810+
// which is possibly in another process, nullify window
811+
// as most, if not all attributes would throw.
812+
if (Cu.isRemoteProxy(this.window)) {
813+
Object.defineProperty(this, "window", {
814+
value: null,
815+
configurable: true,
816+
writable: false,
817+
});
818+
}
819+
809820
// Force flushing pending resources if the actor isn't already destroyed.
810821
// This helps notify the client about pending resources on navigation.
811822
if (!this.isDestroyed()) {

0 commit comments

Comments
 (0)