[Webkit-unassigned] [Bug 94987] Add systemTime to DOM events

[bugzilla-daemon at webkit.org]

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





--- Comment #55 from Rafael Brandao <rafael.lobo at openbossa.org>  2012-10-26 17:43:47 PST ---
(From update of attachment 170978)
View in context: https://bugs.webkit.org/attachment.cgi?id=170978&action=review

Informal review, just a few comments.

> Source/WebCore/dom/Event.h:108
> +    double systemTime() const { return m_timeStamp; }

This is a bit confusing... so m_timeStamp is systemTime and m_createTime is timeStamp()? Could we use m_systemTime instead?

> Source/WebCore/dom/Node.cpp:2661
> +    return EventDispatcher::dispatchEvent(this, WheelEventDispatchMediator::create(event, document()->defaultView(), 1000.0 * document()->loader()->timing()->monotonicTimeToZeroBasedDocumentTime(event.timestamp())));

Perhaps you could make an auxiliary function to deal with this timestamp transformation? It's repeated three times already in the same file.

> Source/WebCore/page/EventHandler.cpp:3065
> +    RefPtr<KeyboardEvent> keypress = KeyboardEvent::create(keyPressEvent, m_frame->document()->defaultView(), 1000.0 * m_frame->document()->loader()->timing()->monotonicTimeToZeroBasedDocumentTime(initialKeyEvent.timestamp()));

I still don't like much "m_frame->document()->loader()->timing()->monotonicTimeToZeroBasedDocumentTime". Are we sure that no one in the middle can be null pointer? Perhaps you should add asserts on your auxiliary function (if you plan to do it).

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