Bug 109796 – Web Inspector: extract DOM counters graph implementation into its own class

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

[Yury Semikhatsky]

MemoryStatistics.js now implements DOM counters graph and also is inherited by NativeMemoryGraph.js. DOM counters specific should go into its own file leaving only common bits in MemoryStatistics.js.

[Yury Semikhatsky]

Created attachment 188290 [details]
Patch

[Alexander Pavlov (apavlov)]

Comment on attachment 188290 [details]
Patch

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

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:32
> + * @param {WebInspector.TimelinePanel} timelinePanel

@param tags should follow the @constructor and @extends tags

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:48
> +WebInspector.DOMCounterUI = function(memoryCountersPane, title, currentValueLabel, rgb, valueGetter)

JSDoc for the params, please

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:50
> +    var swatchColor = "rgb(" + rgb.join(",") + ")";

BTW, why don't you use WebInspector.Color? It will ease your life.

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:66
> +WebInspector.DOMCountersGraph.Counter = function(time, documentCount, nodeCount, listenerCount)

JSDoc for the params, please

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:159
> +            new WebInspector.DOMCounterUI(this, "Document Count", "Documents: %d", [100,0,0], getDocumentCount),

spaces between array elements everywhere

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:242
> +            ctx.arc(x, y, radius, 0, Math.PI*2, true);

whitespace around '*'

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:289
> +        var currentY = originY + (height - (valueGetter(this._counters[this._minimumIndex])- minValue) * yFactor);

whitespace before '-'

> Source/WebCore/inspector/front-end/DOMCountersGraph.js:294
> +             currentY = originY + (height - (valueGetter(this._counters[i])- minValue) * yFactor);

ditto

[Yury Semikhatsky]

(In reply to comment #2)
> (From update of attachment 188290 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188290&action=review
> 
> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:32
> > + * @param {WebInspector.TimelinePanel} timelinePanel
> 
> @param tags should follow the @constructor and @extends tags
> 
Done.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:48
> > +WebInspector.DOMCounterUI = function(memoryCountersPane, title, currentValueLabel, rgb, valueGetter)
> 
> JSDoc for the params, please
> 
Done.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:50
> > +    var swatchColor = "rgb(" + rgb.join(",") + ")";
> 
> BTW, why don't you use WebInspector.Color? It will ease your life.
> 
I only need to format string value here.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:66
> > +WebInspector.DOMCountersGraph.Counter = function(time, documentCount, nodeCount, listenerCount)
> 
> JSDoc for the params, please
> 
Done.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:159
> > +            new WebInspector.DOMCounterUI(this, "Document Count", "Documents: %d", [100,0,0], getDocumentCount),
> 
> spaces between array elements everywhere
> 
Done.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:242
> > +            ctx.arc(x, y, radius, 0, Math.PI*2, true);
> 
> whitespace around '*'
> 
Done.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:289
> > +        var currentY = originY + (height - (valueGetter(this._counters[this._minimumIndex])- minValue) * yFactor);
> 
> whitespace before '-'
> 
Done.

> > Source/WebCore/inspector/front-end/DOMCountersGraph.js:294
> > +             currentY = originY + (height - (valueGetter(this._counters[i])- minValue) * yFactor);
> 
> ditto
Done.

[Yury Semikhatsky]

Created attachment 188294 [details]
Patch for landing

[Yury Semikhatsky]

Committed r142862: <http://trac.webkit.org/changeset/142862>