[Android WebView] Send WebChromeClient.onReceivedTitle when navigatin…

…g back

This is a re-land of https://codereview.chromium.org/1126383005, fixed
not to cause a crash when 'WebView.loadUrl' is called from inside the
onReceivedTitle callback, added a regression test. Also, the document
title is now cached to minimize the amount of callback calls.

Original patch description:

The usual channel for delivering onReceivedTitle (via updating title for
a NavigationEntry) doesn't work in the case of history navigations, because
WebContentsImpl silences no-op updates of entries titles.

Chrome Browser uses another event for updating the title in this case --
via WebContentsDelegate::LoadingStateChanged. This patch enables WebView to
use it as well.

Note that WebChromeClient.onReceivedIcon doesn't need to be re-sent, as
nobody seems to bother checking whether the icon update is a "no-op", so
updates on history navigations just work.

BUG=481570,495234

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

Cr-Commit-Position: refs/heads/master@{#333056}

android_webview/glue/java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java

@@ -630,7 +630,7 @@ public void onReceivedTitle(String title) {
         try {
             TraceEvent.begin("WebViewContentsClientAdapter.onReceivedTitle");
             if (mWebChromeClient != null) {
-                if (TRACE) Log.d(TAG, "onReceivedTitle");
+                if (TRACE) Log.d(TAG, "onReceivedTitle=\"" + title + "\"");
                 mWebChromeClient.onReceivedTitle(mWebView, title);
             }
         } finally {

android_webview/java/src/org/chromium/android_webview/AwContentViewClient.java

@@ -89,7 +89,7 @@ public void onStartContentIntent(Context context, String contentUrl) {
 
     @Override
     public void onUpdateTitle(String title) {
-        mAwContentsClient.onReceivedTitle(title);
+        mAwContentsClient.updateTitle(title, true);
     }
 
     @Override

android_webview/java/src/org/chromium/android_webview/AwContentsClient.java

@@ -13,6 +13,7 @@
 import android.net.http.SslError;
 import android.os.Looper;
 import android.os.Message;
+import android.text.TextUtils;
 import android.view.KeyEvent;
 import android.view.View;
 import android.webkit.ConsoleMessage;
@@ -44,6 +45,12 @@ public abstract class AwContentsClient {
     // Last background color reported from the renderer. Holds the sentinal value INVALID_COLOR
     // if not valid.
     private int mCachedRendererBackgroundColor = INVALID_COLOR;
+    // Holds the last known page title. {@link ContentViewClient#onUpdateTitle} is unreliable,
+    // particularly for navigating backwards and forwards in the history stack. Instead, the last
+    // known document title is kept here, and the clients gets updated whenever the value has
+    // actually changed. Blink also only sends updates when the document title have changed,
+    // so behaviours are consistent.
+    private String mTitle = "";
 
     private static final int INVALID_COLOR = 0;
 
@@ -324,4 +331,10 @@ public abstract SelectActionMode startActionMode(
             View view, ActionHandler actionHandler, boolean floating);
 
     public abstract boolean supportsFloatingActionMode();
+
+    public void updateTitle(String title, boolean forceNotification) {
+        if (!forceNotification && TextUtils.equals(mTitle, title)) return;
+        mTitle = title;
+        mCallbackHelper.postOnReceivedTitle(mTitle);
+    }
 }

android_webview/java/src/org/chromium/android_webview/AwContentsClientCallbackHelper.java

@@ -89,6 +89,7 @@ private static class OnReceivedHttpErrorInfo {
     private static final int MSG_ON_SCALE_CHANGED_SCALED = 7;
     private static final int MSG_ON_RECEIVED_HTTP_ERROR = 8;
     private static final int MSG_ON_PAGE_FINISHED = 9;
+    private static final int MSG_ON_RECEIVED_TITLE = 10;
 
     // Minimum period allowed between consecutive onNewPicture calls, to rate-limit the callbacks.
     private static final long ON_NEW_PICTURE_MIN_PERIOD_MILLIS = 500;
@@ -163,6 +164,11 @@ public void handleMessage(Message msg) {
                     mContentsClient.onPageFinished(url);
                     break;
                 }
+                case MSG_ON_RECEIVED_TITLE: {
+                    final String title = (String) msg.obj;
+                    mContentsClient.onReceivedTitle(title);
+                    break;
+                }
                 default:
                     throw new IllegalStateException(
                             "AwContentsClientCallbackHelper: unhandled message " + msg.what);
@@ -229,4 +235,8 @@ public void postOnReceivedHttpError(AwContentsClient.AwWebResourceRequest reques
     public void postOnPageFinished(String url) {
         mHandler.sendMessage(mHandler.obtainMessage(MSG_ON_PAGE_FINISHED, url));
     }
+
+    public void postOnReceivedTitle(String title) {
+        mHandler.sendMessage(mHandler.obtainMessage(MSG_ON_RECEIVED_TITLE, title));
+    }
 }

android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegate.java

@@ -41,4 +41,9 @@ protected static native void nativeFilesSelectedInChooser(int processId, int ren
     @Override
     @CalledByNative
     public abstract void navigationStateChanged(int flags);
+
+    // Not an override, because WebContentsDelegateAndroid maps this call
+    // into onLoad{Started|Stopped}.
+    @CalledByNative
+    public abstract void loadingStateChanged();
 }

android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java

@@ -271,6 +271,11 @@ public void toggleFullscreenModeForTab(boolean enterFullscreen) {
         }
     }
 
+    @Override
+    public void loadingStateChanged() {
+        mContentsClient.updateTitle(mAwContents.getTitle(), false);
+    }
+
     private static class GetDisplayNameTask extends AsyncTask<Void, Void, String[]> {
         final int mProcessId;
         final int mRenderId;

android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java

@@ -332,4 +332,84 @@ public void testRendererNavigationAndGoBackWithExtraHeaders() throws Throwable {
             webServer.shutdown();
         }
     }
+
+    private static class OnReceivedTitleClient extends TestAwContentsClient {
+        void setOnReceivedTitleCallback(Runnable onReceivedTitleCallback) {
+            mOnReceivedTitleCallback = onReceivedTitleCallback;
+        }
+        @Override
+        public void onReceivedTitle(String title) {
+            super.onReceivedTitle(title);
+            mOnReceivedTitleCallback.run();
+        }
+        private Runnable mOnReceivedTitleCallback;
+    }
+
+    // See crbug.com/494929. Need to make sure that loading a javascript: URL
+    // from inside onReceivedTitle works.
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testLoadUrlFromOnReceivedTitle() throws Throwable {
+        final OnReceivedTitleClient contentsClient = new OnReceivedTitleClient();
+        final AwTestContainerView testContainerView =
+                createAwTestContainerViewOnMainSync(contentsClient);
+        final AwContents awContents = testContainerView.getAwContents();
+        final AwSettings settings = getAwSettingsOnUiThread(awContents);
+        settings.setJavaScriptEnabled(true);
+
+        contentsClient.setOnReceivedTitleCallback(new Runnable() {
+            @Override
+            public void run() {
+                awContents.loadUrl("javascript:testProperty=42;void(0);");
+            }
+        });
+
+        TestWebServer webServer = TestWebServer.start();
+        try {
+            // We need to have a navigation entry, but with an empty title. Note that
+            // trying to load a page with no title makes the received title to be
+            // the URL of the page so instead we use a "204 No Content" response.
+            final String url = webServer.setResponseWithNoContentStatus("/page.html");
+            loadUrlSync(awContents, contentsClient.getOnPageFinishedHelper(), url);
+            TestAwContentsClient.OnReceivedTitleHelper onReceivedTitleHelper =
+                    contentsClient.getOnReceivedTitleHelper();
+            final String pageTitle = "Hello, World!";
+            int onReceivedTitleCallCount = onReceivedTitleHelper.getCallCount();
+            loadUrlAsync(awContents, "javascript:document.title=\"" + pageTitle + "\";void(0);");
+            onReceivedTitleHelper.waitForCallback(onReceivedTitleCallCount);
+            assertEquals(pageTitle, onReceivedTitleHelper.getTitle());
+        } finally {
+            webServer.shutdown();
+        }
+    }
+
+    public void testOnReceivedTitleForUnchangingTitle() throws Throwable {
+        final TestAwContentsClient contentsClient = new TestAwContentsClient();
+        final AwTestContainerView testContainerView =
+                createAwTestContainerViewOnMainSync(contentsClient);
+        final AwContents awContents = testContainerView.getAwContents();
+
+        TestWebServer webServer = TestWebServer.start();
+        try {
+            final String title = "Title";
+            final String url1 = webServer.setResponse("/page1.html",
+                    "<html><head><title>" + title + "</title></head>Page 1</html>", null);
+            final String url2 = webServer.setResponse("/page2.html",
+                    "<html><head><title>" + title + "</title></head>Page 2</html>", null);
+            TestAwContentsClient.OnReceivedTitleHelper onReceivedTitleHelper =
+                    contentsClient.getOnReceivedTitleHelper();
+            int onReceivedTitleCallCount = onReceivedTitleHelper.getCallCount();
+            loadUrlSync(awContents, contentsClient.getOnPageFinishedHelper(), url1);
+            onReceivedTitleHelper.waitForCallback(onReceivedTitleCallCount);
+            assertEquals(title, onReceivedTitleHelper.getTitle());
+            // Verify that even if we load another page with the same title,
+            // onReceivedTitle is still being called.
+            onReceivedTitleCallCount = onReceivedTitleHelper.getCallCount();
+            loadUrlSync(awContents, contentsClient.getOnPageFinishedHelper(), url2);
+            onReceivedTitleHelper.waitForCallback(onReceivedTitleCallCount);
+            assertEquals(title, onReceivedTitleHelper.getTitle());
+        } finally {
+            webServer.shutdown();
+        }
+    }
 }

android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java

@@ -320,4 +320,34 @@ public Boolean call() {
             }
         });
     }
