[webkit-reviews] review denied: [Bug 30707] WebInspector: Refactors InspectorTimelineAgent to eliminate TimelineItem classes : [Attachment 41715] WebInspector: Refactors InspectorTimelineAgent to remove TimelineItem classes

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 23 07:37:53 PDT 2009


Pavel Feldman <pfeldman at chromium.org> has denied Eric Ayers
<zundel at google.com>'s request for review:
Bug 30707: WebInspector: Refactors InspectorTimelineAgent to eliminate
TimelineItem classes
https://bugs.webkit.org/show_bug.cgi?id=30707

Attachment 41715: WebInspector: Refactors InspectorTimelineAgent to remove
TimelineItem classes
https://bugs.webkit.org/attachment.cgi?id=41715&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
I see how lack of getters hurts you, sorry about that - we should probably
implement those. It would be great if you made the stack explicit other than
encapuslating it into your new class. It would look really short then, no heap
allocations, brought back assert. What do I miss, why the additional
complexity?

struct TimelineEventEntry {
    public TimelineEvent(object, children, type) : object(object),
children(children), type(type) {}
    ScriptObject object;
    ScriptArray children;
    int type;
}

Vector< TimelineEventEntry > m_eventStack;


InspectorTimelineAgent::willRecalculateStyle()
{
    pushTimelineItem(TimelineItemFactory::createGenericTimelineItem(m_frontend,
currentTimeInMilliseconds()), RecalculateStylesTimelineItemType);
}

InspectorTimelineAgent::didRecalculateStyle()
{
    popTimelineItem(RecalculateStylesTimelineItemType);
}

InspectorTimelineAgent::pushTimelineItem(ScriptObject event, int type)
{
    ScriptArray children = m_frontend->newScriptArray();
    event.set("type", type);
    event.set("children", children);
    m_eventStack.append(TimelineEventEntry(event, children, type));
}

InspectorTimelineAgent::popTimelineItem(int type)
{
    TimelineEventEntry event = m_eventStack.last();
    m_eventStack.removeLast();
    ASSERT(event.type == type);
    event.object.set("endTime", currentTimeInMilliseconds());

    if (m_eventStack.size()) {
	TimelineEventEntry parent = m_eventStack.last();
	parent.children.set(parent.children.length(), event.object);
    } else {
	m_frontend->addItemToTimeline(event.object);
    }
}


More information about the webkit-reviews mailing list