You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2014/04/23 15:21:05 UTC

git commit: WICKET-5570 Rescheduling the same ajax timer behavior causes memory leak in the browser

Repository: wicket
Updated Branches:
  refs/heads/master 813c0964c -> 57d8f0514


WICKET-5570 Rescheduling the same ajax timer behavior causes memory leak in the browser

(cherry picked from commit f6213fbad2c934be8ffdf20a634c85bcc35a36af)


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/57d8f051
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/57d8f051
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/57d8f051

Branch: refs/heads/master
Commit: 57d8f05141e19d8cf3700b4cb730739c46dd7c71
Parents: 813c096
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
Authored: Wed Apr 23 16:08:52 2014 +0300
Committer: Martin Tzvetanov Grigorov <mg...@apache.org>
Committed: Wed Apr 23 16:20:46 2014 +0300

----------------------------------------------------------------------
 Gruntfile.js                                    |  1 +
 .../wicket/ajax/AbstractAjaxTimerBehavior.java  | 25 +--------
 .../wicket/ajax/res/js/wicket-ajax-jquery.js    | 31 +++++++++++
 .../wicket/ajax/AjaxTimerBehaviorTest.java      | 38 ++++++-------
 .../SimpleTestPageExpectedResult-1.html         |  7 +--
 .../SimpleTestPageExpectedResult.html           |  7 +--
 wicket-core/src/test/js/all.html                |  1 +
 wicket-core/src/test/js/timer.js                | 58 ++++++++++++++++++++
 8 files changed, 115 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/Gruntfile.js
----------------------------------------------------------------------
diff --git a/Gruntfile.js b/Gruntfile.js
index c7fdbb0..e852501 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -50,6 +50,7 @@ module.exports = function(grunt) {
 			"./wicket-core/src/test/js/dom.js",
 			"./wicket-core/src/test/js/channels.js",
 			"./wicket-core/src/test/js/event.js",
+			"./wicket-core/src/test/js/timer.js",
 			"./wicket-core/src/test/js/amd.js"
 		],
 		gymTestsJs = [

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java b/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
index 39c3cc6..5ecf56e 100644
--- a/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
+++ b/wicket-core/src/main/java/org/apache/wicket/ajax/AbstractAjaxTimerBehavior.java
@@ -18,9 +18,7 @@ package org.apache.wicket.ajax;
 
 import org.apache.wicket.Component;
 import org.apache.wicket.Page;
-import org.apache.wicket.core.util.string.JavaScriptUtils;
 import org.apache.wicket.markup.head.IHeaderResponse;
-import org.apache.wicket.markup.head.JavaScriptHeaderItem;
 import org.apache.wicket.markup.head.OnLoadHeaderItem;
 import org.apache.wicket.util.time.Duration;
 
@@ -36,7 +34,6 @@ public abstract class AbstractAjaxTimerBehavior extends AbstractDefaultAjaxBehav
 {
 	private static final long serialVersionUID = 1L;
 
-	private static final String WICKET_TIMERS_ID = AbstractAjaxTimerBehavior.class.getSimpleName() + "-timers";
 	/** The update interval */
 	private Duration updateInterval;
 
@@ -88,10 +85,6 @@ public abstract class AbstractAjaxTimerBehavior extends AbstractDefaultAjaxBehav
 	{
 		super.renderHead(component, response);
 
-		response.render(JavaScriptHeaderItem.forScript(
-			"if (typeof(Wicket.TimerHandles) === 'undefined') {Wicket.TimerHandles = {}}",
-			WICKET_TIMERS_ID));
-
 		if (component.getRequestCycle().find(AjaxRequestTarget.class) == null)
 		{
 			// complete page is rendered, so timeout has to be rendered again
@@ -112,22 +105,12 @@ public abstract class AbstractAjaxTimerBehavior extends AbstractDefaultAjaxBehav
 	protected final String getJsTimeoutCall(final Duration updateInterval)
 	{
 		CharSequence js = getCallbackScript();
-		js = JavaScriptUtils.escapeQuotes(js);
 
-		String timeoutHandle = getTimeoutHandle();
-		// this might look strange, but it is necessary for IE not to leak :(
-		return timeoutHandle+" = setTimeout('" + js + "', " +
-			updateInterval.getMilliseconds() + ')';
+		return String.format("Wicket.Timer.set('%s', function(){%s}, %d);",
+				getComponent().getMarkupId(), js, updateInterval.getMilliseconds());
 	}
 
 	/**
-	 * @return the name of the handle that is used to stop any scheduled timer
-	 */
-	private String getTimeoutHandle() {
-		return "Wicket.TimerHandles['"+getComponent().getMarkupId() + "']";
-	}
-	
-	/**
 	 * 
 	 * @see org.apache.wicket.ajax.AbstractDefaultAjaxBehavior#respond(AjaxRequestTarget)
 	 */
@@ -218,9 +201,7 @@ public abstract class AbstractAjaxTimerBehavior extends AbstractDefaultAjaxBehav
 		{
 			hasTimeout = false;
 
-			String timeoutHandle = getTimeoutHandle();
-			headerResponse.render(OnLoadHeaderItem.forScript("clearTimeout(" + timeoutHandle
-				+ "); delete " + timeoutHandle + ";"));
+			headerResponse.render(OnLoadHeaderItem.forScript("Wicket.Timer.clear('" + getComponent().getMarkupId() + "');"));
 		}
 	}
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
----------------------------------------------------------------------
diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
index 9aca979..a0be252 100644
--- a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
+++ b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
@@ -2655,6 +2655,37 @@
 				}
 				Wicket.Focus.refocusLastFocusedComponentAfterResponse = false;
 			}
+		},
+
+		/**
+		 * Manages the functionality needed by AbstractAjaxTimerBehavior and its subclasses
+		 */
+		Timer: {
+			/**
+			 * Schedules a timer
+			 * @param {string} timerId - the identifier for the timer
+			 * @param {function|string} js - the JavaScript to execute after the timeout
+			 * @param {number} delay - the timeout
+			 */
+			'set': function(timerId, js, delay) {
+				if (typeof(Wicket.TimerHandles) === 'undefined') {
+					Wicket.TimerHandles = {};
+				}
+
+				Wicket.Timer.clear(timerId);
+				Wicket.TimerHandles[timerId] = setTimeout(js, delay);
+			},
+
+			/**
+			 * Un-schedules a timer by its id
+			 * @param {string} timerId - the identifier of the timer
+			 */
+			clear: function(timerId) {
+				if (Wicket.TimerHandles && Wicket.TimerHandles[timerId]) {
+					clearTimeout(Wicket.TimerHandles[timerId]);
+					delete Wicket.TimerHandles[timerId];
+				}
+			}
 		}
 	});
 

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/test/java/org/apache/wicket/ajax/AjaxTimerBehaviorTest.java
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/ajax/AjaxTimerBehaviorTest.java b/wicket-core/src/test/java/org/apache/wicket/ajax/AjaxTimerBehaviorTest.java
index 66e7c5c..3d7f599 100644
--- a/wicket-core/src/test/java/org/apache/wicket/ajax/AjaxTimerBehaviorTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/ajax/AjaxTimerBehaviorTest.java
@@ -75,11 +75,11 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 		tester.clickLink(MockPageWithLinkAndComponent.LINK_ID);
 
 		// first render sets timeout
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 
 		tester.executeBehavior(timer);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 	}
 
 
