[webkit-reviews] review granted: [Bug 23025] Simplifying DOMTimer lifetime management code : [Attachment 26300] Proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 2 10:58:55 PST 2009


Darin Adler <darin at apple.com> has granted Dmitry Titov <dimich at chromium.org>'s
request for review:
Bug 23025: Simplifying DOMTimer lifetime management code
https://bugs.webkit.org/show_bug.cgi?id=23025

Attachment 26300: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=26300&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -    static_cast<Document*>(context)->removeTimeout(timeoutId);
> +    DOMTimer* timer =
static_cast<Document*>(context)->findTimeout(timeoutId);
> +    delete timer;

I personally would like this to just be a one liner without a local variable.

> -    ActiveDOMObject::contextDestroyed();
> +    ActiveDOMObject::contextDestroyed();  // Clears
m_scriptExecutionContext.

Is adding this comment really an improvement?

Normally we use one space before the "//".

r=me


More information about the webkit-reviews mailing list