[Webkit-unassigned] [Bug 25503] Add instrumentation framework to WebCore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=25503


Timothy Hatcher <timothy at hatcher.name> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #38382|review?(timothy at hatcher.nam |review-
               Flag|e)                          |




--- Comment #9 from Timothy Hatcher <timothy at hatcher.name>  2009-08-27 09:26:27 PDT ---
(From update of attachment 38382)

> +InspectorTimelineAgent* Document::inspectorTimelineAgent() const
> +{
> +    return page() ? page()->inspectorTimelineAgent() : 0;
> +}

Make these getters inline functions and it will be better. It would make the
callers simplier to keep them.

> +    if (inspectorTimelineAgent())
> +        inspectorTimelineAgent()->willRecalculateStyle();

This should store the timeline agent into a local so the work isn't done twice
when it is enabled. Same for the other callers.

> +void InspectorTimelineAgent::willDispatchDOMEvent(const Event& event)
> +{
> +    TimelineItem* newItem = newTimelineItem(DOMDispatchTimelineItemType);
> +    newItem->addDOMEventProperties(m_frontend, event);
> +}
> +
> +void InspectorTimelineAgent::didDispatchDOMEvent()
> +{
> +    didCompleteCurrentRecord();
> +}

I worry about didCompleteCurrentRecord() ending the wrong record. As long as
all the recorded things are synchronous it is fine. We should ASSERT here that
the current TimelineItem is the right type (DOMDispatchTimelineItemType in this
case). And in all the other places didCompleteCurrentRecord() is called.

> +void InspectorTimelineAgent::TimelineItem::addItemProperties(double endTime)
> +{
> +    m_scriptObject.set("time", m_startTime);
> +    m_scriptObject.set("type", static_cast<int>(m_itemType));
> +    m_scriptObject.set("duration", endTime - m_startTime);
> +    if (m_childObjects.length())
> +        m_scriptObject.set("children", m_childObjects);
> +}
> +
> +void InspectorTimelineAgent::TimelineItem::addDOMEventProperties(InspectorFrontend* frontend, const Event& event)
> +{
> +    ScriptObject data = frontend->newScriptObject();
> +    data.set("type", event.type().string());
> +    m_scriptObject.set("data", data);
> +}

These should be normal data members, and more like ConsoleMessage when the
ScriptObjects are created. Like what Pavel was saying on IRC.

> +        // Must be kept in sync with Timeline.js

This should say TimelineAgent.js.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.