[Android Webview] Refactor LoadUrlTest and work on embedded_test_server custom handler

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

Index: android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
index 9f402a79dbde720be5cda7a0eaec48e38103e264..4dfd03fcb177942341c9f0e56c2887293c622444 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/LoadUrlTest.java
@@ -7,8 +7,10 @@ package org.chromium.android_webview.test;
 import android.support.test.filters.SmallTest;
 import android.util.Pair;
 
+import org.json.JSONArray;
+import org.json.JSONObject;
+
 import org.chromium.android_webview.AwContents;
-import org.chromium.android_webview.AwSettings;
 import org.chromium.android_webview.test.util.CommonResources;
 import org.chromium.android_webview.test.util.JSUtils;
 import org.chromium.base.annotations.SuppressFBWarnings;
@@ -17,6 +19,8 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.content.browser.test.util.HistoryUtils;
 import org.chromium.net.test.util.TestWebServer;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -27,6 +31,24 @@ import java.util.concurrent.TimeUnit;
  * Test suite for loadUrl().
  */
 public class LoadUrlTest extends AwTestBase {
+    private AwEmbeddedTestServer mTestServer;
+    private static final String ECHO_CURRENT_URL_HEADER = "echo header";

[sgurun-gerrit only, 2017/03/29 18:19:27]

Move these two static strings to the method they are used in. They are related
to redirect case only.

[sgurun-gerrit only, 2017/03/29 18:19:27]

also rename ECHO_CURRENT_URL_HEADER to ECHO_REDIRECTED_URL_HEADER

[shenghuazhang, 2017/04/11 01:38:27]

Done.

+    private static final String ECHO_INITIAL_URL_HEADER = "data content";
+    private static final String FOUND_EXTRA_HEADER = "Found Extra Header on Image URL.";
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        mTestServer =
+                AwEmbeddedTestServer.createAndStartServer(getInstrumentation().getTargetContext());
+    }
+
+    @Override
+    public void tearDown() throws Exception {
+        mTestServer.stopAndDestroyServer();
+        super.tearDown();
+    }
+
     @SmallTest
     @Feature({"AndroidWebView"})
     public void testDataUrl() throws Throwable {
@@ -111,20 +133,34 @@ public class LoadUrlTest extends AwTestBase {
         return result;
     }
 
-    private void validateRequestHeaders(String[] refNamesAndValues, List<String> request) {
-        List<String> matchingHeaders;
-        for (int i = 0; i < refNamesAndValues.length; i += 2) {
-            matchingHeaders = TestWebServer.getMatchingHeadersValues(request, refNamesAndValues[i]);
-            assertEquals(1, matchingHeaders.size());
-            assertEquals(refNamesAndValues[i + 1], matchingHeaders.get(0));
+    private void validateHeadersValue(final AwContents awContents,
+            final TestAwContentsClient contentsClient, String[] extraHeader, boolean realValue)

[sgurun-gerrit only, 2017/03/29 18:19:27]

s/realValue/shouldHeaderExist/

[shenghuazhang, 2017/04/11 01:38:27]

Done.

+            throws Exception {
+        String textContent = getJavaScriptResultBodyTextContent(awContents, contentsClient);
+        String[] header_values = textContent.split("\\\\n");
+        for (int i = 0; i < extraHeader.length; i += 2) {
+            assertEquals(realValue ? extraHeader[i + 1] : "None", header_values[i / 2]);
         }
     }
 
-    private void validateNoRequestHeaders(String[] refNamesAndValues, List<String> request) {
-        List<String> matchingHeaders;
-        for (int i = 0; i < refNamesAndValues.length; i += 2) {
-            matchingHeaders = TestWebServer.getMatchingHeadersValues(request, refNamesAndValues[i]);
-            assertEquals(0, matchingHeaders.size());
+    private void validateHeadersFromJson(final AwContents awContents,
+            final TestAwContentsClient contentsClient, String[] extraHeader, String jsonName,
+            boolean realValue) throws Exception {

[sgurun-gerrit only, 2017/03/29 18:19:27]

s/realValue/shouldHeaderExists/

[shenghuazhang, 2017/04/11 01:38:27]

Done.

+        String textContent = getJavaScriptResultBodyTextContent(awContents, contentsClient)
+                                     .replaceAll("\\\\\"", "\"");
+        JSONObject jsonObject = new JSONObject(textContent);
+        JSONArray jsonArray = jsonObject.getJSONArray(jsonName);
+        for (int i = 0; i < extraHeader.length; i += 2) {
+            String header = jsonArray.getString(i / 2);
+            assertEquals(realValue ? extraHeader[i + 1] : "None", header);
+        }
+    }
+
+    private final String encodeUrl(String url) {
+        try {
+            return URLEncoder.encode(url, "UTF-8");
+        } catch (UnsupportedEncodingException e) {
+            throw new AssertionError(e);
         }
     }
 
@@ -135,32 +171,24 @@ public class LoadUrlTest extends AwTestBase {
         final AwTestContainerView testContainerView =
                 createAwTestContainerViewOnMainSync(contentsClient);
         final AwContents awContents = testContainerView.getAwContents();
-
-        TestWebServer webServer = TestWebServer.start();
-        try {
-            final String imagePath = "/" + CommonResources.FAVICON_FILENAME;
-            webServer.setResponseBase64(imagePath,
-                    CommonResources.FAVICON_DATA_BASE64, CommonResources.getImagePngHeaders(true));
-            final String path = "/load_url_with_extra_headers_test.html";
-            final String url = webServer.setResponse(
-                    path,
-                    CommonResources.getOnImageLoadedHtml(CommonResources.FAVICON_FILENAME),
-                    null);
-            String[] extraHeaders = {
-                "X-ExtraHeaders1", "extra-header-data1",
-                "x-extraHeaders2", "EXTRA-HEADER-DATA2"
-            };
-
-            loadUrlWithExtraHeadersSync(awContents,
-                                        contentsClient.getOnPageFinishedHelper(),
-                                        url,
-                                        createHeadersMap(extraHeaders));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-            // Verify that extra headers are only passed for the main resource.
-            validateNoRequestHeaders(extraHeaders, webServer.getLastRequest(imagePath));
-        } finally {
-            webServer.shutdown();
-        }
+        enableJavaScriptOnUiThread(awContents);
+
+        String[] extraHeaders = {
+                "X-ExtraHeaders1", "extra-header-data1", "x-extraHeaders2", "EXTRA-HEADER-DATA2"};
+
+        final String url1 = mTestServer.getURL(

[sgurun-gerrit only, 2017/03/29 18:19:27]

the handler image-onload-html is very specific to this test. Url1 is not used
locally except for passing to the handler as a query param. Move it to the
handler, dropping imagesrc parameter.

[shenghuazhang, 2017/04/11 01:38:27]

Hmm... If the image resource is the only thing we need, then I guess the url1
could be omitted, and simply make use of the image resource in the handler of
'image-onload-html' directly.

But in the test we will also verify the header values of the image resource
url(url1), and the header should be potentially embedded in the request handled
by handler 'set-image-response'(url1), which is not the one handled by handler
'image-onload-html'(url2).

S.t. we still need a real url request of the image. So the server instance here
to make it's own request:
    final String url1 = mTestServer.getURL("/set-image-response?...)

+                "/set-image-response?resource=" + encodeUrl(CommonResources.FAVICON_DATA_BASE64)
+                + "&header=" + extraHeaders[0] + "&header=" + extraHeaders[2]);
+        final String url2 = mTestServer.getURL("/image-onload-html?imagesrc=" + encodeUrl(url1)

[sgurun-gerrit only, 2017/03/29 18:19:27]

rename url2 to url once you move url1 to handler

[shenghuazhang, 2017/04/11 01:38:27]

Same reason as above.

+                + "&header=" + extraHeaders[0] + "&header=" + extraHeaders[2]);
+
+        loadUrlWithExtraHeadersSync(awContents, contentsClient.getOnPageFinishedHelper(), url2,
+                createHeadersMap(extraHeaders));
+        // Verify that extra headers are passed to the loaded url.
+        validateHeadersValue(awContents, contentsClient, extraHeaders, true);
+        // Verify that extra headers are only passed for the main resource.
+        String textContent = getJavaScriptResultBodyTextContent(awContents, contentsClient);
+        assertFalse(textContent.contains(FOUND_EXTRA_HEADER));

[shenghuazhang, 2017/03/23 01:26:16]

Check the extra headers not exist in the image source url from native side -
handler 'HandleSetImageResponse' (see more comments in
//net/test/embedded_test_server/android/embedded_test_server_android.cc)
and check the result here.

[sgurun-gerrit only, 2017/03/29 18:19:27]

I think this is racy. You are relying on the JS code that fetches the image to
execute and finish its execution to confirm that image loading with extra
headers did not happen. this is probably why you added the title at the handler
js: you can wait for title to be updated to confirm js worked and use it to
assert here

[shenghuazhang, 2017/04/11 01:38:27]

Verifying the title here is a less flaky and more stable way than the JS loading
way as our offline discussion conclusion. Will refactor this test and the
handler to fit it.

     }
 
     @SmallTest
@@ -171,28 +199,17 @@ public class LoadUrlTest extends AwTestBase {
                 createAwTestContainerViewOnMainSync(contentsClient);
         final AwContents awContents = testContainerView.getAwContents();
 
-        TestWebServer webServer = TestWebServer.start();
-        try {
-            final String path = "/no_overriding_of_existing_headers_test.html";
-            final String url = webServer.setResponse(
-                    path,
-                    "<html><body>foo</body></html>",
-                    null);
-            String[] extraHeaders = {"user-agent", "Borewicz 07 & Bond 007"};
-
-            loadUrlWithExtraHeadersSync(awContents,
-                                        contentsClient.getOnPageFinishedHelper(),
-                                        url,
-                                        createHeadersMap(extraHeaders));
-            List<String> matchingHeaders = TestWebServer.getMatchingHeadersValues(
-                    webServer.getLastRequest(path), extraHeaders[0]);
-            assertEquals(1, matchingHeaders.size());
-            // Just check that the value is there, and it's not the one we provided.
-            assertTrue(matchingHeaders.get(0).length() > 0);
-            assertFalse(extraHeaders[1].equals(matchingHeaders.get(0)));
-        } finally {
-            webServer.shutdown();
-        }
+        enableJavaScriptOnUiThread(awContents);
+
+        final String url = mTestServer.getURL("/echoheader?user-agent");
+        String[] extraHeaders = {"user-agent", "Borewicz 07 & Bond 007"};
+
+        loadUrlWithExtraHeadersSync(awContents, contentsClient.getOnPageFinishedHelper(), url,
+                createHeadersMap(extraHeaders));
+        String header = getJavaScriptResultBodyTextContent(awContents, contentsClient);
+        // Just check that the value is there, and it's not the one we provided.
+        assertFalse(header.isEmpty());
+        assertFalse(extraHeaders[1].equals(header));
     }
 
     @SmallTest
@@ -202,30 +219,17 @@ public class LoadUrlTest extends AwTestBase {
         final AwTestContainerView testContainerView =
                 createAwTestContainerViewOnMainSync(contentsClient);
         final AwContents awContents = testContainerView.getAwContents();
-
-        TestWebServer webServer = TestWebServer.start();
-        try {
-            final String path = "/reload_with_extra_headers_test.html";
-            final String url = webServer.setResponse(path,
-                    "<html><body>foo</body></html>",
-                    createHeadersList(new String[] { "cache-control", "no-cache, no-store" }));
-            String[] extraHeaders = {
-                "X-ExtraHeaders1", "extra-header-data1",
-                "x-extraHeaders2", "EXTRA-HEADER-DATA2"
-            };
-
-            loadUrlWithExtraHeadersSync(awContents,
-                                        contentsClient.getOnPageFinishedHelper(),
-                                        url,
-                                        createHeadersMap(extraHeaders));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-
-            reloadSync(awContents, contentsClient.getOnPageFinishedHelper());
-            assertEquals(2, webServer.getRequestCount(path));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-        } finally {
-            webServer.shutdown();
-        }
+        enableJavaScriptOnUiThread(awContents);
+        String[] extraHeaders = {
+                "X-ExtraHeaders1", "extra-header-data1", "x-extraHeaders2", "EXTRA-HEADER-DATA2"};
+        final String url =
+                mTestServer.getURL("/echoheader?" + extraHeaders[0] + "&" + extraHeaders[2]);
+
+        loadUrlWithExtraHeadersSync(awContents, contentsClient.getOnPageFinishedHelper(), url,
+                createHeadersMap(extraHeaders));
+        validateHeadersValue(awContents, contentsClient, extraHeaders, true);
+        reloadSync(awContents, contentsClient.getOnPageFinishedHelper());
+        validateHeadersValue(awContents, contentsClient, extraHeaders, true);
     }
 
     @SuppressFBWarnings("DLS_DEAD_LOCAL_STORE")
@@ -237,34 +241,27 @@ public class LoadUrlTest extends AwTestBase {
                 createAwTestContainerViewOnMainSync(contentsClient);
         final AwContents awContents = testContainerView.getAwContents();
 
-        TestWebServer webServer = TestWebServer.start();
-        try {
-            final String path = "/redirect_and_reload_with_extra_headers_test.html";
-            final String url = webServer.setResponse(path,
-                    "<html><body>foo</body></html>",
-                    createHeadersList(new String[] { "cache-control", "no-cache, no-store" }));
-            final String redirectedPath = "/redirected.html";
-            final String redirectedUrl = webServer.setRedirect(redirectedPath, path);
-            String[] extraHeaders = {
-                "X-ExtraHeaders1", "extra-header-data1",
-                "x-extraHeaders2", "EXTRA-HEADER-DATA2"
-            };
-
-            loadUrlWithExtraHeadersSync(awContents,
-                                        contentsClient.getOnPageFinishedHelper(),
-                                        redirectedUrl,
-                                        createHeadersMap(extraHeaders));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(redirectedPath));
-
-            // WebView will only reload the main page.
-            reloadSync(awContents, contentsClient.getOnPageFinishedHelper());
-            assertEquals(2, webServer.getRequestCount(path));
-            // No extra headers. This is consistent with legacy behavior.
-            validateNoRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-        } finally {
-            webServer.shutdown();
-        }
+        enableJavaScriptOnUiThread(awContents);
+
+        String[] extraHeaders = {
+                "X-ExtraHeaders1", "extra-header-data1", "x-extraHeaders2", "EXTRA-HEADER-DATA2"};
+        final String redirectedUrl = mTestServer.getURL("/echoheader-and-set-data?header="
+                + extraHeaders[0] + "&header=" + extraHeaders[2]);
+        final String initialUrl =
+                mTestServer.getURL("/server-redirect-echoheader?url=" + encodeUrl(redirectedUrl)
+                        + "&header=" + extraHeaders[0] + "&header=" + extraHeaders[2]);
+        loadUrlWithExtraHeadersSync(awContents, contentsClient.getOnPageFinishedHelper(),
+                initialUrl, createHeadersMap(extraHeaders));
+        validateHeadersFromJson(
+                awContents, contentsClient, extraHeaders, ECHO_CURRENT_URL_HEADER, true);

[shenghuazhang, 2017/03/23 01:26:16]

The page body text content is in json format, like,

  {
    "echo header": [header1, header2],
    "data content": [header3, header4];
  }

"echo header" contains the header results in the current request
(redirectedUrl).
"data content" contains the header results of the initialUrl.

[sgurun-gerrit only, 2017/03/29 18:19:27]

Acknowledged.

+        validateHeadersFromJson(
+                awContents, contentsClient, extraHeaders, ECHO_INITIAL_URL_HEADER, true);
+
+        // WebView will only reload the main page.
+        reloadSync(awContents, contentsClient.getOnPageFinishedHelper());
+        // No extra headers. This is consistent with legacy behavior.
+        validateHeadersFromJson(
+                awContents, contentsClient, extraHeaders, ECHO_CURRENT_URL_HEADER, false);
     }
 
     @SuppressFBWarnings("DLS_DEAD_LOCAL_STORE")
@@ -275,48 +272,31 @@ public class LoadUrlTest extends AwTestBase {
         final AwTestContainerView testContainerView =
                 createAwTestContainerViewOnMainSync(contentsClient);
         final AwContents awContents = testContainerView.getAwContents();
-        final AwSettings settings = getAwSettingsOnUiThread(awContents);
-        settings.setJavaScriptEnabled(true);
-
-        TestWebServer webServer = TestWebServer.start();
-        try {
-            final String nextPath = "/next.html";
-            final String nextUrl = webServer.setResponse(nextPath,
-                    "<html><body>Next!</body></html>",
-                    null);
-            final String path = "/renderer_nav_and_go_back_with_extra_headers_test.html";
-            final String url = webServer.setResponse(path,
-                    "<html><body><a id=\"next\" href=\"next.html\">Next!</a></body></html>",
-                    createHeadersList(new String[] { "cache-control", "no-cache, no-store" }));
-            String[] extraHeaders = {
-                "X-ExtraHeaders1", "extra-header-data1",
-                "x-extraHeaders2", "EXTRA-HEADER-DATA2"
-            };
-
-            loadUrlWithExtraHeadersSync(awContents,
-                                        contentsClient.getOnPageFinishedHelper(),
-                                        url,
-                                        createHeadersMap(extraHeaders));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-
-            int currentCallCount = contentsClient.getOnPageFinishedHelper().getCallCount();
-            JSUtils.clickOnLinkUsingJs(this,
-                                       awContents,
-                                       contentsClient.getOnEvaluateJavaScriptResultHelper(),
-                                       "next");
-            contentsClient.getOnPageFinishedHelper().waitForCallback(
-                    currentCallCount, 1, WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);
-            // No extra headers for the page navigated via clicking.
-            validateNoRequestHeaders(extraHeaders, webServer.getLastRequest(nextPath));
-
-            HistoryUtils.goBackSync(getInstrumentation(),
-                                    awContents.getWebContents(),
-                                    contentsClient.getOnPageFinishedHelper());
-            assertEquals(2, webServer.getRequestCount(path));
-            validateRequestHeaders(extraHeaders, webServer.getLastRequest(path));
-        } finally {
-            webServer.shutdown();
-        }
+        enableJavaScriptOnUiThread(awContents);
+
+        String[] extraHeaders = {
+                "X-ExtraHeaders1", "extra-header-data1", "x-extraHeaders2", "EXTRA-HEADER-DATA2"};
+        final String redirectedUrl =
+                mTestServer.getURL("/echoheader?" + extraHeaders[0] + "&" + extraHeaders[2]);
+        final String initialUrl =
+                mTestServer.getURL("/click-redirect?url=" + encodeUrl(redirectedUrl)
+                        + "&echoheader=" + extraHeaders[0] + "&echoheader=" + extraHeaders[2]);
+
+        loadUrlWithExtraHeadersSync(awContents, contentsClient.getOnPageFinishedHelper(),
+                initialUrl, createHeadersMap(extraHeaders));
+        validateHeadersValue(awContents, contentsClient, extraHeaders, true);
+
+        int currentCallCount = contentsClient.getOnPageFinishedHelper().getCallCount();
+        JSUtils.clickOnLinkUsingJs(
+                this, awContents, contentsClient.getOnEvaluateJavaScriptResultHelper(), "click");
+        contentsClient.getOnPageFinishedHelper().waitForCallback(
+                currentCallCount, 1, WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+        // No extra headers for the page navigated via clicking.
+        validateHeadersValue(awContents, contentsClient, extraHeaders, false);
+
+        HistoryUtils.goBackSync(getInstrumentation(), awContents.getWebContents(),
+                contentsClient.getOnPageFinishedHelper());
+        validateHeadersValue(awContents, contentsClient, extraHeaders, true);
     }
 
     private static class OnReceivedTitleClient extends TestAwContentsClient {
@@ -340,8 +320,7 @@ public class LoadUrlTest extends AwTestBase {
         final AwTestContainerView testContainerView =
                 createAwTestContainerViewOnMainSync(contentsClient);
         final AwContents awContents = testContainerView.getAwContents();
-        final AwSettings settings = getAwSettingsOnUiThread(awContents);
-        settings.setJavaScriptEnabled(true);
+        enableJavaScriptOnUiThread(awContents);
 
         contentsClient.setOnReceivedTitleCallback(new Runnable() {
             @Override