Bug 97461 - Web Inspector: NMI: move visited and countObjectSize methods implementation into separate class
Summary: Web Inspector: NMI: move visited and countObjectSize methods implementation i...
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:
Blocks:
 
Reported: 2012-09-24 09:37 PDT by Ilya Tikhonovsky
Modified: 2012-09-27 06:45 PDT (History)
15 users (show)

See Also:

Attachments
Patch (33.09 KB, patch)
2012-09-24 09:41 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
Patch (33.14 KB, patch)
2012-09-24 09:57 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
rebaselined (41.96 KB, patch)
2012-09-27 01:45 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
unnecessary inspectorData argument was removed (41.91 KB, patch)
2012-09-27 01:50 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
mac build fix (41.85 KB, patch)
2012-09-27 02:36 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
comments addressed (42.87 KB, patch)
2012-09-27 06:25 PDT, Ilya Tikhonovsky 		yurys: review+
Details | Formatted Diff | Diff
Show Obsolete (5) 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-24 09:37:58 PDT 
these methods and the data collected by them need to be used in the instrumentation code for other components.
As example when we are visiting bitmaps we need to visit platform specific objects.
These objects will be instrumented with help of component's own instrumentation code 
but we have to keep the single set of visited objects and the map of counters.
Comment 1 Ilya Tikhonovsky 2012-09-24 09:41:48 PDT 
Created attachment 165399 [details]
Patch
Comment 2 Ilya Tikhonovsky 2012-09-24 09:57:03 PDT 
Created attachment 165405 [details]
Patch
Comment 3 Ilya Tikhonovsky 2012-09-27 01:45:50 PDT 
Created attachment 165951 [details]
rebaselined
Comment 4 Ilya Tikhonovsky 2012-09-27 01:50:29 PDT 
Created attachment 165952 [details]
unnecessary inspectorData argument was removed
Comment 5 Build Bot 2012-09-27 02:28:13 PDT 
Comment on attachment 165952 [details]
unnecessary inspectorData argument was removed

Attachment 165952 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14031820
Comment 6 Ilya Tikhonovsky 2012-09-27 02:36:15 PDT 
Created attachment 165958 [details]
mac build fix
Comment 7 Yury Semikhatsky 2012-09-27 05:53:23 PDT 
Comment on attachment 165958 [details]
mac build fix

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

> Source/WTF/wtf/MemoryInstrumentation.h:114
> +    MemoryInstrumentationClient* m_client;

Please move the field declaration below the methods.

> Source/WTF/wtf/MemoryInstrumentation.h:121
>      virtual void checkCountedObject(const void*) = 0;

This method should also delegate to the client.

> Source/WebCore/inspector/InspectorMemoryAgent.cpp:106
> +    MemoryInstrumentationClientImpl* m_client;

style: fields should go after the methods below.
Comment 8 Ilya Tikhonovsky 2012-09-27 06:25:51 PDT 
Created attachment 165988 [details]
comments addressed
Comment 9 Yury Semikhatsky 2012-09-27 06:42:19 PDT 
Comment on attachment 165988 [details]
comments addressed

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

> Source/WTF/wtf/MemoryInstrumentation.h:96
> +    MemoryInstrumentation(MemoryInstrumentationClient* client) : m_client(client) { }

should be marked explicit

> Source/WebCore/inspector/MemoryInstrumentationImpl.cpp:67
> +void MemoryInstrumentationImpl::processDeferredInstrumentedPointers()

Why did this move?
Comment 10 Ilya Tikhonovsky 2012-09-27 06:45:46 PDT 
Committed r129762: <http://trac.webkit.org/changeset/129762>