Deleting a node in a same-process frame and navigating will freeze the content process
Categories
(DevTools :: General, defect, P2)
Tracking
(firefox-esr78 unaffected, firefox86 wontfix, firefox87 wontfix, firefox88 fixed)
| Tracking | Status | |
|---|---|---|
| firefox-esr78 | --- | unaffected |
| firefox86 | --- | wontfix |
| firefox87 | --- | wontfix |
| firefox88 | --- | fixed |
People
(Reporter: mcccs, Assigned: jdescottes)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files)
Steps to reproduce:
- Open the HTML file below
- Right click on the Google logo
- Choose "Inspect Element"
- Hit backspace once to remove the frontmost div
- Right click on the Google logo again
- Choose "View Image"
Kill the content process or Firefox won't quit.
<html><head></head><body>
<iframe style="width:100%;height:100%" srcdoc="<!doctype html>
<html>
<head></head>
<body>
<iframe srcdoc='<!doctype html><html><head></head><body style=width:1000px;height:1500px;margin:0>
<div>
<img style=position:absolute;top:0;left:0 src="https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png">
</div>
</body></html>' width=100% height=100% frameborder=0></iframe>
<div style='position:absolute;top:0;bottom:0;left:0;right:0;background-color:rgba(0,0,0,0)'></div>
</body>
</html>
"></iframe>
</body></html>
|
||
Updated•4 years ago
|
|
Assignee | |
Comment 1•4 years ago
|
||
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 2•4 years ago
|
||
Thanks for filing, and adding an example.
I reduced the STRs a bit:
- open a page with a same-process iframe
- delete an element in the iframe using the inspector
- try to navigate to a new URL
The issue will not be triggered if the deleted element is in a remote frame.
It seems linked to nodes retained by the devtools server. Before deleting a node we first "retain" it : https://searchfox.org/mozilla-central/rev/df23c6e58c6be1eb8399e475878f73d4867bef87/devtools/client/inspector/markup/markup.js#1287
On the server this will simply set the node.retained flag to true on the node actor. This will trigger some additional processing when unloading the frame, especially a call to _childOfWindow. This method seems completely buggy and I this must have been broken for a very long time:
_childOfWindow: function(window, domNode) {
let win = nodeDocument(domNode).defaultView;
while (win) {
if (win === window) {
return true;
}
win = getFrameElement(win);
}
return false;
},
getFrameElement returns a <frame> element, not a window object. And if you look at the implementation of getFrameElement:
const getFrameElement = win => {
const isTopWindow = win && DevToolsUtils.getTopWindow(win) === win;
return isTopWindow ? null : win.browsingContext.embedderElement;
};
If win here is actually a frame element, win.browsingContext.embedderElement will just return the same element (because the browsingContext property on a frame element points to browsingContext embedded in the frame.
Meaning _childOfWindow is an infinite while loop which keeps testing the same frame element forever.
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 3•4 years ago
|
||
This broke after Bug 1647438 which modified the implementation of getFrameElement.
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 4•4 years ago
|
||
Before Bug 1647438, getFrameElement had a different implementation:
const getFrameElement = win => {
const isTopWindow = win && DevToolsUtils.getTopWindow(win) === win;
return isTopWindow ? null : utilsFor(win).containerElement;
};
utilsFor is a memoized helper which returns win.windowUtils. And on an iframeElement, windowUtils is undefined, which means that accessing containerElement threw an exception. Code was still buggy but it just didn't block the process.
|
|
Assignee | |
Comment 5•4 years ago
|
||
Deleting a node with the inspector puts the corresponding NodeActor in the retained nodes, in case the user cancels the deletion.
When a frame is unloaded we try to remove all the retained node to free memory.
For each retained node, we try to climb up the window hierarchy to know if it should be released or not.
However the implementation was buggy, relying on a while loop which was incorrectly mixing window objects and frame elements.
This previously lead to errors, and now leads to infinite loops.
A new test is added to cover this scenario.
|
||
Updated•4 years ago
|
|
||
Updated•4 years ago
|
|
||
Comment 7•4 years ago
|
||
| bugherder | ||
|
||
Updated•4 years ago
|
Description
•