Bug 99958 – Web Inspector: do not double count memory of objects with multiple ancestors

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

[Yury Semikhatsky]

If there is an instrumented ancestor class which is not first in the parents' list and we have a pointer to the ancestor and another one to the descendant the ancestor's memory will be counted twice. To avoid this we should use pointer to the ancestor class when check if an object has already been visited.

[Yury Semikhatsky]

Created attachment 169854 [details]
Patch

[Ilya Tikhonovsky]

Comment on attachment 169854 [details]
Patch

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

> Source/WTF/wtf/MemoryInstrumentation.h:144
> +        memoryObjectInfo->reportObjectInfo<T>(0, 0, sizeof(T));

we can report actual pointer here and use it latter.

> Source/WTF/wtf/MemoryInstrumentation.h:268
> +    if (pointer) {

unnecessary line

> Source/WTF/wtf/MemoryInstrumentation.h:272
> +    } else
> +        pointer = m_pointer;

ditto

[Yury Semikhatsky]

Created attachment 169861 [details]
Patch

[Ilya Tikhonovsky]

Comment on attachment 169861 [details]
Patch

lgtm

[Yury Semikhatsky]

(In reply to comment #2)
> (From update of attachment 169854 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169854&action=review
> 
> > Source/WTF/wtf/MemoryInstrumentation.h:144
> > +        memoryObjectInfo->reportObjectInfo<T>(0, 0, sizeof(T));
> 
> we can report actual pointer here and use it latter.
> 
Goof point. Done!

> > Source/WTF/wtf/MemoryInstrumentation.h:268
> > +    if (pointer) {
> 
> unnecessary line
> 
Done.

> > Source/WTF/wtf/MemoryInstrumentation.h:272
> > +    } else
> > +        pointer = m_pointer;
> 
> ditto
Done.

[WebKit Review Bot]

Comment on attachment 169861 [details]
Patch

Clearing flags on attachment: 169861

Committed r132059: <http://trac.webkit.org/changeset/132059>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

This patch appears to have broken test_wtf API tests:
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/4844/steps/API%20tests/logs/stdio

[ RUN      ] MemoryInstrumentationTest.instrumentedWithMultipleAncestors
1   0x59156f
2   0x2b2d921bf4c0
3   0x7fff776dcfa0

[Yury Semikhatsky]

(In reply to comment #8)
> This patch appears to have broken test_wtf API tests:
> http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/4844/steps/API%20tests/logs/stdio
> 
> [ RUN      ] MemoryInstrumentationTest.instrumentedWithMultipleAncestors
> 1   0x59156f
> 2   0x2b2d921bf4c0
> 3   0x7fff776dcfa0

Fix landed in http://trac.webkit.org/changeset/132065