Bug 109578

Summary: Web Inspector: add initial implementation of native memory graph to Timeline
Product: WebKit Reporter: Yury Semikhatsky <yurys>
Component: Web Inspector (Deprecated)Assignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Screenshot with the patch applied
none
Patch apavlov: review+

Description Yury Semikhatsky 2013-02-12 06:11:30 PST 
We'd like to show native memory size on the Timeline much like we already do with DOM counters.
Comment 1 Yury Semikhatsky 2013-02-12 06:16:58 PST 
Created attachment 187853 [details]
Patch
Comment 2 Yury Semikhatsky 2013-02-12 06:17:48 PST 
Created attachment 187855 [details]
Screenshot with the patch applied
Comment 3 Ilya Tikhonovsky 2013-02-12 06:37:50 PST 
Comment on attachment 187853 [details]
Patch

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

looks good to me

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:50
> +    debugger;

please remove this line.

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:54
> +    hsl[2] -= 3;

magic constant

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:294
> +        valueGetter = function(counter) {
> +            return counter.total;
> +        }

why do yo need this function?
Comment 4 Yury Semikhatsky 2013-02-12 07:06:44 PST 
Created attachment 187862 [details]
Patch
Comment 5 Yury Semikhatsky 2013-02-12 07:08:00 PST 
(In reply to comment #3)
> (From update of attachment 187853 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187853&action=review
> 
> looks good to me
> 
> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:50
> > +    debugger;
> 
> please remove this line.
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:54
> > +    hsl[2] -= 3;
> 
> magic constant
> 
Introduced constant.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:294
> > +        valueGetter = function(counter) {
> > +            return counter.total;
> > +        }
> 
> why do yo need this function?
Don't need, removed.
Comment 6 Alexander Pavlov (apavlov) 2013-02-12 07:25:08 PST 
Comment on attachment 187862 [details]
Patch

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

> Source/WebCore/WebCore.vcproj/WebCore.vcproj:77345
> +					RelativePath="..\inspector\front-end\NativeMemoryGraph.js"

Move down below

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:53
> +    var borderLightnessDifference = 3;

const

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:103
> +        function getCounterValue(name, entry)

jsdoc?

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:115
> +                var counterUI = new WebInspector.NativeMemoryCounterUI(this, name, [i*20, 65, 63], getCounterValue.bind(this, name))

i * 20

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:127
> +    _onRecordAdded: function(event)

jsdoc here and below

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:140
> +            for (var n in nativeCounters) {

n -> name ?

> Source/WebCore/inspector/front-end/NativeMemoryGraph.js:199
> +            imageData: imageData };

"}" on the next line

> Source/WebCore/inspector/front-end/WebKit.qrc:116
> +    <file>NativeMemoryGraph.js</file>

Move down below
Comment 7 Yury Semikhatsky 2013-02-12 07:50:55 PST 
(In reply to comment #6)
> (From update of attachment 187862 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187862&action=review
> 
> > Source/WebCore/WebCore.vcproj/WebCore.vcproj:77345
> > +					RelativePath="..\inspector\front-end\NativeMemoryGraph.js"
> 
> Move down below
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:53
> > +    var borderLightnessDifference = 3;
> 
> const
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:103
> > +        function getCounterValue(name, entry)
> 
> jsdoc?
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:115
> > +                var counterUI = new WebInspector.NativeMemoryCounterUI(this, name, [i*20, 65, 63], getCounterValue.bind(this, name))
> 
> i * 20
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:127
> > +    _onRecordAdded: function(event)
> 
> jsdoc here and below
> 
Done.

> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:140
> > +            for (var n in nativeCounters) {
> 
> n -> name ?
> 
> > Source/WebCore/inspector/front-end/NativeMemoryGraph.js:199
> > +            imageData: imageData };
> 
> "}" on the next line
> 
Done.

> > Source/WebCore/inspector/front-end/WebKit.qrc:116
> > +    <file>NativeMemoryGraph.js</file>
> 
> Move down below
Done.
Comment 8 Yury Semikhatsky 2013-02-12 07:58:12 PST 
Committed r142626: <http://trac.webkit.org/changeset/142626>