Bug 99958

Summary:

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

Product:

WebKit

Reporter:

Yury Semikhatsky <yurys>

Component:

Web Inspector (Deprecated)

Assignee:

Yury Semikhatsky <yurys>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, benjamin, cdumez, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Yury Semikhatsky

Reported 2012-10-21 23:27:24 PDT

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.

Attachments
Patch (6.75 KB, patch) 2012-10-22 02:00 PDT, Yury Semikhatsky	no flags	Details Formatted Diff Diff
Patch (7.03 KB, patch) 2012-10-22 02:37 PDT, Yury Semikhatsky	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Yury Semikhatsky

Comment 1 2012-10-22 02:00:09 PDT

Created attachment 169854 [details] Patch

Ilya Tikhonovsky

Comment 2 2012-10-22 02:19:26 PDT

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

Comment 3 2012-10-22 02:37:55 PDT

Created attachment 169861 [details] Patch

Ilya Tikhonovsky

Comment 4 2012-10-22 02:39:26 PDT

Comment on attachment 169861 [details] Patch lgtm

Yury Semikhatsky

Comment 5 2012-10-22 02:39:57 PDT

(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 6 2012-10-22 03:29:12 PDT

Comment on attachment 169861 [details] Patch Clearing flags on attachment: 169861 Committed r132059: <http://trac.webkit.org/changeset/132059>

WebKit Review Bot

Comment 7 2012-10-22 03:29:15 PDT

All reviewed patches have been landed. Closing bug.

Chris Dumez

Comment 8 2012-10-22 04:40:47 PDT

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

Comment 9 2012-10-22 05:16:15 PDT

(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

Note You need to log in before you can comment on or make changes to this bug.