Bug 96511

Summary: Web Inspector: NMI: migrate core instrumentation code to WTF namespace
Product: WebKit Reporter: Ilya Tikhonovsky <loislo>
Component: Web Inspector (Deprecated)Assignee: Ilya Tikhonovsky <loislo>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, alph, apavlov, benjamin, bweinstein, cmarcelo, eric.carlson, eric, feature-media-reviews, gustavo, haraken, japhet, joepeck, keishi, loislo, macpherson, menard, pfeldman, philn, pmuellr, rik, timothy, webkit.review.bot, xan.lopez, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
with fix for mac build yurys: review+, webkit-ews: commit-queue-

Description Ilya Tikhonovsky 2012-09-12 07:20:46 PDT 
The only problem is a custom trait for KURL that looks weird if instrumentation code is moved to WTF
I'll instrument it via normal reportMemoryUsage in the next patch.
Comment 1 Ilya Tikhonovsky 2012-09-12 07:26:39 PDT 
Created attachment 163623 [details]
Patch
Comment 2 Gyuyoung Kim 2012-09-12 07:50:38 PDT 
Comment on attachment 163623 [details]
Patch

Attachment 163623 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13824725
Comment 3 Early Warning System Bot 2012-09-12 07:50:54 PDT 
Comment on attachment 163623 [details]
Patch

Attachment 163623 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13833405
Comment 4 Early Warning System Bot 2012-09-12 07:53:39 PDT 
Comment on attachment 163623 [details]
Patch

Attachment 163623 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13835229
Comment 5 Ilya Tikhonovsky 2012-09-12 08:16:45 PDT 
Created attachment 163634 [details]
Patch
Comment 6 Early Warning System Bot 2012-09-12 09:00:02 PDT 
Comment on attachment 163634 [details]
Patch

Attachment 163634 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13821837
Comment 7 Ilya Tikhonovsky 2012-09-12 09:22:28 PDT 
(In reply to comment #6)
> (From update of attachment 163634 [details])
> Attachment 163634 [details] did not pass qt-wk2-ews (qt):
> Output: http://queues.webkit.org/results/13821837

Looks like problem with dependency tracking.
Comment 8 Yury Semikhatsky 2012-09-12 11:26:56 PDT 
Comment on attachment 163634 [details]
Patch

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

> Source/WebCore/bindings/js/ScriptProfiler.cpp:41
> +#include <wtf/Forward.h>

You already included it in the header file.

> Source/WebCore/css/MediaQuery.h:-39
> -class MemoryObjectInfo;

How does this class declaration come here now?
Comment 9 Ilya Tikhonovsky 2012-09-12 11:30:07 PDT 
(In reply to comment #8)
> (From update of attachment 163634 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163634&action=review
> 
> > Source/WebCore/bindings/js/ScriptProfiler.cpp:41
> > +#include <wtf/Forward.h>
> 
> You already included it in the header file.
> 
> > Source/WebCore/css/MediaQuery.h:-39
> > -class MemoryObjectInfo;
> 
> How does this class declaration come here now?

It comes from wtf/Forward.h
Comment 10 Build Bot 2012-09-12 13:20:13 PDT 
Comment on attachment 163634 [details]
Patch

Attachment 163634 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13839196
Comment 11 Ilya Tikhonovsky 2012-09-12 22:29:31 PDT 
Created attachment 163776 [details]
Patch
Comment 12 Ilya Tikhonovsky 2012-09-12 23:05:49 PDT 
Created attachment 163784 [details]
with fix for mac build
Comment 13 Early Warning System Bot 2012-09-12 23:55:29 PDT 
Comment on attachment 163784 [details]
with fix for mac build

Attachment 163784 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13824984
Comment 14 Yury Semikhatsky 2012-09-13 00:24:06 PDT 
(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 163634 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=163634&action=review
> > 
> > > Source/WebCore/bindings/js/ScriptProfiler.cpp:41
> > > +#include <wtf/Forward.h>
> > 
> > You already included it in the header file.
> > 
> > > Source/WebCore/css/MediaQuery.h:-39
> > > -class MemoryObjectInfo;
> > 
> > How does this class declaration come here now?
> 
> It comes from wtf/Forward.h

Still don't see it.
Comment 15 Ilya Tikhonovsky 2012-09-13 00:42:02 PDT 
(In reply to comment #14)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (From update of attachment 163634 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=163634&action=review
> > > 
> > > > Source/WebCore/bindings/js/ScriptProfiler.cpp:41
> > > > +#include <wtf/Forward.h>
> > > 
> > > You already included it in the header file.
> > > 
> > > > Source/WebCore/css/MediaQuery.h:-39
> > > > -class MemoryObjectInfo;
> > > 
> > > How does this class declaration come here now?
> > 
> > It comes from wtf/Forward.h
> 
> Still don't see it.

I found that it is including indirectly via 
wtf/WTFString.h -> wtf/StringImpl.h -> wtf/Forward.h
Comment 16 Ilya Tikhonovsky 2012-09-13 00:50:53 PDT 
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > (From update of attachment 163634 [details] [details] [details] [details])
> > > > View in context: https://bugs.webkit.org/attachment.cgi?id=163634&action=review
> > > > 
> > > > > Source/WebCore/bindings/js/ScriptProfiler.cpp:41
> > > > > +#include <wtf/Forward.h>
> > > > 
> > > > You already included it in the header file.
> > > > 
> > > > > Source/WebCore/css/MediaQuery.h:-39
> > > > > -class MemoryObjectInfo;
> > > > 
> > > > How does this class declaration come here now?
> > > 
> > > It comes from wtf/Forward.h
> > 
> > Still don't see it.
> 
> I found that it is including indirectly via 
> wtf/WTFString.h -> wtf/StringImpl.h -> wtf/Forward.h

I think I can remove this include from StringImpl.h
Looks like it is using only for "template <typename T> class StringBuffer;"
But this change will affect toщ much cpp files so I'd like to do this in a separate patch.
Comment 17 Ilya Tikhonovsky 2012-09-13 01:11:33 PDT 
Committed r128418: <http://trac.webkit.org/changeset/128418>