aw: Turn on favicon retrieval only for tests that need it

URL override/intercept tests assumes it will only get requests that it
issues, so races with the favicon.ico request, making them flaky.
Disable favicon for all tests except the ones that are explicitly
testing favicon.

Also revert a previous hack to deal with racing with favicon.

BUG=347599

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

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

5 files changed, 10 insertions, 22 deletions

diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFaviconTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFaviconTest.java
index afde895..fd86014 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFaviconTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFaviconTest.java
@@ -82,6 +82,7 @@ public class AwContentsClientFaviconTest extends AwTestBase {
     @Override
     protected void setUp() throws Exception {
         super.setUp();
+        AwContents.setShouldDownloadFavicons();
         mWebServer = new TestWebServer(false);
     }
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
index 242fc59..fff12d2 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
@@ -10,7 +10,6 @@ import android.util.Pair;
 import org.chromium.android_webview.AwContents;
 import org.chromium.android_webview.InterceptedRequestData;
 import org.chromium.android_webview.test.util.CommonResources;
-import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.TestFileUtil;
 import org.chromium.content.browser.test.util.CallbackHelper;
@@ -40,16 +39,12 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase {
                 = new ConcurrentHashMap<String, InterceptedRequestData>();
             // This is read from the IO thread, so needs to be marked volatile.
             private volatile InterceptedRequestData mShouldInterceptRequestReturnValue = null;
-            private String mUrlToWaitFor;
             void setReturnValue(InterceptedRequestData value) {
                 mShouldInterceptRequestReturnValue = value;
             }
             void setReturnValueForUrl(String url, InterceptedRequestData value) {
                 mReturnValuesByUrls.put(url, value);
             }
-            public void setUrlToWaitFor(String url) {
-                mUrlToWaitFor = url;
-            }
             public List<String> getUrls() {
                 assert getCallCount() > 0;
                 return mShouldInterceptRequestUrls;
@@ -60,10 +55,8 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase {
                 return mShouldInterceptRequestReturnValue;
             }
             public void notifyCalled(String url) {
-                if (mUrlToWaitFor == null || mUrlToWaitFor.equals(url)) {
-                    mShouldInterceptRequestUrls.add(url);
-                    notifyCalled();
-                }
+                mShouldInterceptRequestUrls.add(url);
+                notifyCalled();
             }
         }
 
@@ -436,12 +429,8 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase {
         assertEquals(0, mWebServer.getRequestCount("/" + CommonResources.ABOUT_FILENAME));
     }
 
-    /*
-     @SmallTest
-     @Feature({"AndroidWebView"})
-     http://crbug.com/347599
-    */
-    @DisabledTest
+    @SmallTest
+    @Feature({"AndroidWebView"})
     public void testCalledForImage() throws Throwable {
         final String imagePath = "/" + CommonResources.FAVICON_FILENAME;
         mWebServer.setResponseBase64(imagePath,
@@ -495,13 +484,10 @@ public class AwContentsClientShouldInterceptRequestTest extends AwTestBase {
                     "<iframe src=\"" + aboutPageUrl + "\"/>"));
 
         int callCount = mShouldInterceptRequestHelper.getCallCount();
-        // These callbacks can race with favicon.ico callback.
-        mShouldInterceptRequestHelper.setUrlToWaitFor(aboutPageUrl);
         loadUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(), pageWithIframe);
-
-        mShouldInterceptRequestHelper.waitForCallback(callCount, 1);
-        assertEquals(1, mShouldInterceptRequestHelper.getUrls().size());
-        assertEquals(aboutPageUrl, mShouldInterceptRequestHelper.getUrls().get(0));
+        mShouldInterceptRequestHelper.waitForCallback(callCount, 2);
+        assertEquals(2, mShouldInterceptRequestHelper.getUrls().size());
+        assertEquals(aboutPageUrl, mShouldInterceptRequestHelper.getUrls().get(1));
     }
 
     private void calledForUrlTemplate(final String url) throws Exception {
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 ad524c0..cdd9a59 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
@@ -314,6 +314,7 @@ public class AwContentsTest extends AwTestBase {
     @SmallTest
     @Feature({"AndroidWebView"})
     public void testGetFavicon() throws Throwable {
+        AwContents.setShouldDownloadFavicons();
         final AwTestContainerView testView = createAwTestContainerViewOnMainSync(mContentsClient);
         final AwContents awContents = testView.getAwContents();
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
index 421070d..b078c1c 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
@@ -306,7 +306,6 @@ public class AwTestBase
         testContainerView.initialize(new AwContents(
                 mBrowserContext, testContainerView, testContainerView.getInternalAccessDelegate(),
                 awContentsClient, awSettings, testDependencyFactory.createLayoutSizer()));
-        AwContents.setShouldDownloadFavicons();
         return testContainerView;
     }
 
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
index a50a151..b4307f10 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/NavigationHistoryTest.java
@@ -41,6 +41,7 @@ public class NavigationHistoryTest extends AwTestBase {
     @Override
     public void setUp() throws Exception {
         super.setUp();
+        AwContents.setShouldDownloadFavicons();
         mContentsClient = new TestAwContentsClient();
         final AwTestContainerView testContainerView =
             createAwTestContainerViewOnMainSync(mContentsClient);