Bug 96367 – Web Inspector: NMI: remove the dependency of platform from WebCore in NMI code.

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

[Ilya Tikhonovsky]

the idea is to move 'platform' specific instrumentation code
from WebCoreMemoryInstrumentation into the new PlatformMemoryInstrumentation.
the instrumentation for String* also has to be moved.

[Ilya Tikhonovsky]

Created attachment 163310 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 163310 [details]
Patch

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

> Source/WebCore/platform/PlatformMemoryInstrumentation.h:42
> +template<> void MemoryInstrumentationTraits::addInstrumentedObject<KURL>(MemoryInstrumentation*, const KURL* const&, MemoryObjectType, MemoryOwningType);

These should go into Source/WTF

[Yury Semikhatsky]

(In reply to comment #2)
> (From update of attachment 163310 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163310&action=review
> 
> > Source/WebCore/platform/PlatformMemoryInstrumentation.h:42
> > +template<> void MemoryInstrumentationTraits::addInstrumentedObject<KURL>(MemoryInstrumentation*, const KURL* const&, MemoryObjectType, MemoryOwningType);
> 
> These should go into Source/WTF

I mean that Strin* instrumentation should be in Source/WTF.

[Build Bot]

Comment on attachment 163310 [details]
Patch

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

[Build Bot]

Comment on attachment 163310 [details]
Patch

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

[Ilya Tikhonovsky]

Created attachment 163346 [details]
Patch

[Ilya Tikhonovsky]

Created attachment 163347 [details]
rebaselined

[Ilya Tikhonovsky]

Created attachment 163353 [details]
with fix for debug build

[Build Bot]

Comment on attachment 163353 [details]
with fix for debug build

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

[Build Bot]

Comment on attachment 163353 [details]
with fix for debug build

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

[Early Warning System Bot]

Comment on attachment 163353 [details]
with fix for debug build

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

[Gyuyoung Kim]

Comment on attachment 163353 [details]
with fix for debug build

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

[WebKit Review Bot]

Comment on attachment 163353 [details]
with fix for debug build

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

New failing tests:
inspector/profiler/memory-instrumentation-cached-images.html

[Ilya Tikhonovsky]

Created attachment 163851 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 163851 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        the target is to move 'platform' specific instrumentation code

the -> The

> Source/WebCore/ChangeLog:11
> +        Drive by fix: New type DOM.Image was introduced.

I don't see DOM.Image in the change, did you mean Page.Image?

[Ilya Tikhonovsky]

Committed r128451: <http://trac.webkit.org/changeset/128451>

[Csaba Osztrogonác]

(In reply to comment #16)
> Committed r128451: <http://trac.webkit.org/changeset/128451>

It caused a regression. Could you check it, please?
https://bugs.webkit.org/show_bug.cgi?id=96653

[Ilya Tikhonovsky]

(In reply to comment #17)
> (In reply to comment #16)
> > Committed r128451: <http://trac.webkit.org/changeset/128451>
> 
> It caused a regression. Could you check it, please?
> https://bugs.webkit.org/show_bug.cgi?id=96653

Will do.

[Csaba Osztrogonác]

(In reply to comment #13)
> (From update of attachment 163353 [details])
> Attachment 163353 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/13809008
> 
> New failing tests:
> inspector/profiler/memory-instrumentation-cached-images.html

You could have avoided to check in this regression 
if you take notice of the EWS bot ...

[Ilya Tikhonovsky]

(In reply to comment #19)
> (In reply to comment #13)
> > (From update of attachment 163353 [details] [details])
> > Attachment 163353 [details] [details] did not pass chromium-ews (chromium-xvfb):
> > Output: http://queues.webkit.org/results/13809008
> > 
> > New failing tests:
> > inspector/profiler/memory-instrumentation-cached-images.html
> 
> You could have avoided to check in this regression 
> if you take notice of the EWS bot ...

You're right, I'm sorry that did not check this before.