[Android WebView] Fire onPageFinished from WebContentsObserver::didStopLoading

android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java

Index: android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
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 53715c67b5f8c6d520ee8ec445e1054c31c8abbc..ba7e1c6efb2a0386072695e6bc58e4e5804a4b2d 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
@@ -26,6 +26,9 @@ public class AwWebContentsObserver extends WebContentsObserver {
     // Whether this webcontents has ever committed any navigation.
     private boolean mCommittedNavigation;
 
+    // Temporarily stores the URL passed the last time to didFinishLoad callback.
+    private String mLastDidFinishLoadUrl;
+
     public AwWebContentsObserver(
             WebContents webContents, AwContents awContents, AwContentsClient awContentsClient) {
         super(webContents);
@@ -33,15 +36,35 @@ public class AwWebContentsObserver extends WebContentsObserver {
         mAwContentsClient = new WeakReference<>(awContentsClient);
     }
 
+    private AwContentsClient getClientIfNeedToFireCallback(String validatedUrl) {
+        AwContentsClient client = mAwContentsClient.get();
+        if (client != null) {
+            String unreachableWebDataUrl = AwContentsStatics.getUnreachableWebDataUrl();
+            if (unreachableWebDataUrl == null || !unreachableWebDataUrl.equals(validatedUrl)) {
+                return client;
+            }
+        }
+        return null;
+    }
+
     @Override
     public void didFinishLoad(long frameId, String validatedUrl, boolean isMainFrame) {
-        AwContentsClient client = mAwContentsClient.get();
-        if (client == null) return;
-        String unreachableWebDataUrl = AwContentsStatics.getUnreachableWebDataUrl();
-        boolean isErrorUrl =
-                unreachableWebDataUrl != null && unreachableWebDataUrl.equals(validatedUrl);
-        if (isMainFrame && !isErrorUrl) {
+        if (isMainFrame && getClientIfNeedToFireCallback(validatedUrl) != null) {
+            AwContents awContents = mAwContents.get();
+            // For loadDataWithBaseURL 'validatedUrl' is 'baseUrl', but we need 'historyUrl',
+            // as this is what will then be passed to 'didStopLoading'.
+            String currentUrl = awContents != null ? awContents.getUrl() : validatedUrl;

[gsennton, 2015/11/13 02:35:39]

Why was this not the case before https://codereview.chromium.org/1381003004
?

[mnaganov (inactive), 2015/11/13 17:41:37]

Because we were only using 'didFinishLoad' before. Now we need to orchestrate
two callbacks, and as it turns out, they receive different urls if the loading
is done via loadDataWithBaseURL, as the comment says.

+            mLastDidFinishLoadUrl = currentUrl != null ? currentUrl : "about:blank";
+        }
+    }
+
+    @Override
+    public void didStopLoading(String validatedUrl) {
+        if (validatedUrl.length() == 0) validatedUrl = "about:blank";

[gsennton, 2015/11/13 02:35:39]

When does this happen?

[mnaganov (inactive), 2015/11/13 17:41:37]

When the client does loadUrl("about:blank"). Discovered via
ClientOnPageFinishedTest#testCalledAfterError.

+        AwContentsClient client = getClientIfNeedToFireCallback(validatedUrl);
+        if (client != null && validatedUrl.equals(mLastDidFinishLoadUrl)) {
             client.getCallbackHelper().postOnPageFinished(validatedUrl);
+            mLastDidFinishLoadUrl = null;
         }
     }