Skip to content

Commit 801e826

Browse files
rbrirbri
committed
next try on postponedActions
1 parent 18be75e commit 801e826

File tree

4 files changed

+144
-17
lines changed

4 files changed

+144
-17
lines changed

‎src/main/java/org/htmlunit/html/ScriptElementSupport.java‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.htmlunit.WebWindow;
2828
import org.htmlunit.html.HtmlPage.JavaScriptLoadResult;
2929
import org.htmlunit.javascript.AbstractJavaScriptEngine;
30+
import org.htmlunit.javascript.AbstractJavaScriptEngine.PostponedActionsBlocker;
3031
import org.htmlunit.javascript.PostponedAction;
3132
import org.htmlunit.javascript.host.Window;
3233
import org.htmlunit.javascript.host.dom.Document;
@@ -74,7 +75,8 @@ public static void onAllChildrenAddedToPage(final ScriptElement script, final bo
7475
LOG.debug("Script node added: " + element.asXml());
7576
}
7677

77-
final WebClient webClient = element.getPage().getWebClient();
78+
final SgmlPage page = element.getPage();
79+
final WebClient webClient = page.getWebClient();
7880
if (!webClient.isJavaScriptEngineEnabled()) {
7981
LOG.debug("Script found but not executed because javascript engine is disabled");
8082
return;
@@ -86,7 +88,7 @@ public static void onAllChildrenAddedToPage(final ScriptElement script, final bo
8688
return;
8789
}
8890

89-
final WebWindow webWindow = element.getPage().getEnclosingWindow();
91+
final WebWindow webWindow = page.getEnclosingWindow();
9092
if (webWindow != null) {
9193
final StringBuilder description = new StringBuilder()
9294
.append("Execution of ")
@@ -96,7 +98,7 @@ public static void onAllChildrenAddedToPage(final ScriptElement script, final bo
9698
description.append(" (").append(srcAttrib).append(')');
9799
}
98100

99-
final PostponedAction action = new PostponedAction(element.getPage(), description.toString()) {
101+
final PostponedAction action = new PostponedAction(page, description.toString()) {
100102
@Override
101103
public void execute() {
102104
// see HTMLDocument.setExecutingDynamicExternalPosponed(boolean)
@@ -105,7 +107,7 @@ public void execute() {
105107
if (window != null) {
106108
jsDoc = (HTMLDocument) window.getDocument();
107109
jsDoc.setExecutingDynamicExternalPosponed(element.getStartLineNumber() == -1
108-
&& ATTRIBUTE_NOT_DEFINED != srcAttrib);
110+
&& !hasNoSrcAttrib);
109111
}
110112
try {
111113
executeScriptIfNeeded(script, false, false);
@@ -119,18 +121,43 @@ public void execute() {
119121
};
120122

121123
final AbstractJavaScriptEngine<?> engine = webClient.getJavaScriptEngine();
122-
if (element.hasAttribute("async") && !engine.isScriptRunning()) {
123-
final HtmlPage owningPage = element.getHtmlPageOrNull();
124-
owningPage.addAfterLoadAction(action);
125-
}
126-
else if (element.hasAttribute("async")
127-
|| postponed && StringUtils.isBlank(element.getTextContent())) {
128-
engine.addPostponedAction(action);
124+
125+
final PostponedActionsBlocker blocker = engine.blockPostponedActions(page);
126+
try {
127+
if (element.hasAttribute("async") && !engine.isScriptRunning()) {
128+
final HtmlPage owningPage = element.getHtmlPageOrNull();
129+
owningPage.addAfterLoadAction(action);
130+
}
131+
else if (element.hasAttribute("async")) {
132+
engine.addPostponedAction(action);
133+
}
134+
else if (!hasNoSrcAttrib) {
135+
engine.addPostponedAction(action);
136+
}
137+
else if (postponed && !hasNoSrcAttrib) {
138+
engine.addPostponedAction(action);
139+
}
140+
else if (postponed && engine.postponedActionPending()) {
141+
engine.addPostponedAction(action);
142+
}
143+
else {
144+
try {
145+
action.execute();
146+
// no need to process postponed actions here, they are part
147+
// of the script execution
148+
return;
149+
}
150+
catch (final RuntimeException e) {
151+
throw e;
152+
}
153+
catch (final Exception e) {
154+
throw new RuntimeException(e);
155+
}
156+
}
129157
}
130-
else {
158+
finally {
131159
try {
132-
action.execute();
133-
engine.processPostponedActions();
160+
blocker.release();
134161
}
135162
catch (final RuntimeException e) {
136163
throw e;

‎src/main/java/org/htmlunit/javascript/AbstractJavaScriptEngine.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ public interface AbstractJavaScriptEngine<SCRIPT> {
5252
*/
5353
void addPostponedAction(PostponedAction action);
5454

55+
/**
56+
* @return true it postponed actions available
57+
*/
58+
boolean postponedActionPending();
59+
5560
/**
5661
* <span style="color:red">INTERNAL API - SUBJECT TO CHANGE AT ANY TIME - USE AT YOUR OWN RISK.</span><br>
5762
* Process postponed actions, if any.

‎src/main/java/org/htmlunit/javascript/JavaScriptEngine.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -994,8 +994,7 @@ private void doProcessPostponedActions() {
994994
}
995995

996996
/**
997-
* Adds an action that should be executed first when the script currently being executed has finished.
998-
* @param action the action
997+
* {@inheritDoc}
999998
*/
1000999
@Override
10011000
public void addPostponedAction(final PostponedAction action) {
@@ -1011,6 +1010,19 @@ public void addPostponedAction(final PostponedAction action) {
10111010
actions.add(action);
10121011
}
10131012

1013+
/**
1014+
* {@inheritDoc}
1015+
*/
1016+
@Override
1017+
public boolean postponedActionPending() {
1018+
if (shutdownPending_) {
1019+
return false;
1020+
}
1021+
1022+
final List<PostponedAction> actions = postponedActions_.get();
1023+
return actions != null && actions.size() > 0;
1024+
}
1025+
10141026
/**
10151027
* Handles an exception that occurred during execution of JavaScript code.
10161028
* @param scriptException the exception
@@ -1077,6 +1089,12 @@ public PostponedActionsBlocker blockPostponedActions(final Page page) {
10771089
return postponedActionsBlocker_;
10781090
}
10791091

1092+
if (postponedActionsBlocker_.owningPage_ != page) {
1093+
postponedActionsBlocker_.release();
1094+
postponedActionsBlocker_ = new RootPostponedActionsBlocker(this, page);
1095+
return postponedActionsBlocker_;
1096+
}
1097+
10801098
return new ChildPostponedActionsBlocker(postponedActionsBlocker_);
10811099
}
10821100

‎src/test/java/org/htmlunit/javascript/host/html/HTMLDocumentWrite2Test.java‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,7 @@ public void writeScriptInManyTimes() throws Exception {
896896
+ " document.write('<script>log(\"foo1\");</' + 'script>');\n"
897897

898898
+ " log('1');\n"
899-
+ " document.write('<script src=\"scriptA.js\">;</' + 'script>');\n"
899+
+ " document.write('<script src=\"scriptA.js\"></' + 'script>');\n"
900900
+ " log('2');\n"
901901

902902
+ " document.write('<script src=\"scriptB.js\">');\n"
@@ -920,6 +920,83 @@ public void writeScriptInManyTimes() throws Exception {
920920
loadPageVerifyTitle2(html);
921921
}
922922

923+
/**
924+
* @throws Exception if the test fails
925+
*/
926+
@Test
927+
@Alerts({"0", "foo1", "1", "foo2", "2", "3", "4", "A", "foo3"})
928+
public void writeScriptPostponed() throws Exception {
929+
final String html = "<html>\n"
930+
+ "<head>\n"
931+
+ "<script>\n"
932+
+ LOG_TITLE_FUNCTION
933+
+ " log('0');\n"
934+
+ "</script>\n"
935+
936+
+ "<script>log(\"foo1\");</script>'\n"
937+
938+
+ "<script>\n"
939+
+ " log('1');\n"
940+
+ " document.write('<script>log(\"foo2\");</' + 'script>');\n"
941+
+ " log('2');\n"
942+
+ " document.write('<script src=\"scriptA.js\"></' + 'script>');\n"
943+
+ " log('3');\n"
944+
+ " document.write('<script>log(\"foo3\");</' + 'script>');\n"
945+
+ " log('4');\n"
946+
+ "</script>\n"
947+
948+
+ "</head>\n"
949+
+ "<body>\n"
950+
+ "</body></html>";
951+
952+
final URL scriptUrlA = new URL(URL_FIRST, "scriptA.js");
953+
final URL scriptUrlB = new URL(URL_FIRST, "scriptB.js");
954+
955+
getMockWebConnection().setDefaultResponse(html);
956+
getMockWebConnection().setResponse(scriptUrlA, "log('A');\n", MimeType.TEXT_JAVASCRIPT);
957+
958+
loadPageVerifyTitle2(html);
959+
}
960+
961+
/**
962+
* @throws Exception if the test fails
963+
*/
964+
@Test
965+
@Alerts({"0", "A", "1", "foo2", "2", "3", "4", "B", "foo3"})
966+
public void writeScriptPostponedBeforeWrite() throws Exception {
967+
final String html = "<html>\n"
968+
+ "<head>\n"
969+
+ "<script>\n"
970+
+ LOG_TITLE_FUNCTION
971+
+ " log('0');\n"
972+
+ "</script>\n"
973+
974+
+ "<script src='scriptA.js'></script>'\n"
975+
976+
+ "<script>\n"
977+
+ " log('1');\n"
978+
+ " document.write('<script>log(\"foo2\");</' + 'script>');\n"
979+
+ " log('2');\n"
980+
+ " document.write('<script src=\"scriptB.js\"></' + 'script>');\n"
981+
+ " log('3');\n"
982+
+ " document.write('<script>log(\"foo3\");</' + 'script>');\n"
983+
+ " log('4');\n"
984+
+ "</script>\n"
985+
986+
+ "</head>\n"
987+
+ "<body>\n"
988+
+ "</body></html>";
989+
990+
final URL scriptUrlA = new URL(URL_FIRST, "scriptA.js");
991+
final URL scriptUrlB = new URL(URL_FIRST, "scriptB.js");
992+
993+
getMockWebConnection().setDefaultResponse(html);
994+
getMockWebConnection().setResponse(scriptUrlA, "log('A');\n", MimeType.TEXT_JAVASCRIPT);
995+
getMockWebConnection().setResponse(scriptUrlB, "log('B');\n", MimeType.TEXT_JAVASCRIPT);
996+
997+
loadPageVerifyTitle2(html);
998+
}
999+
9231000
/**
9241001
* Test for bug 1613119.
9251002
* @throws Exception if the test fails

0 commit comments

Comments
 (0)