Change in gwt[master]: Removes the static book keeping in the Timer class.

[Goktug Gokdogan]

Goktug Gokdogan has uploaded a new change for review.

https://gwt-review.googlesource.com/2851


Change subject: Removes the static book keeping in the Timer class.
......................................................................

Removes the static book keeping in the Timer class.

This changes removes code that keeps track of Timer statically.
Based on history the code was always there from the beginning but
my guess is, this is no longer needed: New-ish Scheduler API is not
doing the same workaround and also there are other places that are
using Impl.setTimeout directly without the workaround.

Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
---
M user/src/com/google/gwt/user/client/Timer.java
1 file changed, 0 insertions(+), 31 deletions(-)



diff --git a/user/src/com/google/gwt/user/client/Timer.java
b/user/src/com/google/gwt/user/client/Timer.java
index be226b8..f95a5a7 100644
--- a/user/src/com/google/gwt/user/client/Timer.java
+++ b/user/src/com/google/gwt/user/client/Timer.java
@@ -15,10 +15,6 @@
*/
package com.google.gwt.user.client;

-import com.google.gwt.event.logical.shared.CloseEvent;
-import com.google.gwt.event.logical.shared.CloseHandler;
-
-import java.util.ArrayList;

/**
* A simplified, browser-safe timer class. This class serves the same
purpose as
@@ -43,12 +39,6 @@
*/
public abstract class Timer {

- private static ArrayList<Timer> timers = new ArrayList<Timer>();
-
- static {
- hookWindowClosing();
- }
-
private static native void clearInterval(int id) /*-{
@com.google.gwt.core.client.impl.Impl::clearInterval(I)(id);
}-*/;
@@ -68,18 +58,6 @@
$entry(function() {
timer.@com.google.gwt.user.client.Timer::fire(I)(cancelCounter); }),
delay);
}-*/;
-
- private static void hookWindowClosing() {
- // Catch the window closing event.
- Window.addCloseHandler(new CloseHandler<Window>() {
-
- public void onClose(CloseEvent<Window> event) {
- while (timers.size() > 0) {
- timers.get(0).cancel();
- }
- }
- });
- }

private boolean isRepeating;

@@ -101,7 +79,6 @@
} else {
clearTimeout(timerId);
}
- timers.remove(this);
}

/**
@@ -123,7 +100,6 @@
cancel();
isRepeating = false;
timerId = createTimeout(this, delayMillis, cancelCounter);
- timers.add(this);
}

/**
@@ -139,7 +115,6 @@
cancel();
isRepeating = true;
timerId = createInterval(this, periodMillis, cancelCounter);
- timers.add(this);
}

/*
@@ -150,12 +125,6 @@
final void fire(int scheduleCancelCounter) {
if (scheduleCancelCounter != cancelCounter) {
return;
- }
-
- // If this is a one-shot timer, remove it from the timer list. This
will
- // allow it to be garbage collected.
- if (!isRepeating) {
- timers.remove(this);
}

// Run the timer's code.

--
To view, visit https://gwt-review.googlesource.com/2851
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>

[Matthew Dempsky]

Matthew Dempsky has posted comments on this change.

Change subject: Removes the static book keeping in the Timer class.
......................................................................

Patch Set 1: Code-Review+2

--
To view, visit https://gwt-review.googlesource.com/2851
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>

Gerrit-Reviewer: Matthew Dempsky <mdem...@google.com>
Gerrit-HasComments: No

[John A. Tamplin]

John A. Tamplin has posted comments on this change.

Change subject: Removes the static book keeping in the Timer class.
......................................................................

Patch Set 1: Code-Review+1

My recollection is that the close hook was required to avoid memory leaks
in IE. Please make sure removing this doesn't cause a leak in IE8+ (since
we will deprecate IE6-7 before trunk gets released again).

--
To view, visit https://gwt-review.googlesource.com/2851
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>

Gerrit-Reviewer: John A. Tamplin <j...@jaet.org>

[Goktug Gokdogan]

Goktug Gokdogan has posted comments on this change.

Change subject: Removes the static book keeping in the Timer class.
......................................................................

Patch Set 1:

Thanks for the insight John. That was what I was guessing.

The reason I think this is fine without testing is; internally GWT-SDK uses
Scheduler much more than the Timer API. So even if there is a leak, keeping
this code in Timer will not help much.

--
To view, visit https://gwt-review.googlesource.com/2851
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>

Gerrit-Reviewer: Goktug Gokdogan <gok...@google.com>

Gerrit-Reviewer: John A. Tamplin <j...@jaet.org>

Gerrit-Reviewer: Leeroy Jenkins <jen...@gwtproject.org>

[Goktug Gokdogan]

Goktug Gokdogan has posted comments on this change.

Change subject: Removes the static book keeping in the Timer class.
......................................................................

Patch Set 1: Verified+1

--
To view, visit https://gwt-review.googlesource.com/2851
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: comment

Gerrit-Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>

[Goktug Gokdogan]

Goktug Gokdogan has submitted this change and it was merged.

Change subject: Removes the static book keeping in the Timer class.
......................................................................


Removes the static book keeping in the Timer class.

This changes removes code that keeps track of Timer statically.
Based on history the code was always there from the beginning but
my guess is, this is no longer needed: New-ish Scheduler API is not
doing the same workaround and also there are other places that are
using Impl.setTimeout directly without the workaround.

Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
---
M user/src/com/google/gwt/user/client/Timer.java
1 file changed, 0 insertions(+), 31 deletions(-)

Approvals:
John A. Tamplin: Looks good to me, but someone else must approve
Matthew Dempsky: Looks good to me, approved
Leeroy Jenkins: Verified
Goktug Gokdogan: Verified

Gerrit-MessageType: merged

Gerrit-Change-Id: Idf6a9c81a5654bc2eb736a2b4ef81d5cf0517728
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <gok...@google.com>