[ htmlunit-Patches-1210246 ] More browser-like setTimeout() semantics and setInterval()

Previous Topic Next Topic
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view

[ htmlunit-Patches-1210246 ] More browser-like setTimeout() semantics and setInterval()

Patches item #1210246, was opened at 2005-05-28 03:33
Message generated for change (Comment added) made by mguillem
You can respond by visiting:

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Steven Grimm (koreth)
Assigned to: Nobody/Anonymous (nobody)
Summary: More browser-like setTimeout() semantics and setInterval()

Initial Comment:
I realize there is already a patch to implement
setInterval, but it uses the same model as setTimeout,
which has a big problem: it spawns a thread per
scheduled event.

Here is an implementation that uses one timer thread
per WebClient. Not only is it less resource-intensive,
it also avoids the possibility that two timed events
can fire up concurrently (i.e., it makes timers work
more like those of a real browser.)

The implementation is a new class, ScriptTimerThread
(unit tests included, of course) that is attached to
the WebClient. The setInterval() and setTimeout()
methods just add events to the script timer thread's
work queue, and they're executed in time order.

This also allows implementation of a major new feature
which will greatly help with unit-testing of
timer-heavy JavaScript: you can manipulate the thread
timer's clock so you don't have to wait for events to
happen in real time. This not only allows timer event
code to be tested much more quickly than with the old
model, but also reduces the risk that tests of such
code will fail due to heavy load on the test system.

See the test suite and the comment at the top of
ScriptTimerThread.java for more details.


>Comment By: Marc Guillemot (mguillem)
Date: 2005-10-17 16:20

Logged In: YES

I think that there are some interesting features in your
patch but they need some improvements to be applied.

In your javadoc you say:
"There is one timer thread for each WebClient, which means
multiple scheduled events can never run concurrently. All
major Web browsers provide the same guarantee."

That's wrong. Just try 2 timeouts in Firefox, block
temporily one with an alert: the other one contine to execute.

Please don't forget to run maven htmlunit:checkstyle to be
sure that your code is conform to htmlunit's guide line.


Comment By: Steven Grimm (koreth)
Date: 2005-06-01 11:47

Logged In: YES

Try patch's "-p" option to remove directory prefixes. I
generated the patch using "diff -ru
--unidirectional-new-files" and verified that I could apply
it to a clean copy from CVS using the patch command before I
uploaded it. If you'd prefer I use a different set of diff
options to generate the patch, I'll be happy to.

As for the coding conventions, sorry about that. I thought
I'd done a decent job of mimicing the style of the existing
code, but I guess not. I have never used checkstyle but if I
have time over the next week or two I'll go read up on it
and see about reformatting my code to fit the project's
conventions. I have been using my patched version of
htmlunit for my own unit testing and it has proven useful,
so I'd certainly like to see it out there for others to use
as well.


Comment By: Brad Clarke (yourgod)
Date: 2005-05-31 17:51

Logged In: YES

The concepts expressed sound quite good but the patch itself
has numerous problems making evaluating it very time
consuming at best:

Patch doesn't apply without editing because the path starts
with ../../htmlunit

Method of adding new files in the patch showed errors in
eclipse but managed to figure it out anyway. What was used
to generate this patch?

And most importantly, there are 138 checkstyle/javadoc
warnings, mosting because the coding style is quite
different from our standards.


You can respond by visiting:

This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
HtmlUnit-develop mailing list
[hidden email]