Cherry-pick "[Android WebView] Provide user-initiated provisional load detection"
Original CL:
https://crrev.com/7a3234c9a19a38726a4a984ff12e2b406423c1d3
Original description:
[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
Note that this patch also contains bits from this commit required for changes
in AwWebContentsObserver to work:
https://crrev.com/5668d12d29ab659ad01dc0e0ffdf24f2eb5f187a
Bug: 19729876
Change-Id: I12c9610a13a382d725cc9a14fcb9879c92f1f461
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index b7b30b1..1a04448 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -833,7 +833,7 @@
if (mWebContentsObserver != null) {
mWebContentsObserver.detachFromWebContents();
}
- mWebContentsObserver = new AwWebContentsObserver(mWebContents, mContentsClient);
+ mWebContentsObserver = new AwWebContentsObserver(mWebContents, this, mContentsClient);
}
/**
@@ -2012,7 +2012,7 @@
public boolean getDidAttemptLoad() {
if (mDidAttemptLoad) return mDidAttemptLoad;
- mDidAttemptLoad = mWebContentsObserver.hasStartedAnyProvisionalLoad();
+ mDidAttemptLoad = mWebContentsObserver.hasStartedNonApiProvisionalLoadInMainFrame();
return mDidAttemptLoad;
}
diff --git a/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java b/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
index cd45ead..bf01280 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
@@ -5,23 +5,30 @@
package org.chromium.android_webview;
import org.chromium.content.browser.WebContentsObserver;
+import org.chromium.content_public.browser.NavigationEntry;
import org.chromium.content_public.browser.WebContents;
import org.chromium.net.NetError;
+import org.chromium.ui.base.PageTransition;
+
+import java.lang.ref.WeakReference;
/**
* Routes notifications from WebContents to AwContentsClient and other listeners.
*/
public class AwWebContentsObserver extends WebContentsObserver {
+ private final WeakReference<AwContents> mAwContents;
private final AwContentsClient mAwContentsClient;
- private boolean mHasStartedAnyProvisionalLoad = false;
+ private boolean mStartedNonApiProvisionalLoadInMainFrame = false;
- public AwWebContentsObserver(WebContents webContents, AwContentsClient awContentsClient) {
+ public AwWebContentsObserver(
+ WebContents webContents, AwContents awContents, AwContentsClient awContentsClient) {
super(webContents);
+ mAwContents = new WeakReference<AwContents>(awContents);
mAwContentsClient = awContentsClient;
}
- boolean hasStartedAnyProvisionalLoad() {
- return mHasStartedAnyProvisionalLoad;
+ boolean hasStartedNonApiProvisionalLoadInMainFrame() {
+ return mStartedNonApiProvisionalLoadInMainFrame;
}
@Override
@@ -81,6 +88,14 @@
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;
+ }
+ }
}
}
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwWebContentsObserverTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwWebContentsObserverTest.java
index fc4cc68..bb8e2ae 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwWebContentsObserverTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwWebContentsObserverTest.java
@@ -34,7 +34,8 @@
@Override
public void run() {
mWebContentsObserver = new AwWebContentsObserver(
- mTestContainerView.getContentViewCore().getWebContents(), mContentsClient);
+ mTestContainerView.getContentViewCore().getWebContents(),
+ mTestContainerView.getAwContents(), mContentsClient);
}
});
}
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
index b657d9d..e9307a3 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
@@ -282,7 +282,7 @@
public void testOnPageFinishedNotCalledOnDomModificationForBlankWebView() throws Throwable {
TestWebServer webServer = TestWebServer.start();
try {
- doTestOnPageFinishedNotCalledOnDomMutation(webServer);
+ doTestOnPageFinishedNotCalledOnDomMutation(webServer, null);
} finally {
webServer.shutdown();
}
@@ -290,20 +290,14 @@
@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();
}
@@ -317,7 +311,7 @@
final String testUrl =
webServer.setResponse("/test.html", CommonResources.ABOUT_HTML, null);
loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), testUrl);
- doTestOnPageFinishedNotCalledOnDomMutation(webServer);
+ doTestOnPageFinishedNotCalledOnDomMutation(webServer, null);
} finally {
webServer.shutdown();
}
@@ -331,13 +325,13 @@
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 =
@@ -350,8 +344,10 @@
// 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());
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
index 2dd9faa..ce16bf9 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/NavigationPopupTest.java
@@ -50,8 +50,8 @@
// 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 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
diff --git a/content/browser/frame_host/navigation_controller_android.cc b/content/browser/frame_host/navigation_controller_android.cc
index 52ef78e..cdc7cad 100644
--- a/content/browser/frame_host/navigation_controller_android.cc
+++ b/content/browser/frame_host/navigation_controller_android.cc
@@ -49,7 +49,8 @@
j_virtual_url.obj(),
j_original_url.obj(),
j_title.obj(),
- j_bitmap.obj());
+ j_bitmap.obj(),
+ entry->GetTransitionType());
}
static void AddNavigationEntryToHistory(JNIEnv* env,
diff --git a/content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java b/content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java
index d039c8e..2b26d82 100644
--- a/content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java
+++ b/content/public/android/java/src/org/chromium/content/browser/framehost/NavigationControllerImpl.java
@@ -233,8 +233,8 @@
@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);
diff --git a/content/public/android/java/src/org/chromium/content_public/browser/NavigationEntry.java b/content/public/android/java/src/org/chromium/content_public/browser/NavigationEntry.java
index b0295bf..b818a93 100644
--- a/content/public/android/java/src/org/chromium/content_public/browser/NavigationEntry.java
+++ b/content/public/android/java/src/org/chromium/content_public/browser/NavigationEntry.java
@@ -17,18 +17,20 @@
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 void updateFavicon(Bitmap favicon) {
mFavicon = favicon;
}
+
+ public int getTransition() {
+ return mTransition;
+ }
}
