[Webkit-unassigned] [Bug 30467] Add DOMTimer support to InspectorTimelineAgent.

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41335|review?                     |review-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2009-10-16 23:28:56 PDT ---
(From update of attachment 41335)

> +void InspectorTimelineAgent::didInstallTimer(int timerId, int timeout, bool singleShot)
> +{
> +    m_frontend->addItemToTimeline(TimerFireTimelineItem::newTimerInstallScriptObject(m_frontend, sessionTimeInMilliseconds(), timerId, timeout, singleShot));
> +}
TimerF
It seems like you are pushing script objects right to the frontend, without
keeping them in the controller. That is Ok for 'profile' mode, but you don't
need TimelineItem and TimerFireTimeline classes in that case. I'd suggest that
you do one of the following:

1) Get rid of these classes, leave only static factories for creating
corresponding events. Glue them together on the frontend side so that you did
not need m_currentTimelineItem.

2) Leave TimelineItem and TimerTimelineItem classes;
   Collect them into the timeline events vector within TimelineAgent;
   Create populateScriptObjects and resetScriptObjects methods in TimelineAgent
and call them from within corresponding inspector controller methods;
   Provide a way for TimelineAgent to push events to the frontend on the fly if
latter is available. (Either via passing inspector controller into the timeline
agent or coming up with some specific interface).

My gut is telling me that (2) is better long term, but I can't really explain
why. It seems that (1) is enough at the moment. But (2) allows capturing the
state in the frontend-less mode and all other inspector controller metrics
(including profiles) are supporting this mode.

Drive-by question: Should you introduce some kind of a bit set (or convert
TimelineItemTypes consts into one) so that you could filter events based on
some preferences. I am sure you will be doing filtering, but given the
granularity of the events of your interest, it might make sense to filter on
the inspector controller side.

I am putting r- because I'd like to explore how
InspectorController/InspectorFrontend interaction could be changed in order to
allow you plugging your new agent in a more elegant manner. Please also share
with me your thoughts on (1) vs (2).

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