Android WebView visited link highlighting implementation part 2

Hook up getVisitedHistory client callback.

As discussed, this is called on the first LoadURL and adds to the
existing table.

Unfortunately the public api does not allow iterating over the URLs, so
it could potentially use a lot of memory.

A new test is exposing gpu related crashes after calling destroy.

t_b_r for tests added
TBR=mkosiba@chromium.org,benm@chromium.org
BUG=172184
Android only change. Ran through android bots.
NOTRY=true

Review URL: https://chromiumcodereview.appspot.com/11886055

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@178851 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 120 insertions, 1 deletions

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 de25e9e..4fed78c 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -98,6 +98,7 @@ public class AwContents {
     private final AwSettings mSettings;
     private boolean mIsPaused;
     private Bitmap mFavicon;
+    private boolean mHasRequestedVisitedHistoryFromClient;
 
     // Must call nativeUpdateLastHitTestData first to update this before use.
     private final HitTestData mPossiblyStaleHitTestData;
@@ -367,6 +368,22 @@ public class AwContents {
         return mFavicon;
     }
 
+    private void requestVisitedHistoryFromClient() {
+        ValueCallback<String[]> callback = new ValueCallback<String[]>() {
+            @Override
+            public void onReceiveValue(final String[] value) {
+                ThreadUtils.runOnUiThread(new Runnable() {
+                    @Override
+                    public void run() {
+                        if (mNativeAwContents == 0) return;
+                        nativeAddVisitedLinks(mNativeAwContents, value);
+                    }
+                });
+            }
+        };
+        mContentsClient.getVisitedHistory(callback);
+    }
+
     /**
      * Load url without fixing up the url string. Consumers of ContentView are responsible for
      * ensuring the URL passed in is properly formatted (i.e. the scheme has been added if left
@@ -397,6 +414,14 @@ public class AwContents {
             // callbacks, so it's safe to use string comparison as an equality check later on.
             mInterceptNavigationDelegate.onUrlLoadRequested(mContentViewCore.getUrl());
         }
+
+        // The behavior of WebViewClassic uses the populateVisitedLinks callback in WebKit.
+        // Chromium does not use this use code path and the best emulation of this behavior to call
+        // request visited links once on the first URL load of the WebView.
+        if (!mHasRequestedVisitedHistoryFromClient) {
+          mHasRequestedVisitedHistoryFromClient = true;
+          requestVisitedHistoryFromClient();
+        }
     }
 
     /**
@@ -965,6 +990,8 @@ public class AwContents {
     private native void nativeSetInterceptNavigationDelegate(int nativeAwContents,
             InterceptNavigationDelegate navigationInterceptionDelegate);
 
+    private native void nativeAddVisitedLinks(int nativeAwContents, String[] visitedLinks);
+
     private native boolean nativeDrawSW(int nativeAwContents, Canvas canvas);
     private native void nativeSetScrollForHWFrame(int nativeAwContents, int scrollX, int scrollY);
     private native int nativeFindAllSync(int nativeAwContents, String searchString);
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
index e80bafd5..225e6ec 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
@@ -14,6 +14,7 @@ import android.util.Log;
 import android.view.KeyEvent;
 import android.webkit.ConsoleMessage;
 import android.webkit.GeolocationPermissions;
+import android.webkit.ValueCallback;
 
 import org.chromium.content.browser.ContentViewClient;
 import org.chromium.content.browser.ContentViewCore;
@@ -208,7 +209,9 @@ public abstract class AwContentsClient extends ContentViewClient {
     //--------------------------------------------------------------------------------------------
 
     // TODO(boliu): Make this abstract.
-    public void doUpdateVisitedHistory(String url, boolean isReload) {}
+    public void getVisitedHistory(ValueCallback<String[]> callback) {}
+
+    public abstract void doUpdateVisitedHistory(String url, boolean isReload);
 
     public abstract void onProgressChanged(int progress);
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java
index a4b9fb4..3c116d1 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java
@@ -284,6 +284,57 @@ public class AwContentsTest extends AndroidWebViewTestBase {
         }
     }
 
+    @Feature({"AndroidWebView"})
+    @SmallTest
+    public void testGetVisitedHistoryExerciseCodePath() throws Throwable {
+        // Due to security/privacy restrictions around the :visited css property, it is not
+        // possible test this end to end without using the flaky and brittle capturing picture of
+        // the web page. So we are doing the next best thing, exercising all the code paths.
+
+        mContentsClient.mSaveGetVisitedHistoryCallback = true;
+        AwTestContainerView testView = createAwTestContainerViewOnMainSync(mContentsClient);
+        AwContents awContents = testView.getAwContents();
+
+        final String path = "/testGetVisitedHistoryExerciseCodePath.html";
+        final String visitedLinks[] = {"http://foo.com", "http://bar.com", null};
+        final String html = "<a src=\"http://foo.com\">foo</a><a src=\"http://bar.com\">bar</a>";
+
+        TestWebServer webServer = null;
+        try {
+            webServer = new TestWebServer(false);
+            final String pageUrl = webServer.setResponse(path, html, null);
+
+            loadUrlSync(awContents, mContentsClient.getOnPageFinishedHelper(), pageUrl);
+            assertNotNull(mContentsClient.mGetVisitedHistoryCallback);
+
+            mContentsClient.mGetVisitedHistoryCallback.onReceiveValue(visitedLinks);
+            mContentsClient.mGetVisitedHistoryCallback.onReceiveValue(null);
+
+            loadUrlSync(awContents, mContentsClient.getOnPageFinishedHelper(), pageUrl);
+        } finally {
+            if (webServer != null) webServer.shutdown();
+        }
+    }
+
+    /*
+     * @Feature({"AndroidWebView"})
+     * @SmallTest
+     * Exercising code after destroy causes gpu related crashes. See crbug.com/172184.
+     */
+    @DisabledTest
+    public void testGetVisitedHistoryCallbackAfterDestroy() throws Throwable {
+        mContentsClient.mSaveGetVisitedHistoryCallback = true;
+        AwTestContainerView testView = createAwTestContainerViewOnMainSync(mContentsClient);
+        AwContents awContents = testView.getAwContents();
+
+        loadUrlSync(awContents, mContentsClient.getOnPageFinishedHelper(), "about:blank");
+        assertNotNull(mContentsClient.mGetVisitedHistoryCallback);
+
+        awContents.destroy();
+        mContentsClient.mGetVisitedHistoryCallback.onReceiveValue(new String[] {"abc.def"});
+        mContentsClient.mGetVisitedHistoryCallback.onReceiveValue(null);
+    }
+
     private void autoLoginTestHelper(final String testName, final String xAutoLoginHeader,
             final String expectedRealm, final String expectedAccount, final String expectedArgs)
             throws Throwable {
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/NullContentsClient.java b/android_webview/javatests/src/org/chromium/android_webview/test/NullContentsClient.java
index 80ac4f2..7f681f1 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/NullContentsClient.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/NullContentsClient.java
@@ -9,6 +9,7 @@ import android.os.Message;
 import android.view.KeyEvent;
 import android.webkit.ConsoleMessage;
 import android.webkit.GeolocationPermissions;
+import android.webkit.ValueCallback;
 
 import org.chromium.android_webview.AwContentsClient;
 import org.chromium.android_webview.AwHttpAuthHandler;
@@ -31,6 +32,10 @@ class NullContentsClient extends AwContentsClient {
     }
 
     @Override
+    public void getVisitedHistory(ValueCallback<String[]> callback) {
+    }
+
+    @Override
     public void doUpdateVisitedHistory(String url, boolean isReload) {
     }
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java b/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
index 371f3c9..7d99adf3 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
@@ -5,6 +5,7 @@
 package org.chromium.android_webview.test;
 
 import android.webkit.ConsoleMessage;
+import android.webkit.ValueCallback;
 
 import org.chromium.content.browser.test.util.CallbackHelper;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnPageStartedHelper;
@@ -125,6 +126,16 @@ class TestAwContentsClient extends NullContentsClient {
         }
     }
 
+    ValueCallback<String[]> mGetVisitedHistoryCallback;
+    boolean mSaveGetVisitedHistoryCallback = false;
+
+    @Override
+    public void getVisitedHistory(ValueCallback<String[]> callback) {
+        if (mSaveGetVisitedHistoryCallback) {
+            mGetVisitedHistoryCallback = callback;
+        }
+    }
+
     String mLastVisitedUrl;
     boolean mLastVisitIsReload;
 
diff --git a/android_webview/native/aw_contents.cc b/android_webview/native/aw_contents.cc
index 3750650..f170e492 100644
--- a/android_webview/native/aw_contents.cc
+++ b/android_webview/native/aw_contents.cc
@@ -6,6 +6,7 @@
 
 #include <sys/system_properties.h>
 
+#include "android_webview/browser/aw_browser_context.h"
 #include "android_webview/browser/aw_browser_main_parts.h"
 #include "android_webview/browser/net_disk_cache_remover.h"
 #include "android_webview/browser/renderer_host/aw_render_view_host_ext.h"
@@ -25,6 +26,7 @@
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
 #include "base/pickle.h"
+#include "base/string16.h"
 #include "base/supports_user_data.h"
 #include "cc/layer.h"
 #include "components/navigation_interception/intercept_navigation_delegate.h"
@@ -611,6 +613,25 @@ void AwContents::SetInterceptNavigationDelegate(JNIEnv* env,
       make_scoped_ptr(new InterceptNavigationDelegate(env, delegate)));
 }
 
