Bug 96405 – Web Inspector: NMI move String* instrumentation to wtf

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

[Ilya Tikhonovsky]

It is hard to upstream instrumentation traits for string classes.
It is much simple to instrument string classes directly.
Also this instrumentation is solving the problem with SubStrings.

[Ilya Tikhonovsky]

Created attachment 163398 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 163398 [details]
Patch

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

> Source/WTF/wtf/text/StringImpl.h:723
> +        // count size used by internal buffer but skip strings that were constructed from literals.

count -> Count

> Source/WTF/wtf/text/StringImpl.h:731
> +        typename MemoryObjectInfo::ClassInfo info(memoryObjectInfo, this, 0, selfSize);

You should be consistent in using either constant MemoryObjectType::Unknown or 0 everywhere.

> Source/WTF/wtf/text/StringImpl.h:735
> +        else if (m_hashAndFlags & s_hashFlagHas16BitShadow) // Substrings never have it's own shadow.

typo: Substring never has its own shadow.

[Ilya Tikhonovsky]

Created attachment 163546 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 163546 [details]
Patch

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

> Source/WTF/wtf/text/StringImpl.h:735
> +        else if (m_hashAndFlags & s_hashFlagHas16BitShadow) // Substring never has it's own shadow.

it's -> its

[Ilya Tikhonovsky]

(In reply to comment #2)
> (From update of attachment 163398 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163398&action=review
> 
> > Source/WTF/wtf/text/StringImpl.h:723
> > +        // count size used by internal buffer but skip strings that were constructed from literals.
> 
> count -> Count

Done

> 
> > Source/WTF/wtf/text/StringImpl.h:731
> > +        typename MemoryObjectInfo::ClassInfo info(memoryObjectInfo, this, 0, selfSize);
> 
> You should be consistent in using either constant MemoryObjectType::Unknown or 0 everywhere.

Ok I think we can use 0 everywhere otherwise I need to have a separate header file for generic memory types and cpp file for the value.

> 
> > Source/WTF/wtf/text/StringImpl.h:735
> > +        else if (m_hashAndFlags & s_hashFlagHas16BitShadow) // Substrings never have it's own shadow.
> 
> typo: Substring never has its own shadow.

Done.

[Ilya Tikhonovsky]

Committed r128279: <http://trac.webkit.org/changeset/128279>

[WebKit Review Bot]

Re-opened since this is blocked by 96487

[WebKit Review Bot]

Comment on attachment 163546 [details]
Patch

Attachment 163546 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13811919

New failing tests:
MemoryInstrumentationTest.visitFirstMemberInNonVirtualClass
MemoryInstrumentationTest.dataRef
MemoryInstrumentationTest.sizeOf
inspector/profiler/memory-instrumentation-cached-images.html
MemoryInstrumentationTest.ptrVsRef
MemoryInstrumentationTest.refPtr
MemoryInstrumentationTest.ownerTypePropagation
MemoryInstrumentationTest.ownPtr
MemoryInstrumentationTest.ownPtrNotInstrumented
MemoryInstrumentationTest.visitStrings

[Ilya Tikhonovsky]

Committed r128300: <http://trac.webkit.org/changeset/128300>