+
+    // See http://crbug.com/481570
+    @SmallTest
+    public void testTitleUpdatedWhenGoingBack() throws Throwable {
+        final TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
+                mContentsClient.getOnPageFinishedHelper();
+        NavigationHistory list = getNavigationHistory(mAwContents);
+        assertEquals(0, list.getEntryCount());
+
+        final String page1Url = addPage1ToServer(mWebServer);
+        final String page2Url = addPage2ToServer(mWebServer);
+
+        TestAwContentsClient.OnReceivedTitleHelper onReceivedTitleHelper =
+                mContentsClient.getOnReceivedTitleHelper();
+        // It would be unreliable to retrieve the call count after the first loadUrlSync,
+        // as it is not synchronous with respect to updating the title. Instead, we capture
+        // the initial call count (zero?) here, and keep waiting until we receive the update
+        // from the second page load.
+        int onReceivedTitleCallCount = onReceivedTitleHelper.getCallCount();
+        loadUrlSync(mAwContents, onPageFinishedHelper, page1Url);
+        loadUrlSync(mAwContents, onPageFinishedHelper, page2Url);
+        do {
+            onReceivedTitleHelper.waitForCallback(onReceivedTitleCallCount);
+            onReceivedTitleCallCount = onReceivedTitleHelper.getCallCount();
+        } while(!PAGE_2_TITLE.equals(onReceivedTitleHelper.getTitle()));
+        HistoryUtils.goBackSync(getInstrumentation(), mAwContents.getWebContents(),
+                onPageFinishedHelper);
+        onReceivedTitleHelper.waitForCallback(onReceivedTitleCallCount);
+        assertEquals(PAGE_1_TITLE, onReceivedTitleHelper.getTitle());
+    }
 }

