From: <rb...@us...> - 2013-08-25 08:04:18
|
Revision: 8446 http://sourceforge.net/p/htmlunit/code/8446 Author: rbri Date: 2013-08-25 08:04:14 +0000 (Sun, 25 Aug 2013) Log Message: ----------- WebClient.getTopLevelWindows() returns a snapshot of the list of open top level windows. This avoids ConcurrentModificationExceptions when using this list by a client. Issue 1534 Modified Paths: -------------- trunk/htmlunit/src/changes/changes.xml trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java Modified: trunk/htmlunit/src/changes/changes.xml =================================================================== --- trunk/htmlunit/src/changes/changes.xml 2013-08-25 07:29:56 UTC (rev 8445) +++ trunk/htmlunit/src/changes/changes.xml 2013-08-25 08:04:14 UTC (rev 8446) @@ -8,6 +8,10 @@ <body> <release version="2.13" date="???" description="Bugfixes"> + <action type="fix" dev="rbri" issue="1534"> + WebClient.getTopLevelWindows() returns a snapshot of the list of open top level windows. + This avoids ConcurrentModificationExceptions when using this list by a client. + </action> <action type="fix" dev="rbri"> In case the server reports an error via http error code, the curretn page content was not replaced with the content of that error page if the error was the result Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java =================================================================== --- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java 2013-08-25 07:29:56 UTC (rev 8445) +++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java 2013-08-25 08:04:14 UTC (rev 8446) @@ -954,7 +954,7 @@ WebAssert.notNull("name", name); for (final WebWindow webWindow : windows_) { - if (webWindow.getName().equals(name)) { + if (name.equals(webWindow.getName())) { return webWindow; } } @@ -1362,6 +1362,8 @@ /** * Returns an immutable list of open web windows (whether they are top level windows or not). + * This is a snapshot; future changes are not reflected by this list. + * * @return an immutable list of open web windows (whether they are top level windows or not) * @see #getWebWindowByName(String) * @see #getTopLevelWindows() @@ -1386,12 +1388,14 @@ /** * Returns an immutable list of open top level windows. + * This is a snapshot; future changes are not reflected by this list. + * * @return an immutable list of open top level windows * @see #getWebWindowByName(String) * @see #getWebWindows() */ public List<TopLevelWindow> getTopLevelWindows() { - return Collections.unmodifiableList(topLevelWindows_); + return Collections.unmodifiableList(new ArrayList<TopLevelWindow>(topLevelWindows_)); } /** @@ -1701,12 +1705,7 @@ } // NB: this implementation is too simple as a new TopLevelWindow may be opened by // some JS script while we are closing the others - final List<TopLevelWindow> topWindows = new ArrayList<TopLevelWindow>(); - for (final WebWindow window : topLevelWindows_) { - if (window instanceof TopLevelWindow) { - topWindows.add((TopLevelWindow) window); - } - } + final List<TopLevelWindow> topWindows = new ArrayList<TopLevelWindow>(topLevelWindows_); for (final TopLevelWindow topWindow : topWindows) { if (topLevelWindows_.contains(topWindow)) { topWindow.close(); Modified: trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java =================================================================== --- trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java 2013-08-25 07:29:56 UTC (rev 8445) +++ trunk/htmlunit/src/test/java/com/gargoylesoftware/htmlunit/WebClientTest.java 2013-08-25 08:04:14 UTC (rev 8446) @@ -82,6 +82,7 @@ * @author Ahmed Ashour * @author Daniel Gredler * @author Sudhan Moghe + * @author Ronald Brill */ @RunWith(BrowserRunner.class) public class WebClientTest extends SimpleWebTestCase { @@ -1627,7 +1628,7 @@ @Test public void testUrlEncoding() throws Exception { final URL url = new URL("http://host/x+y\u00E9/a\u00E9 b?c \u00E9 d"); - final HtmlPage page = loadPage(BrowserVersion.FIREFOX_3_6, "<html></html>", new ArrayList<String>(), url); + final HtmlPage page = loadPage(BrowserVersion.FIREFOX_17, "<html></html>", new ArrayList<String>(), url); final WebRequest wrs = page.getWebResponse().getWebRequest(); assertEquals("http://host/x+y%C3%A9/a%C3%A9%20b?c%20%E9%20d", wrs.getUrl()); } @@ -2158,6 +2159,35 @@ } /** + * Test that the result of getTopLevelWindows() is usable without + * getting a ConcurrentModificationException. + * + * @throws Exception if an error occurs + */ + @Test + public void getTopLevelWindowsJSConcurrency() throws Exception { + final String html = "<html><head><title>Toplevel</title></head>\n<body>\n" + + "<script>\n" + + " setInterval(function() {\n" + + " window.open('');\n" + + " }, 10);\n" + + "</script>\n" + + "</body></html>\n"; + + final WebClient client = getWebClientWithMockWebConnection(); + getMockWebConnection().setResponse(URL_FIRST, html); + + client.getPage(URL_FIRST); + final List<TopLevelWindow> windows = client.getTopLevelWindows(); + for (int i = 0; i < 100; i++) { + for (TopLevelWindow window : windows) { + Thread.sleep(13); + window.getName(); + } + } + } + + /** * Regression test for * <a href="http://sourceforge.net/support/tracker.php?aid=2819046>bug 2819046</a>. * |