OpenJDK / openjfx / 9-dev / rt
changeset 9471:08a57f273c76
8140501: WebView crashes when loading content in a locationlistener
Reviewed-by: arajkumar, azvegint, kcr
author | ghb |
---|---|
date | Tue, 22 Dec 2015 13:39:46 +0300 |
parents | bd97f8ca31fc |
children | ee36b4050d29 |
files | modules/web/src/main/java/com/sun/webkit/WebPage.java modules/web/src/main/native/Source/WebCore/loader/ResourceLoader.cpp modules/web/src/main/native/Source/WebCore/mapfile-macosx modules/web/src/main/native/Source/WebCore/mapfile-vers modules/web/src/main/native/Source/WebCore/platform/java/WebPage.cpp modules/web/src/test/java/javafx/scene/web/LoadTest.java |
diffstat | 6 files changed, 107 insertions(+), 5 deletions(-) [+] |
line wrap: on
line diff
--- a/modules/web/src/main/java/com/sun/webkit/WebPage.java Tue Dec 22 13:35:02 2015 +0300 +++ b/modules/web/src/main/java/com/sun/webkit/WebPage.java Tue Dec 22 13:39:46 2015 +0300 @@ -1184,8 +1184,16 @@ if (!frames.contains(frameID)) { return; } - twkOpen(frameID, url); - + if (twkIsLoading(frameID)) { + Invoker.getInvoker().postOnEventThread(() -> { + // Postpone new load request while webkit is + // about to commit the DocumentLoader from + // provisional state to committed state + twkOpen(frameID, url); + }); + } else { + twkOpen(frameID, url); + } } finally { unlockPage(); } @@ -1206,8 +1214,16 @@ return; } // TODO: handle contentType - twkLoad(frameID, text, contentType); - + if (twkIsLoading(frameID)) { + // Postpone loading new content while webkit is + // about to commit the DocumentLoader from + // provisional state to committed state + Invoker.getInvoker().postOnEventThread(() -> { + twkLoad(frameID, text, contentType); + }); + } else { + twkLoad(frameID, text, contentType); + } } finally { unlockPage(); } @@ -2438,6 +2454,7 @@ private native void twkOpen(long pFrame, String url); private native void twkLoad(long pFrame, String text, String contentType); + private native boolean twkIsLoading(long pFrame); private native void twkStop(long pFrame); private native void twkStopAll(long pPage); // sync private native void twkRefresh(long pFrame);
--- a/modules/web/src/main/native/Source/WebCore/loader/ResourceLoader.cpp Tue Dec 22 13:35:02 2015 +0300 +++ b/modules/web/src/main/native/Source/WebCore/loader/ResourceLoader.cpp Tue Dec 22 13:39:46 2015 +0300 @@ -282,7 +282,7 @@ m_request = request; - if (!redirectResponse.isNull() && !m_documentLoader->isCommitted()) + if (!redirectResponse.isNull() && (m_documentLoader && !m_documentLoader->isCommitted())) frameLoader()->client().dispatchDidReceiveServerRedirectForProvisionalLoad(); }
--- a/modules/web/src/main/native/Source/WebCore/mapfile-macosx Tue Dec 22 13:35:02 2015 +0300 +++ b/modules/web/src/main/native/Source/WebCore/mapfile-macosx Tue Dec 22 13:39:46 2015 +0300 @@ -1882,6 +1882,7 @@ _Java_com_sun_webkit_WebPage_twkIsEditable _Java_com_sun_webkit_WebPage_twkIsJavaScriptEnabled _Java_com_sun_webkit_WebPage_twkLoad + _Java_com_sun_webkit_WebPage_twkIsLoading _Java_com_sun_webkit_WebPage_twkOpen _Java_com_sun_webkit_WebPage_twkPostPaint _Java_com_sun_webkit_WebPage_twkPrePaint
--- a/modules/web/src/main/native/Source/WebCore/mapfile-vers Tue Dec 22 13:35:02 2015 +0300 +++ b/modules/web/src/main/native/Source/WebCore/mapfile-vers Tue Dec 22 13:39:46 2015 +0300 @@ -1887,6 +1887,7 @@ Java_com_sun_webkit_WebPage_twkIsJavaScriptEnabled; Java_com_sun_webkit_WebPage_twkLoad; Java_com_sun_webkit_WebPage_twkOpen; + Java_com_sun_webkit_WebPage_twkIsLoading; Java_com_sun_webkit_WebPage_twkPostPaint; Java_com_sun_webkit_WebPage_twkPrePaint; Java_com_sun_webkit_WebPage_twkPrint;
--- a/modules/web/src/main/native/Source/WebCore/platform/java/WebPage.cpp Tue Dec 22 13:35:02 2015 +0300 +++ b/modules/web/src/main/native/Source/WebCore/platform/java/WebPage.cpp Tue Dec 22 13:39:46 2015 +0300 @@ -1076,6 +1076,14 @@ env->ReleaseStringUTFChars(text, stringChars); } +JNIEXPORT jboolean JNICALL Java_com_sun_webkit_WebPage_twkIsLoading + (JNIEnv* env, jobject self, jlong pFrame) +{ + Frame* frame = static_cast<Frame*>(jlong_to_ptr(pFrame)); + + return bool_to_jbool(frame && frame->loader().isLoading()); +} + JNIEXPORT void JNICALL Java_com_sun_webkit_WebPage_twkStop (JNIEnv* env, jobject self, jlong pFrame) {
--- a/modules/web/src/test/java/javafx/scene/web/LoadTest.java Tue Dec 22 13:35:02 2015 +0300 +++ b/modules/web/src/test/java/javafx/scene/web/LoadTest.java Tue Dec 22 13:39:46 2015 +0300 @@ -25,6 +25,8 @@ package javafx.scene.web; +import static javafx.concurrent.Worker.State.READY; +import static javafx.concurrent.Worker.State.RUNNING; import static javafx.concurrent.Worker.State.FAILED; import static javafx.concurrent.Worker.State.SUCCEEDED; import static org.junit.Assert.assertEquals; @@ -33,6 +35,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; import javafx.concurrent.Worker.State; import javafx.event.EventHandler; import org.junit.Test; @@ -185,4 +188,76 @@ assertEquals("Unexpected location", "", webEngine.getLocation()); assertNotNull("Document is null", webEngine.getDocument()); } + + /** + * @test + * @bug 8140501 + * summary loadContent on location changed + */ + @Test public void loadContentOnLocationChange() throws Exception { + final CountDownLatch latch = new CountDownLatch(2); + + submit(() -> { + WebEngine webEngine = new WebEngine(); + webEngine.locationProperty().addListener((observable, oldValue, newValue) -> { + // NOTE: blank url == about:blank + // loading a empty or null url to WebKit + // will be treated as blank url + // ref : https://html.spec.whatwg.org + if (newValue.equalsIgnoreCase("about:blank")) { + webEngine.loadContent(""); + assertTrue("loadContent in READY State", webEngine.getLoadWorker().getState() == READY); + } + }); + + webEngine.getLoadWorker().stateProperty().addListener(((observable, oldValue, newValue) -> { + if (newValue == SUCCEEDED) { + latch.countDown(); + } + })); + + webEngine.load(""); + assertTrue("load task completed successfully", webEngine.getLoadWorker().getState() == SUCCEEDED); + }); + try { + latch.await(); + } catch (InterruptedException ex) { + throw new AssertionError(ex); + } + } + + /** + * @test + * @bug 8140501 + * summary load url on location changed + */ + @Test public void loadUrlOnLocationChange() throws Exception { + // Cancelling loadContent is synchronous, + // there wont be 2 SUCCEEDED event + final CountDownLatch latch = new CountDownLatch(1); + + submit(() -> { + WebEngine webEngine = new WebEngine(); + webEngine.locationProperty().addListener((observable, oldValue, newValue) -> { + if (newValue.equalsIgnoreCase("")) { + webEngine.load(""); + assertTrue("Load in READY State", webEngine.getLoadWorker().getState() == READY); + } + }); + + webEngine.getLoadWorker().stateProperty().addListener(((observable, oldValue, newValue) -> { + if (newValue == SUCCEEDED) { + latch.countDown(); + } + })); + + webEngine.loadContent(""); + assertTrue("loadContent task running", webEngine.getLoadWorker().getState() == RUNNING); + }); + try { + latch.await(); + } catch (InterruptedException ex) { + throw new AssertionError(ex); + } + } }