[webkit-reviews] review granted: [Bug 53188] requestAnimationFrame callbacks should not fire within a modal dialog : [Attachment 82535] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 15 15:54:42 PST 2011


Alexey Proskuryakov <ap at webkit.org> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 53188: requestAnimationFrame callbacks should not fire within a modal
dialog
https://bugs.webkit.org/show_bug.cgi?id=53188

Attachment 82535: Patch
https://bugs.webkit.org/attachment.cgi?id=82535&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=82535&action=review

> I believe I am using the correct license block.  Which part is incorrect?

Apple Computer, Inc. has been renamed to Apple, Inc. some time ago. We have the
canonical text of the license at
<http://www.webkit.org/coding/bsd-license.html>, although I think that most
Google contributors use a slightly different one. I'd like to know where it
comes from, so that I could point that out in review more specifically than
"please ask your manager".

> I plan to address that issue ASAP (as a separate patch).

Yes, the GC issue is certainly for a separate patch.

> Source/WebCore/dom/Document.cpp:4917
> +    return m_scriptedAnimationController->registerCallback(callback, e);

Missed this one - please don't abbreviate "element". This could actually use a
name that explains what element it is (targetElement? animationElement?
rootElement?)

> Source/WebCore/dom/ScriptedAnimationController.cpp:29
> +#include "config.h"
> +
> +#include "ScriptedAnimationController.h"

This still has an extra newline.

> Source/WebCore/dom/ScriptedAnimationController.cpp:60
> +ScriptedAnimationController::CallbackId
ScriptedAnimationController::registerCallback(PassRefPtr<RequestAnimationFrameC
allback> callback, Element* e)

Please don't abbreviate "element"

> Source/WebCore/dom/ScriptedAnimationController.h:69
> +} // namespace WebCore

This still has the comment. Not a big deal - I think that many people missed
the webkit-dev discussion, and still land new code with these comments.


More information about the webkit-reviews mailing list