[cp-patches] javax.swing.Timer rewrite

[Mark Wielaard]

Hi,

As discussed on the main list Timer was not behaving correctly. Joao
tested and suggested to just wrap util.Timer. And this seems to work
well and the code is much simpler now. Thanks Joao.

2005-11-13  Mark Wielaard  <address@hidden>

        As suggested by Joao Victor <address@hidden>:
        * javax/swing/Timer.java (Waker): Removed class.
        (Task): New class.
        (timer): New field.
        (running): Removed field.
        (waker): Likewise.
        (task): New field.
        (isRunning): Check whether task is null.
        (start): Create task and schedule it with timer.
        (stop): Cancel task and clear field.
        (queueEvent): Synchronized on queueLock.

It would be appreciated if someone could try out this patch and see if
it works as expected. Mauve tests all pass and the programs that I
tested seem to work as expected.

BTW. While testing this patch I saw that DefaultCaret is not always
stopping its blinkingTimer when it looses focus. This is mostly harmless
since the caret is hidden in that case anyway. But it makes the Timer do
much more work then needed. I didn't completely understand the
interaction between the focus events, the visibility of the caret and
the textComponent editiable and enabled properties so I haven't fixed
this yet.

Cheers,

Mark

Index: javax/swing/Timer.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/Timer.java,v
retrieving revision 1.24
diff -u -r1.24 Timer.java
--- javax/swing/Timer.java      6 Nov 2005 22:59:25 -0000       1.24
+++ javax/swing/Timer.java      13 Nov 2005 23:12:30 -0000
@@ -46,7 +46,12 @@
 import javax.swing.event.EventListenerList;
 
 /**
- * Fires one or more action events after the specified delay.
+ * Fires one or more action events after the specified delay.  This is
+ * a specialised version of <code>java.util.Timer</code> just for
+ * firing <code>ActionEvent</code>s. All Timers share one (daemon)
+ * Thread (or java.util.Timer). All events are fired from the event
+ * queue.
+ * 
  * @author Ronald Veldema
  * @author Audrius Meskauskas (address@hidden) - bug fixes
  * and documentation comments
@@ -55,64 +60,19 @@
   implements Serializable
 {
   /**
-   * The timer thread
+   * Given to the shared java.util.Timer to (possibly repeatedly) call
+   * queueEvent().
    */
-  private class Waker
-    extends Thread
+  private class Task extends java.util.TimerTask
   {
-    /**
-     * Fires events, pausing for required intervals.
-     */
     public void run()
     {
-      synchronized (queueLock)
-        {
-         try
-           {
-              try
-                {
-                  queueLock.wait(initialDelay);
-                }
-              catch (InterruptedException e)
-                {
-                  // Ignored
-                }
-
-              if (!running)
-                return;
-
-              queueEvent();
-
-              if (repeats)
-                while (running)
-                  {
-                    try
-                      {
-                        queueLock.wait(delay);
-                      }
-                    catch (InterruptedException e)
-                      {
-                         // Ignored
-                      }
-
-                    if (!running)
-                      break;
-
-                    queueEvent();
-
-                    if (logTimers)
-                      System.out.println("javax.swing.Timer -> clocktick");
-
-                    if (!repeats)
-                      break;
-                  }
-            }
-         finally
-           {
-             // The timer is no longer running.
-             running = false;
-           }
-        }
+      if (logTimers)
+       System.out.println("javax.swing.Timer -> queueEvent()");
+      queueEvent();
+
+      if (!repeats)
+       task = null;
     }
   }
 
@@ -134,6 +94,14 @@
     };
 
   /**
+   * The static java.util.Timer daemon which will be used to schedule
+   * all javax.swing.Timer.Task objects. The daemon will always be
+   * running, even if there's no task scheduled in it.
+   */
+  private static java.util.Timer timer = new java.util.Timer("swing.Timer",
+                                                            true);
+
+  /**
    * If <code>true</code>, the timer prints a message to
    * address@hidden System#out} when firing each event.
    */
@@ -155,12 +123,6 @@
   boolean repeats = true;
 
   /**
-   * <code>true</code> if the timer is currently active, firing events
-   * as scheduled. Should only be checked/set with queueLock held.
-   */
-  boolean running;
-
-  /**
    * The delay between subsequent repetetive events.
    */
   int delay;
@@ -178,10 +140,9 @@
   int ticks;
 
   /**
-   * Stores the thread that posts events to the queue at required time
-   * intervals.
+   * The task that calls queueEvent(). When null this Timer is stopped.
    */
-  private Waker waker;
+  private Task task;
 
   /**
    * This object manages a "queue" of virtual actionEvents, maintained as a
@@ -376,7 +337,7 @@
    */
   public boolean isRunning()
   {
-    return running;
+    return task != null;
   }
 
   /**
@@ -414,14 +375,15 @@
    */
   public void start()
   {
-    synchronized (queueLock)
+    Task t = task;
+    if (t == null)
       {
-       if (!running)
-         {
-           running = true;
-           waker = new Waker();
-           waker.start();
-         }
+       t = new Task();
+       if (isRepeats())
+         timer.schedule(t, getInitialDelay(), getDelay());
+       else
+         timer.schedule(t, getInitialDelay());
+       task = t;
       }
   }
 
@@ -430,12 +392,11 @@
    */
   public void stop()
   {
-    synchronized (queueLock)
+    Task t = task;
+    if (t != null)
       {
-       running = false;
-        queue = 0;
-       queueLock.notifyAll();
-       waker = null;
+       t.cancel();
+       task = null;
       }
   }
 
@@ -493,12 +454,14 @@
   /**
   * Post a scheduled event to the event queue.
   * Package-private to avoid an accessor method.
-  * Called with queueLock held and running = true from Waker.run().
   */
   void queueEvent()
   {
-      queue++;
-      if (queue == 1)
-       SwingUtilities.invokeLater(drainer);
+    synchronized(queueLock)
+      {
+       queue++;
+       if (queue == 1)
+         SwingUtilities.invokeLater(drainer);
+      }
   }
 }

signature.asc
Description: This is a digitally signed message part