[webkit-reviews] review granted: [Bug 22469] Rename DOMWindowTimer into DOMTimer and move it into separate file : [Attachment 25454] proposed patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dmitry Titov <dimich at chromium.org>'s
request for review:
Bug 22469: Rename DOMWindowTimer into DOMTimer and move it into separate file
https://bugs.webkit.org/show_bug.cgi?id=22469

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +				<FileConfiguration
> +					Name="Release_PGO|Win32"
> +					>
> +					<Tool
> +						Name="VCCLCompilerTool"
> +						WholeProgramOptimization="true"

> +					/>
> +				</FileConfiguration>

I don't think we need this in the vcproj, do we?

> +static int lastUsedTimeoutId = 0;

This can go inside the DOMTimer constructor instead of outside the function. It
would be nice to scope it a little more tightly.

> +#include "ActiveDOMObject.h"

Why are you including this header in DOMTimer.h?

> +class DOMTimer : public TimerBase {
> +public:
> +    // Creates a new timer with the next id and nesting level.
> +    DOMTimer(JSDOMWindowBase* object, ScheduledAction* action);

Normally we'd omit the names "object" and "action" here, because the types make
it clear what the arguments are without names.

> +    // Creates a timer from PausedTimeout, takes timeoutId and nestingLevel
as they were persisted.
> +    DOMTimer(int timeoutId, int nestingLevel, JSDOMWindowBase* object,
ScheduledAction* action);

Same here.

r=me