Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Detached Elements" does not show nodes that were never attached #190

Open
warpech opened this issue Oct 13, 2023 · 6 comments
Open

"Detached Elements" does not show nodes that were never attached #190

warpech opened this issue Oct 13, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@warpech
Copy link

warpech commented Oct 13, 2023

Hi, Thanks for a great tool!

I discovered one problem with it that I think should be treated as a bug. Or, probably the docs need a clarification.


The announcement blog post (https://blogs.windows.com/msedgedev/2021/12/09/debug-memory-leaks-detached-elements-tool-devtools/) gives 2 valid reasons why elements can be unattached (numbering mine):

Usually, we create DOM nodes in JavaScript to insert them somewhere in the page to display them. But it is possible to create nodes and
(1) never attach them or
(2) remove nodes from the page and keep references to them in JavaScript.

The announcement blog post explains it as if the "Detached Elements" tab was able to detect cases (1) and (2), but, in fact, it only detects case (2).

I think both cases are valid. It would be useful for this tool to support both. Ideally with some button to filter the kind you're interested in.

Reproduction

Run the following code as index.html:

<script type="module">
  const cache = [];

  const detachedSection = document.createElement("section");
  cache.push(detachedSection);

  const detachedArticle = document.createElement("article");
  document.body.appendChild(detachedArticle);
  detachedArticle.remove();
  cache.push(detachedArticle);

  console.log("Disconnected elements", cache);
</script>

This script creates two nodes, but only one (article) is reported by the tool, as seen on the screenshot below:

Screenshot 2023-10-13 at 4 18 46 PM


Edge version: 120.0.2158.0 (Official build) Canary (arm64)

@warpech warpech added the bug Something isn't working label Oct 13, 2023
@captainbrosset
Copy link
Contributor

Thanks for reaching out and reporting this. Let me take this with the dev team and get back to you with more details.

@warpech
Copy link
Author

warpech commented Oct 20, 2023

A homebrew solution, that worked in my case, is to monkey-patch document.createElement() and document.createTextNode() to attach the created nodes to a temporary element, and immediately detach them.

const tmpAttachment = document.createElement('div');
tmpAttachment.id = 'tmpAttachment';
document.body.appendChild(tmpAttachment);

{
  const old = document.createElement;
  document.createElement = function (...args) {
    const element = old.apply(document, args);
    tmpAttachment.appendChild(element);
    tmpAttachment.replaceChildren();
    return element;
  };
}
{
  const old = document.createTextNode;
  document.createTextNode = function (...args) {
    const element = old.apply(document, args);
    tmpAttachment.appendChild(element);
    tmpAttachment.replaceChildren();
    return element;
  };
}

@captainbrosset
Copy link
Contributor

Cool! Thanks for sharing. Note that we're still looking into this.

@captainbrosset
Copy link
Contributor

I took a quick look at the implementation and my conclusion is that if it's not by design, it's at least a limitation of the current implementation. I haven't worked on the code base directly for a long time, and my knowledge of the C++ code of Chromium is limited, but my understanding is that the detached elements are retrieved by a process that gets kicked off when a node is removed from the DOM.

In Chromium, here, the function DispatchChildRemovalEvents starts with probe::WillRemoveDOMNode(&child); which is what, I think, starts off the whole process.

@captainbrosset
Copy link
Contributor

@joselea I know you don't work on this right now, but by any chance, would you remember something about this?

@robpaveza
Copy link
Contributor

This is an interesting edge case. The tool was never designed to handle this specific case (for full disclosure, and this might clarify why the tool behaves the way it does, it monitors the removal of nodes from the DOM and keeps those in a weak-ref list; when the API is called, they're re-checked).

That having been said, we're currently working on upstreaming similar functionality to the overall Chromium project; and the way in which this new functionality is built (using the V8 heap walker) is likely to be able to detect this case. I can't make promises about when exactly this will land but I'm estimating that it'll probably be around Chromium/Edge 129.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants