[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