@@ -109,15 +109,15 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 
 		tester.startPage(page);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 
 		tester.clickLink(MockPageWithLinkAndComponent.LINK_ID);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 
 		tester.executeBehavior(timer);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 	}
 
 	/**
@@ -146,16 +146,16 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 
 		tester.startPage(page);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 
 		tester.clickLink(MockPageWithLinkAndComponent.LINK_ID);
 
 		// ajax update does not set timeout
-		assertMatches("setTimeout", 0);
+		assertMatches("Wicket.Timer.set", 0);
 
 		tester.executeBehavior(timer);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 	}
 
 
@@ -184,18 +184,18 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 
 		tester.startPage(page);
 
-		assertMatches("setTimeout", 0);
+		assertMatches("Wicket.Timer.set", 0);
 
 		tester.clickLink(MockPageWithLinkAndComponent.LINK_ID);
 
-		assertMatches("setTimeout", 0);
+		assertMatches("Wicket.Timer.set", 0);
 
 		label.setVisible(true);
 
 		tester.startPage(page);
 
 		// no visible, so timeout is set
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 	}
 
 	/**
@@ -236,12 +236,12 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 
 		tester.startPage(page);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 
 		tester.executeBehavior(timer);
 
-		assertMatches("clearTimeout", 1);
-		assertMatches("setTimeout", 0);
+		assertMatches("Wicket.Timer.clear", 1);
+		assertMatches("Wicket.Timer.set", 0);
 	}
 
 	/**
@@ -321,7 +321,7 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 		// restart the timer
 		tester.clickLink(MockPageWithLinkAndComponent.LINK_ID);
 
-		assertMatches("setTimeout", 1);
+		assertMatches("Wicket.Timer.set", 1);
 		// WICKET-5439 label is not updated automatically
 		assertMatches("wicket:id=\"component\"", 0);
 
@@ -333,10 +333,10 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 	}
 
 	/**
-	 * Validates the reponse, then makes sure the timer injects itself again when called.
+	 * Validates the response, then makes sure the timer injects itself again when called.
 	 * 
-	 * @param timer
-	 * @param wasAjax
+	 * @param string
+	 * @param count
 	 */
 	private void assertMatches(String string, int count)
 	{
@@ -361,4 +361,4 @@ public class AjaxTimerBehaviorTest extends WicketTestCase
 
 		assertEquals(count, found);
 	}
-}
\ No newline at end of file
+}

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult-1.html
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult-1.html b/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult-1.html
index 39333e4..e1b83a8 100644
--- a/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult-1.html
+++ b/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult-1.html
@@ -12,9 +12,4 @@ Wicket.Ajax.DebugWindow.enabled=true;
 Wicket.Ajax.baseUrl="wicket/bookmarkable/org.apache.wicket.ajax.markup.html.componentMap.SimpleTestPage?0-1.IBehaviorListener.0-testPanel-baseSpan-linja1";
 /*]^]^>*/
 </script>
