Bug 22469 – Rename DOMWindowTimer into DOMTimer and move it into separate file

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Dmitry Titov]

This is a preparation step for adding timers to Workers. The plan is to eventually move the setTimeout and friends from JSDOMWindowBase to ScriptExecutionContext, make timers thread-aware and at the end to have timers in Workers same way as they are on window object.

The root bug is https://bugs.webkit.org/show_bug.cgi?id=22328
Per suggestion from ap@, I'm doing it in small steps. Here is the first step - to move DOMWindowTimer into its own file and rename it DOMTimer.

Subsequently, I'll make DOMTimer to be thread-safe and be able to work on a Worker thread.

[Dmitry Titov]

Created attachment 25454 [details]
proposed patch

[Sam Weinig]

Moving the code seems fine, but I think it should stay in bindings/js as the code is still very JS specific.  If we want to abstract DOMTimers and the setTimeout/setInterval methods enough that they can be used with other bindings (ObjC/COM), then we should move the class then.

[Darin Adler]

Comment on attachment 25454 [details]
proposed patch

> +				<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

[Dmitry Titov]

Created attachment 25460 [details]
patch with changes after review

Made changes recommended by Darin and moved new files to bindings/js per Sam's suggestion.

[Dmitry Titov]

Created attachment 25487 [details]
More changes.

removed tabs from ChangeLog, seems good.

[Eric Seidel (no email)]

Comment on attachment 25487 [details]
More changes.

Still looks good.

[Alexey Proskuryakov]

Committed revision 38780.