[webkit-reviews] review denied: [Bug 25503] Add instrumentation framework to WebCore : [Attachment 38838] Addresses review suggestions and updates to r47896
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Sep 3 13:04:43 PDT 2009
Timothy Hatcher <timothy at hatcher.name> has denied Kelly Norton
<knorton at google.com>'s request for review:
Bug 25503: Add instrumentation framework to WebCore
https://bugs.webkit.org/show_bug.cgi?id=25503
Attachment 38838: Addresses review suggestions and updates to r47896
https://bugs.webkit.org/attachment.cgi?id=38838&action=review
------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
Looks good! Some small nits. Will r+ when you fix these.
> + InspectorTimelineAgent* timelineAgent = inspectorTimelineAgent();
> + if (timelineAgent)
> + timelineAgent->willRecalculateStyle();
You can combine these into 2 lines. Same comment for the rest of the palces
like this.
if (InspectorTimelineAgent* timelineAgent = inspectorTimelineAgent())
timelineAgent->willRecalculateStyle();
> + int i = 0;
> + for (TimelineItems::const_iterator it(m_children.begin()); it !=
m_children.end(); ++it) {
> + ScriptObject childObject = (*it)->convertToScriptObject(frontend);
> + children.set(i++, childObject);
> + }
Just use a normal for loop and a count and not iterators.
> +InspectorTimelineAgent* Page::inspectorTimelineAgent() const
> +{
> + return m_inspectorController->timelineAgent();
> +}
You should inline this in the header.
More information about the webkit-reviews
mailing list