[webkit-reviews] review denied: [Bug 67986] Web Inspector: requestAnimationFrame callbacks don't show up in the timeline panel : [Attachment 107165] [patch] second iteration. Fire Animation Callback was renamed to Animation Frame Event in Timeline UI
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 13 06:07:20 PDT 2011
Pavel Feldman <pfeldman at chromium.org> has denied Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 67986: Web Inspector: requestAnimationFrame callbacks don't show up in the
timeline panel
https://bugs.webkit.org/show_bug.cgi?id=67986
Attachment 107165: [patch] second iteration. Fire Animation Callback was
renamed to Animation Frame Event in Timeline UI
https://bugs.webkit.org/attachment.cgi?id=107165&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=107165&action=review
Could you please add a test to the new type of the events?
> Source/WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:88
> + if (InspectorInstrumentation::hasFrontends()) {
This copy/paste seems a bit too large.
> Source/WebCore/dom/ScriptedAnimationController.cpp:131
> +
InspectorInstrumentation::didFireAnimationFrameEvent(m_document);
You should surround only handleEvent with these, no need to include reset of
the code.
> Source/WebCore/inspector/front-end/TimelinePanel.js:413
> + this._registeredAnimationCallbackRecords[record.data.callbackId]
= record;
callbackId -> id
> Source/WebCore/inspector/front-end/TimelinePanel.js:1015
> + } else if (record.type === recordTypes.FireAnimationFrameEvent ||
record.type === recordTypes.CancelAnimationFrameCallback) {
I don't think we should set original call site to the cancel animation event
(to be consistent with timer).
More information about the webkit-reviews
mailing list