Bug 100866 – Web Inspector: NMI instrument ShadowRoot. It gives us ~400k on plus.google.com

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

[Ilya Tikhonovsky]

EOM

[Ilya Tikhonovsky]

Created attachment 171678 [details]
Patch

[Early Warning System Bot]

Comment on attachment 171678 [details]
Patch

Attachment 171678 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14684182

[Early Warning System Bot]

Comment on attachment 171678 [details]
Patch

Attachment 171678 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14687204

[Build Bot]

Comment on attachment 171678 [details]
Patch

Attachment 171678 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14677209

[Build Bot]

Comment on attachment 171678 [details]
Patch

Attachment 171678 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14686207

[EFL EWS Bot]

Comment on attachment 171678 [details]
Patch

Attachment 171678 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14687216

[Yury Semikhatsky]

Comment on attachment 171678 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        I put instrumentation code for ElementRareData to Element.cpp and NodeRareData to Node.cpp

We don't usually put implementation of one class into .cpp of another so could you put the instrumentation into appropriate .cpp files?

> Source/WebCore/dom/DocumentOrderedMap.cpp:167
> +    MemoryClassInfo info(memoryObjectInfo, this);

Is WebCoreMemoryTypes::DOM missing?

[Ilya Tikhonovsky]

Created attachment 171994 [details]
Patch

[Ilya Tikhonovsky]

(In reply to comment #7)
> (From update of attachment 171678 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171678&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        I put instrumentation code for ElementRareData to Element.cpp and NodeRareData to Node.cpp
> 
> We don't usually put implementation of one class into .cpp of another so could you put the instrumentation into appropriate .cpp files?

done

> 
> > Source/WebCore/dom/DocumentOrderedMap.cpp:167
> > +    MemoryClassInfo info(memoryObjectInfo, this);
> 
> Is WebCoreMemoryTypes::DOM missing?

This map looks like a common container.

[Yury Semikhatsky]

Comment on attachment 171994 [details]
Patch

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

> Source/WebCore/dom/Element.cpp:82
> +#include <wtf/MemoryInstrumentation.h>

No need to include it as it is already included in WebCoreMemoryInstrumentation.

[Yury Semikhatsky]

(In reply to comment #9)
> (In reply to comment #7)
> > 
> > > Source/WebCore/dom/DocumentOrderedMap.cpp:167
> > > +    MemoryClassInfo info(memoryObjectInfo, this);
> > 
> > Is WebCoreMemoryTypes::DOM missing?
> 
> This map looks like a common container.

It should belong to the DOM domain by default which can be overriden when it is not so.

[Build Bot]

Comment on attachment 171994 [details]
Patch

Attachment 171994 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14684750

[Ilya Tikhonovsky]

Created attachment 172004 [details]
for mac bot

[Ilya Tikhonovsky]

Committed r133275: <http://trac.webkit.org/changeset/133275>