+void AwContents::AddVisitedLinks(JNIEnv* env,
+                                   jobject obj,
+                                   jobjectArray jvisited_links) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  std::vector<string16> visited_link_strings;
+  base::android::AppendJavaStringArrayToStringVector(
+      env, jvisited_links, &visited_link_strings);
+
+  std::vector<GURL> visited_link_gurls;
+  for (std::vector<string16>::const_iterator itr = visited_link_strings.begin();
+       itr != visited_link_strings.end();
+       ++itr) {
+    visited_link_gurls.push_back(GURL(*itr));
+  }
+
+  AwBrowserContext::FromWebContents(web_contents_.get())
+      ->AddVisitedURLs(visited_link_gurls);
+}
+
 static jint Init(JNIEnv* env,
                  jobject obj,
                  jobject web_contents_delegate) {
diff --git a/android_webview/native/aw_contents.h b/android_webview/native/aw_contents.h
index fffed32..347705a 100644
--- a/android_webview/native/aw_contents.h
+++ b/android_webview/native/aw_contents.h
@@ -88,6 +88,7 @@ class AwContents : public FindHelper::Listener,
   void SetIoThreadClient(JNIEnv* env, jobject obj, jobject client);
   void SetInterceptNavigationDelegate(JNIEnv* env, jobject obj,
                                       jobject delegate);
+  void AddVisitedLinks(JNIEnv* env, jobject obj, jobjectArray jvisited_links);
   base::android::ScopedJavaLocalRef<jbyteArray> GetCertificate(
       JNIEnv* env, jobject obj);
   void RequestNewHitTestDataAt(JNIEnv* env, jobject obj, jint x, jint y);