Bug 96367 - Web Inspector: NMI: remove the dependency of platform from WebCore in NMI code.
Summary: Web Inspector: NMI: remove the dependency of platform from WebCore in NMI code.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ilya Tikhonovsky
URL:
Keywords:
Depends on: 96405 96653
Blocks:
  Show dependency tree / graph
 
Reported: 2012-09-11 02:23 PDT by Ilya Tikhonovsky
Modified: 2012-09-14 00:27 PDT (History)
26 users (show)

See Also:

Attachments
Patch (38.92 KB, patch)
2012-09-11 02:26 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
Patch (82.18 KB, patch)
2012-09-11 06:18 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
rebaselined (82.10 KB, patch)
2012-09-11 06:26 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
with fix for debug build (82.84 KB, patch)
2012-09-11 06:46 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
Patch (29.15 KB, patch)
2012-09-13 06:05 PDT, Ilya Tikhonovsky 		yurys: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2012-09-11 02:23:39 PDT 
the idea is to move 'platform' specific instrumentation code
from WebCoreMemoryInstrumentation into the new PlatformMemoryInstrumentation.
the instrumentation for String* also has to be moved.
Comment 1 Ilya Tikhonovsky 2012-09-11 02:26:22 PDT 
Created attachment 163310 [details]
Patch
Comment 2 Yury Semikhatsky 2012-09-11 02:37:45 PDT 
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
Comment 3 Yury Semikhatsky 2012-09-11 02:38:47 PDT 
(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.
Comment 4 Build Bot 2012-09-11 02:57:36 PDT 
Comment on attachment 163310 [details]
Patch

Attachment 163310 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13811479
Comment 5 Build Bot 2012-09-11 03:11:41 PDT 
Comment on attachment 163310 [details]
Patch

Attachment 163310 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13826160
Comment 6 Ilya Tikhonovsky 2012-09-11 06:18:04 PDT 
Created attachment 163346 [details]
Patch
Comment 7 Ilya Tikhonovsky 2012-09-11 06:26:28 PDT 
Created attachment 163347 [details]
rebaselined
Comment 8 Ilya Tikhonovsky 2012-09-11 06:46:51 PDT 
Created attachment 163353 [details]
with fix for debug build
Comment 9 Build Bot 2012-09-11 07:25:15 PDT 
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
Comment 10 Build Bot 2012-09-11 07:28:55 PDT 
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
Comment 11 Early Warning System Bot 2012-09-11 07:52:11 PDT 
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
Comment 12 Gyuyoung Kim 2012-09-11 07:59:05 PDT 
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
Comment 13 WebKit Review Bot 2012-09-11 08:12:49 PDT 
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
Comment 14 Ilya Tikhonovsky 2012-09-13 06:05:03 PDT 
Created attachment 163851 [details]
Patch
Comment 15 Yury Semikhatsky 2012-09-13 06:20:19 PDT 
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?
Comment 16 Ilya Tikhonovsky 2012-09-13 06:22:24 PDT 
Committed r128451: <http://trac.webkit.org/changeset/128451>
Comment 17 Csaba Osztrogonác 2012-09-13 07:50:13 PDT 
(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
Comment 18 Ilya Tikhonovsky 2012-09-13 07:51:00 PDT 
(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.
Comment 19 Csaba Osztrogonác 2012-09-13 07:52:35 PDT 
(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 ...
Comment 20 Ilya Tikhonovsky 2012-09-14 00:27:09 PDT 
(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.