android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java

@@ -26,7 +26,6 @@
  * AwContentsClient subclass used for testing.
  */
 public class TestAwContentsClient extends NullContentsClient {
-    private String mUpdatedTitle;
     private boolean mAllowSslError;
     private final OnPageStartedHelper mOnPageStartedHelper;
     private final OnPageFinishedHelper mOnPageFinishedHelper;
@@ -40,6 +39,7 @@ public class TestAwContentsClient extends NullContentsClient {
     private final OnEvaluateJavaScriptResultHelper mOnEvaluateJavaScriptResultHelper;
     private final AddMessageToConsoleHelper mAddMessageToConsoleHelper;
     private final OnScaleChangedHelper mOnScaleChangedHelper;
+    private final OnReceivedTitleHelper mOnReceivedTitleHelper;
     private final PictureListenerHelper mPictureListenerHelper;
     private final ShouldOverrideUrlLoadingHelper mShouldOverrideUrlLoadingHelper;
     private final DoUpdateVisitedHistoryHelper mDoUpdateVisitedHistoryHelper;
@@ -59,6 +59,7 @@ public TestAwContentsClient() {
         mOnEvaluateJavaScriptResultHelper = new OnEvaluateJavaScriptResultHelper();
         mAddMessageToConsoleHelper = new AddMessageToConsoleHelper();
         mOnScaleChangedHelper = new OnScaleChangedHelper();
+        mOnReceivedTitleHelper = new OnReceivedTitleHelper();
         mPictureListenerHelper = new PictureListenerHelper();
         mShouldOverrideUrlLoadingHelper = new ShouldOverrideUrlLoadingHelper();
         mDoUpdateVisitedHistoryHelper = new DoUpdateVisitedHistoryHelper();
@@ -156,13 +157,32 @@ public PictureListenerHelper getPictureListenerHelper() {
         return mPictureListenerHelper;
     }
 
+    /**
+     * Callback helper for onReceivedTitle.
+     */
+    public static class OnReceivedTitleHelper extends CallbackHelper {
+        private String mTitle;
+
+        public void notifyCalled(String title) {
+            mTitle = title;
+            super.notifyCalled();
+        }
+        public String getTitle() {
+            return mTitle;
+        }
+    }
+
+    public OnReceivedTitleHelper getOnReceivedTitleHelper() {
+        return mOnReceivedTitleHelper;
+    }
+
     @Override
     public void onReceivedTitle(String title) {
-        mUpdatedTitle = title;
+        mOnReceivedTitleHelper.notifyCalled(title);
     }
 
     public String getUpdatedTitle() {
-        return mUpdatedTitle;
+        return mOnReceivedTitleHelper.getTitle();
     }
 
     @Override

android_webview/native/aw_web_contents_delegate.cc

@@ -204,6 +204,18 @@ void AwWebContentsDelegate::ActivateContents(WebContents* contents) {
   }
 }
 
+void AwWebContentsDelegate::LoadingStateChanged(WebContents* source,
+                                                bool to_different_document) {
+  // Page title may have changed, need to inform the embedder.
+  // |source| may be null if loading has started.
+  JNIEnv* env = AttachCurrentThread();
+
+  ScopedJavaLocalRef<jobject> java_delegate = GetJavaDelegate(env);
+  if (java_delegate.obj()) {
+    Java_AwWebContentsDelegate_loadingStateChanged(env, java_delegate.obj());
+  }
+}
+
 void AwWebContentsDelegate::RequestMediaAccessPermission(
     WebContents* web_contents,
     const content::MediaStreamRequest& request,

android_webview/native/aw_web_contents_delegate.h

@@ -49,6 +49,8 @@ class AwWebContentsDelegate
 
   void CloseContents(content::WebContents* source) override;
   void ActivateContents(content::WebContents* contents) override;
+  void LoadingStateChanged(content::WebContents* source,
+                           bool to_different_document) override;
   void RequestMediaAccessPermission(
       content::WebContents* web_contents,
       const content::MediaStreamRequest& request,