Commit 7a3234c9 authored by mnaganov's avatar mnaganov Committed by Commit bot

[Android WebView] Provide user-initiated provisional load detection

Do not synthesize page loading events on DOM modification, if the provisional
load has been started from the API side.

It appears that a lot of apps tend to use the following scenario:

webView.loadUrl(...);
webView.loadUrl('javascript:...');

Which was triggering page loading events to be emitted. This scenario is
dubious, as no one guarantees that loading will actually finish prior to
executing javascript. But for compatibility reasons we must take it into
account and not emit page loading events for "about:blank", as it seems
that some apps do unexpected things when they receive it.

BUG=458569,469099

Review URL: https://codereview.chromium.org/1024103002

Cr-Commit-Position: refs/heads/master@{#321857}
parent 03b77088
......@@ -2294,7 +2294,7 @@ public class AwContents implements SmartClipProvider,
public boolean getDidAttemptLoad() {
if (mDidAttemptLoad) return mDidAttemptLoad;
mDidAttemptLoad = mWebContentsObserver.hasStartedAnyProvisionalLoad();
mDidAttemptLoad = mWebContentsObserver.hasStartedNonApiProvisionalLoadInMainFrame();
return mDidAttemptLoad;
}
......
......@@ -6,9 +6,11 @@ package org.chromium.android_webview;
import org.chromium.android_webview.AwContents.VisualStateCallback;
import org.chromium.base.ThreadUtils;
import org.chromium.content_public.browser.NavigationEntry;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.browser.WebContentsObserver;
import org.chromium.net.NetError;
import org.chromium.ui.base.PageTransition;
import java.lang.ref.WeakReference;
......@@ -22,7 +24,7 @@ public class AwWebContentsObserver extends WebContentsObserver {
// and should be found and cleaned up.
private final WeakReference<AwContents> mAwContents;
private final AwContentsClient mAwContentsClient;
private boolean mHasStartedAnyProvisionalLoad = false;
private boolean mStartedNonApiProvisionalLoadInMainFrame = false;
public AwWebContentsObserver(
WebContents webContents, AwContents awContents, AwContentsClient awContentsClient) {
......@@ -31,8 +33,8 @@ public class AwWebContentsObserver extends WebContentsObserver {
mAwContentsClient = awContentsClient;
}
boolean hasStartedAnyProvisionalLoad() {
return mHasStartedAnyProvisionalLoad;
boolean hasStartedNonApiProvisionalLoadInMainFrame() {
return mStartedNonApiProvisionalLoadInMainFrame;
}
@Override
......@@ -99,6 +101,14 @@ public class AwWebContentsObserver extends WebContentsObserver {
String validatedUrl,
boolean isErrorPage,
boolean isIframeSrcdoc) {
mHasStartedAnyProvisionalLoad = true;
if (!isMainFrame) return;
AwContents awContents = mAwContents.get();
if (awContents != null) {
NavigationEntry pendingEntry = awContents.getNavigationController().getPendingEntry();
if (pendingEntry != null
&& (pendingEntry.getTransition() & PageTransition.FROM_API) == 0) {
mStartedNonApiProvisionalLoadInMainFrame = true;
}
}
}
}
......@@ -285,7 +285,7 @@ public class ClientOnPageFinishedTest extends AwTestBase {
public void testOnPageFinishedNotCalledOnDomModificationForBlankWebView() throws Throwable {
TestWebServer webServer = TestWebServer.start();
try {
doTestOnPageFinishedNotCalledOnDomMutation(webServer);
doTestOnPageFinishedNotCalledOnDomMutation(webServer, null);
} finally {
webServer.shutdown();
}
......@@ -293,20 +293,14 @@ public class ClientOnPageFinishedTest extends AwTestBase {
@MediumTest
@Feature({"AndroidWebView"})
public void testOnPageFinishedCalledOnDomModificationAfterNonCommittedLoad() throws Throwable {
public void testOnPageFinishedNotCalledOnDomModificationAfterNonCommittedLoadFromApi()
throws Throwable {
enableJavaScriptOnUiThread(mAwContents);
TestWebServer webServer = TestWebServer.start();
try {
final String noContentUrl = webServer.setResponseWithNoContentStatus("/nocontent.html");
TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
mContentsClient.getOnPageFinishedHelper();
final int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
loadUrlAsync(mAwContents, noContentUrl);
// Mutate DOM.
executeJavaScriptAndWaitForResult(mAwContents, mContentsClient,
"document.body.innerHTML='Hello, World!'");
onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
assertEquals("about:blank", onPageFinishedHelper.getUrl());
doTestOnPageFinishedNotCalledOnDomMutation(webServer, noContentUrl);
} finally {
webServer.shutdown();
}
......@@ -320,7 +314,7 @@ public class ClientOnPageFinishedTest extends AwTestBase {
final String testUrl =
webServer.setResponse("/test.html", CommonResources.ABOUT_HTML, null);
loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), testUrl);
doTestOnPageFinishedNotCalledOnDomMutation(webServer);
doTestOnPageFinishedNotCalledOnDomMutation(webServer, null);
} finally {
webServer.shutdown();
}
......@@ -334,13 +328,13 @@ public class ClientOnPageFinishedTest extends AwTestBase {
try {
loadDataSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
CommonResources.ABOUT_HTML, "text/html", false);
doTestOnPageFinishedNotCalledOnDomMutation(webServer);
doTestOnPageFinishedNotCalledOnDomMutation(webServer, null);
} finally {
webServer.shutdown();
}
}
private void doTestOnPageFinishedNotCalledOnDomMutation(TestWebServer webServer)
private void doTestOnPageFinishedNotCalledOnDomMutation(TestWebServer webServer, String syncUrl)
throws Throwable {
enableJavaScriptOnUiThread(mAwContents);
TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
......@@ -353,8 +347,10 @@ public class ClientOnPageFinishedTest extends AwTestBase {
// we load another valid page. Since callbacks arrive sequentially if the next callback
// we get is for the synchronizationUrl we know that DOM mutation did not schedule
// a callback for the iframe.
final String syncUrl = webServer.setResponse("/sync.html", "", null);
loadUrlAsync(mAwContents, syncUrl);
if (syncUrl == null) {
syncUrl = webServer.setResponse("/sync.html", "", null);
loadUrlAsync(mAwContents, syncUrl);
}
onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
assertEquals(syncUrl, onPageFinishedHelper.getUrl());
assertEquals(onPageFinishedCallCount + 1, onPageFinishedHelper.getCallCount());
......
......@@ -50,8 +50,8 @@ public class NavigationPopupTest extends ChromeShellTestBase {
// Exists solely to expose protected methods to this test.
private static class TestNavigationEntry extends NavigationEntry {
public TestNavigationEntry(int index, String url, String virtualUrl, String originalUrl,
String title, Bitmap favicon) {
super(index, url, virtualUrl, originalUrl, title, favicon);
String title, Bitmap favicon, int transition) {
super(index, url, virtualUrl, originalUrl, title, favicon, transition);
}
}
......@@ -62,9 +62,9 @@ public class NavigationPopupTest extends ChromeShellTestBase {
public TestNavigationController() {
mHistory = new TestNavigationHistory();
mHistory.addEntry(new TestNavigationEntry(
1, "about:blank", null, null, "About Blank", null));
1, "about:blank", null, null, "About Blank", null, 0));
mHistory.addEntry(new TestNavigationEntry(
5, UrlUtils.encodeHtmlDataUri("<html>1</html>"), null, null, null, null));
5, UrlUtils.encodeHtmlDataUri("<html>1</html>"), null, null, null, null, 0));
}
@Override
......
......@@ -49,7 +49,8 @@ static base::android::ScopedJavaLocalRef<jobject> CreateJavaNavigationEntry(
j_virtual_url.obj(),
j_original_url.obj(),
j_title.obj(),
j_bitmap.obj());
j_bitmap.obj(),
entry->GetTransitionType());
}
static void AddNavigationEntryToHistory(JNIEnv* env,
......
......@@ -248,8 +248,8 @@ import org.chromium.content_public.browser.NavigationHistory;
@CalledByNative
private static NavigationEntry createNavigationEntry(int index, String url,
String virtualUrl, String originalUrl, String title, Bitmap favicon) {
return new NavigationEntry(index, url, virtualUrl, originalUrl, title, favicon);
String virtualUrl, String originalUrl, String title, Bitmap favicon, int transition) {
return new NavigationEntry(index, url, virtualUrl, originalUrl, title, favicon, transition);
}
private native boolean nativeCanGoBack(long nativeNavigationControllerAndroid);
......
......@@ -17,18 +17,20 @@ public class NavigationEntry {
private final String mVirtualUrl;
private final String mTitle;
private Bitmap mFavicon;
private int mTransition;
/**
* Default constructor.
*/
public NavigationEntry(int index, String url, String virtualUrl, String originalUrl,
String title, Bitmap favicon) {
String title, Bitmap favicon, int transition) {
mIndex = index;
mUrl = url;
mVirtualUrl = virtualUrl;
mOriginalUrl = originalUrl;
mTitle = title;
mFavicon = favicon;
mTransition = transition;
}
/**
......@@ -92,4 +94,8 @@ public class NavigationEntry {
public void updateFavicon(Bitmap favicon) {
mFavicon = favicon;
}
public int getTransition() {
return mTransition;
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment