Bug 76545 – [chromium] Web Inspector: highlight DOM nodes from detached DOM trees

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Ilya Tikhonovsky]

often the cause of memory leaks are DOM objects that not connected to a DOMWindow.
I'd like to highlight these objects in the Profiler's heap snapshot for the better discoverability.

[Ilya Tikhonovsky]

Created attachment 122935 [details]
Patch

[Ilya Tikhonovsky]

Created attachment 122936 [details]
screenshot

[Ilya Tikhonovsky]

Created attachment 122937 [details]
another screenshot

[Mikhail Naganov]

Comment on attachment 122935 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122935&action=review

Minor comments, otherwise lgtm.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:723
> +        this._nodeFlags = {canBeQueried: 1, detachedDOMTreeNode: 2};

Please add a comment that this is a set of boolean flags. Otherwise, it's not clear which value should the next flag have: 3 or 4.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1050
> +    _markDetachedDOMTreeNodes: function(nativeRoot)

Is function argument being used?

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1054
> +        for (var iter = this.rootNode.edges; iter.hasNext(); iter.next()) {

I'd suggest to cache this.

> Source/WebCore/inspector/front-end/heapProfiler.css:124
> +    background-color: hsl(0, 100%, 80%);

Why hsl? The rest of the file uses rgb or #. Is this a new trend in DevTools?

[Ilya Tikhonovsky]

Created attachment 122941 [details]
Patch

[Ilya Tikhonovsky]

> > Source/WebCore/inspector/front-end/HeapSnapshot.js:1054
> > +        for (var iter = this.rootNode.edges; iter.hasNext(); iter.next()) {
> 
> I'd suggest to cache this.

It is using only here and this method is calling only once.

> 
> > Source/WebCore/inspector/front-end/heapProfiler.css:124
> > +    background-color: hsl(0, 100%, 80%);
> 
> Why hsl? The rest of the file uses rgb or #. Is this a new trend in DevTools?

All other comments were fixed.

[Ilya Tikhonovsky]

Committed r105292: <http://trac.webkit.org/changeset/105292>