-<script type="text/javascript" id="AbstractAjaxTimerBehavior-timers">
-/*<![CDATA[*/
-if (typeof(Wicket.TimerHandles) === 'undefined') {Wicket.TimerHandles = {}}
-/*]^]^>*/
-</script>
-</head>]]></header-contribution><evaluate encoding="wicket1"><![CDATA[(function(){Wicket.TimerHandles['linja11']^ = setTimeout('Wicket.Ajax.ajax({\"u\":\"./org.apache.wicket.ajax.markup.html.componentMap.SimpleTestPage?0-1.IBehaviorListener.0-testPanel-baseSpan-linja1\",\"c\":\"linja11\"});', 2000)})();]]></evaluate></ajax-response>
\ No newline at end of file
+</head>]]></header-contribution><evaluate><![CDATA[(function(){Wicket.Timer.set('linja11', function(){Wicket.Ajax.ajax({"u":"./org.apache.wicket.ajax.markup.html.componentMap.SimpleTestPage?0-1.IBehaviorListener.0-testPanel-baseSpan-linja1","c":"linja11"});}, 2000);})();]]></evaluate></ajax-response>
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult.html
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult.html b/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult.html
index 34d1f98..ec6225e 100644
--- a/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult.html
+++ b/wicket-core/src/test/java/org/apache/wicket/ajax/markup/html/componentMap/SimpleTestPageExpectedResult.html
@@ -13,17 +13,12 @@ Wicket.Ajax.DebugWindow.enabled=true;
 Wicket.Ajax.baseUrl="wicket/bookmarkable/org.apache.wicket.ajax.markup.html.componentMap.SimpleTestPage?0";
 /*]]>*/
 </script>
-<script type="text/javascript" id="AbstractAjaxTimerBehavior-timers">
-/*<![CDATA[*/
-if (typeof(Wicket.TimerHandles) === 'undefined') {Wicket.TimerHandles = {}}
-/*]]>*/
-</script>
 
   <title>ajax-test</title>
 <script type="text/javascript" >
 /*<![CDATA[*/
 Wicket.Event.add(window, "load", function(event) { 
-Wicket.TimerHandles['linja11'] = setTimeout('Wicket.Ajax.ajax({\"u\":\"./org.apache.wicket.ajax.markup.html.componentMap.SimpleTestPage?0-1.IBehaviorListener.0-testPanel-baseSpan-linja1\",\"c\":\"linja11\"});', 2000);
+Wicket.Timer.set('linja11', function(){Wicket.Ajax.ajax({"u":"./org.apache.wicket.ajax.markup.html.componentMap.SimpleTestPage?0-1.IBehaviorListener.0-testPanel-baseSpan-linja1","c":"linja11"});}, 2000);;
 ;});
 /*]]>*/
 </script>

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/test/js/all.html
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/js/all.html b/wicket-core/src/test/js/all.html
index a6192ce..0b62a6a 100644
--- a/wicket-core/src/test/js/all.html
+++ b/wicket-core/src/test/js/all.html
@@ -22,6 +22,7 @@
 	<script type="text/javascript" src="form.js"></script>
 	<script type="text/javascript" src="head.js"></script>
 	<script type="text/javascript" src="ajax.js"></script>
+	<script type="text/javascript" src="timer.js"></script>
 </head>
 
 <body>

http://git-wip-us.apache.org/repos/asf/wicket/blob/57d8f051/wicket-core/src/test/js/timer.js
----------------------------------------------------------------------
diff --git a/wicket-core/src/test/js/timer.js b/wicket-core/src/test/js/timer.js
new file mode 100644
index 0000000..e018f24
--- /dev/null
+++ b/wicket-core/src/test/js/timer.js
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+/*global ok: true, start: true, test: true, equal: true, deepEqual: true,
+ QUnit: true, module: true, expect: true, stop: true */
+
+jQuery(document).ready(function() {
+	"use strict";
+
+	module('Wicket.Timer');
+
+	test('set', function () {
+		stop();
+		expect(2);
+
+		var timerId = 'timerId',
+			run = function() {
+				ok("The timer is ran!");
+				ok(Wicket.TimerHandles[timerId], "There is a handle to the timeout!");
+				start();
+			};
+
+		Wicket.Timer.set(timerId, run, 1);
+	});
+
+	test('clear', function () {
+		stop();
+		expect(3);
+
+		var timerId = 'timerId',
+			run = function() {
+				ok("The timer is ran!");
+
+				ok(Wicket.TimerHandles[timerId], "There is a handle to the timeout!");
+
+				Wicket.Timer.clear(timerId);
+
+				ok(typeof(Wicket.TimerHandles[timerId]) === 'undefined', "There is NO handle to the timeout!");
+
+				start();
+			};
+
+		Wicket.Timer.set('timerId', run, 1);
+	});
+});