Menu
▾
▴
[HtmlUnit] SVN: [7829] trunk/htmlunit/src
|
From: <asa...@us...> - 2012-12-05 11:59:12
|
Revision: 7829
http://sourceforge.net/p/htmlunit/code/7829
Author: asashour
Date: 2012-12-05 11:59:08 +0000 (Wed, 05 Dec 2012)
Log Message:
-----------
- Fix a potential ConcurrentModificationException, on calling WebClient.getWebWindows().
- Reason: Webclient.windows_, which is created by "Collections.synchronizedList()" must always be inside 'synchronized'
Modified Paths:
--------------
trunk/htmlunit/src/changes/changes.xml
trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
Modified: trunk/htmlunit/src/changes/changes.xml
===================================================================
--- trunk/htmlunit/src/changes/changes.xml 2012-12-05 09:44:07 UTC (rev 7828)
+++ trunk/htmlunit/src/changes/changes.xml 2012-12-05 11:59:08 UTC (rev 7829)
@@ -8,6 +8,9 @@
<body>
<release version="2.12" date="???" description="Bugfixes">
+ <action type="fix" dev="asashour">
+ Fix a potential ConcurrentModificationException, on calling WebClient.getWebWindows().
+ </action>
<action type="fix" dev="asashour" issue="1344">
WebClient.closeAllWindows() to delete all temporary created big files.
</action>
Modified: trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java
===================================================================
--- trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java 2012-12-05 09:44:07 UTC (rev 7828)
+++ trunk/htmlunit/src/main/java/com/gargoylesoftware/htmlunit/WebClient.java 2012-12-05 11:59:08 UTC (rev 7829)
@@ -428,26 +428,28 @@
oldPage.cleanUp();
}
Page newPage = null;
- if (windows_.contains(webWindow) || getBrowserVersion().hasFeature(GENERATED_150)) {
- newPage = pageCreator_.createPage(webResponse, webWindow);
+ synchronized (windows_) {
+ if (windows_.contains(webWindow) || getBrowserVersion().hasFeature(GENERATED_150)) {
+ newPage = pageCreator_.createPage(webResponse, webWindow);
- if (windows_.contains(webWindow)) {
- fireWindowContentChanged(new WebWindowEvent(webWindow, WebWindowEvent.CHANGE, oldPage, newPage));
+ if (windows_.contains(webWindow)) {
+ fireWindowContentChanged(new WebWindowEvent(webWindow, WebWindowEvent.CHANGE, oldPage, newPage));
- // The page being loaded may already have been replaced by another page via JavaScript code.
- if (webWindow.getEnclosedPage() == newPage) {
- newPage.initialize();
- // hack: onload should be fired the same way for all type of pages
- // here is a hack to handle non HTML pages
- if (webWindow instanceof FrameWindow && !(newPage instanceof HtmlPage)) {
- final FrameWindow fw = (FrameWindow) webWindow;
- final BaseFrameElement frame = fw.getFrameElement();
- if (frame.hasEventHandlers("onload")) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Executing onload handler for " + frame);
+ // The page being loaded may already have been replaced by another page via JavaScript code.
+ if (webWindow.getEnclosedPage() == newPage) {
+ newPage.initialize();
+ // hack: onload should be fired the same way for all type of pages
+ // here is a hack to handle non HTML pages
+ if (webWindow instanceof FrameWindow && !(newPage instanceof HtmlPage)) {
+ final FrameWindow fw = (FrameWindow) webWindow;
+ final BaseFrameElement frame = fw.getFrameElement();
+ if (frame.hasEventHandlers("onload")) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Executing onload handler for " + frame);
+ }
+ final Event event = new Event(frame, Event.TYPE_LOAD);
+ ((Node) frame.getScriptObject()).executeEvent(event);
}
- final Event event = new Event(frame, Event.TYPE_LOAD);
- ((Node) frame.getScriptObject()).executeEvent(event);
}
}
}
@@ -1154,9 +1156,11 @@
public WebWindow getWebWindowByName(final String name) throws WebWindowNotFoundException {
WebAssert.notNull("name", name);
- for (final WebWindow webWindow : windows_) {
- if (webWindow.getName().equals(name)) {
- return webWindow;
+ synchronized (windows_) {
+ for (final WebWindow webWindow : windows_) {
+ if (webWindow.getName().equals(name)) {
+ return webWindow;
+ }
}
}
@@ -1207,7 +1211,9 @@
*/
public void registerWebWindow(final WebWindow webWindow) {
WebAssert.notNull("webWindow", webWindow);
- windows_.add(webWindow);
+ synchronized (windows_) {
+ windows_.add(webWindow);
+ }
}
/**
@@ -1219,7 +1225,9 @@
*/
public void deregisterWebWindow(final WebWindow webWindow) {
WebAssert.notNull("webWindow", webWindow);
- windows_.remove(webWindow);
+ synchronized (windows_) {
+ windows_.remove(webWindow);
+ }
fireWindowClosed(new WebWindowEvent(webWindow, WebWindowEvent.CLOSE, webWindow.getEnclosedPage(), null));
}
@@ -1565,7 +1573,7 @@
* @see #getTopLevelWindows()
*/
public List<WebWindow> getWebWindows() {
- return Collections.unmodifiableList(windows_);
+ return Collections.unmodifiableList(new ArrayList<WebWindow>(windows_));
}
/**
@@ -1995,18 +2003,20 @@
public int waitForBackgroundJavaScript(final long timeoutMillis) {
int count = 0;
final long endTime = System.currentTimeMillis() + timeoutMillis;
- for (Iterator<WebWindow> i = windows_.iterator(); i.hasNext();) {
- final WebWindow window;
- try {
- window = i.next();
+ synchronized (windows_) {
+ for (Iterator<WebWindow> i = windows_.iterator(); i.hasNext();) {
+ final WebWindow window;
+ try {
+ window = i.next();
+ }
+ catch (final ConcurrentModificationException e) {
+ i = windows_.iterator();
+ count = 0;
+ continue;
+ }
+ final long newTimeout = endTime - System.currentTimeMillis();
+ count += window.getJobManager().waitForJobs(newTimeout);
}
- catch (final ConcurrentModificationException e) {
- i = windows_.iterator();
- count = 0;
- continue;
- }
- final long newTimeout = endTime - System.currentTimeMillis();
- count += window.getJobManager().waitForJobs(newTimeout);
}
if (count != getAggregateJobCount()) {
final long newTimeout = endTime - System.currentTimeMillis();
@@ -2042,18 +2052,20 @@
public int waitForBackgroundJavaScriptStartingBefore(final long delayMillis) {
int count = 0;
final long endTime = System.currentTimeMillis() + delayMillis;
- for (Iterator<WebWindow> i = windows_.iterator(); i.hasNext();) {
- final WebWindow window;
- try {
- window = i.next();
+ synchronized (windows_) {
+ for (Iterator<WebWindow> i = windows_.iterator(); i.hasNext();) {
+ final WebWindow window;
+ try {
+ window = i.next();
+ }
+ catch (final ConcurrentModificationException e) {
+ i = windows_.iterator();
+ count = 0;
+ continue;
+ }
+ final long newDelay = endTime - System.currentTimeMillis();
+ count += window.getJobManager().waitForJobsStartingBefore(newDelay);
}
- catch (final ConcurrentModificationException e) {
- i = windows_.iterator();
- count = 0;
- continue;
- }
- final long newDelay = endTime - System.currentTimeMillis();
- count += window.getJobManager().waitForJobsStartingBefore(newDelay);
}
if (count != getAggregateJobCount()) {
final long newDelay = endTime - System.currentTimeMillis();
@@ -2088,17 +2100,19 @@
*/
private int getAggregateJobCount() {
int count = 0;
- for (Iterator<WebWindow> i = windows_.iterator(); i.hasNext();) {
- final WebWindow window;
- try {
- window = i.next();
+ synchronized (windows_) {
+ for (Iterator<WebWindow> i = windows_.iterator(); i.hasNext();) {
+ final WebWindow window;
+ try {
+ window = i.next();
+ }
+ catch (final ConcurrentModificationException e) {
+ i = windows_.iterator();
+ count = 0;
+ continue;
+ }
+ count += window.getJobManager().getJobCount();
}
- catch (final ConcurrentModificationException e) {
- i = windows_.iterator();
- count = 0;
- continue;
- }
- count += window.getJobManager().getJobCount();
}
return count;
}
|
