Fix Android zombie server on crash

Fixes Android instrumentation tests that use EmbeddedTestServerRule
leaving behind a zombie server process if they crash. This is typically
not much of an issue, as the next test to use EmbeddedTestServerRule
will pick a free port instead of the one in use, but a number of tests
have to set a static port due to being render tests (and a variable port
will cause the test to fail due to the port being visible in the URL
bar).

Bug: 1019983
Change-Id: I9adcb22e75ebf0f83d15db6c825e896e618e8f36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2056935Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742706}

build/android/pylib/local/device/local_device_instrumentation_test_run.py

@@ -658,9 +658,10 @@ class LocalDeviceInstrumentationTestRun(
       # steps that involve ADB. These steps should NOT depend on any info in
       # the results! Things such as whether the test CRASHED have not yet been
       # determined.
-      post_test_steps = [restore_flags, restore_timeout_scale,
-                         handle_coverage_data, handle_render_test_data,
-                         pull_ui_screen_captures]
+      post_test_steps = [
+          restore_flags, restore_timeout_scale, handle_coverage_data,
+          handle_render_test_data, pull_ui_screen_captures
+      ]
       if self._env.concurrent_adb:
         reraiser_thread.RunAsync(post_test_steps)
       else:
@@ -697,6 +698,15 @@ class LocalDeviceInstrumentationTestRun(
       self._SaveScreenshot(device, screenshot_device_file, test_display_name,
                            results, 'dialog_box_screenshot')
 
+    # The crash result can be set above or in
+    # InstrumentationTestRun.GenerateTestResults. If a test crashes,
+    # subprocesses such as the one used by EmbeddedTestServerRule can be left
+    # alive in a bad state, so kill them now.
+    for r in results:
+      if r.GetType() == base_test_result.ResultType.CRASH:
+        for apk in self._test_instance.additional_apks:
+          device.ForceStop(apk.GetPackageName())
+
     # Handle failures by:
     #   - optionally taking a screenshot
     #   - logging the raw output at INFO level

chrome/android/javatests/src/org/chromium/chrome/browser/vr/VrBrowserDialogTest.java

@@ -60,9 +60,6 @@ public class VrBrowserDialogTest {
 
     @Before
     public void setUp() {
-        // Need to ensure that the SERVER_PORT used is fixed for RenderTests
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         mVrBrowserTestFramework = new VrBrowserTestFramework(mVrTestRule);
 
         // Notifications on O+ are handled via Android Notification Channels, and thus can cause

chrome/android/javatests/src/org/chromium/chrome/browser/vr/VrBrowserNativeUiTest.java

@@ -298,9 +298,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testKeyboardAppearsOnUrlBarClick() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         NativeUiUtils.clickElementAndWaitForUiQuiescence(UserFriendlyElementName.URL, new PointF());
         // For whatever reason, the laser has a lot of random noise (not visible to an actual user)
         // when the keyboard is present on certain OS/hardware configurations (currently known to
@@ -329,9 +326,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testOverflowMenuAppears() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         // TODO(https://crbug.com/930840): Remove this when the weird gradient behavior is fixed.
         mRenderTestRule.setPixelDiffThreshold(2);
         NativeUiUtils.clickElementAndWaitForUiQuiescence(
@@ -347,9 +341,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testDataUrlEmphasis() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         NativeUiUtils.enableMockedInput();
         mVrTestRule.loadUrl("data:,Hello%2C%20World!", PAGE_LOAD_TIMEOUT_S);
         NativeUiUtils.waitForUiQuiescence();
@@ -364,9 +355,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testFileUrlEmphasis() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         NativeUiUtils.enableMockedInput();
         mVrTestRule.loadUrl(XrTestFramework.getFileUrlForHtmlTestFile("2d_permission_page"),
                 PAGE_LOAD_TIMEOUT_S);
@@ -383,9 +371,6 @@ public class VrBrowserNativeUiTest {
     @Feature({"Browser", "RenderTest"})
     public void testRepositionBarDoesNotAppearWithKeyboardOpen()
             throws InterruptedException, TimeoutException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         // Use the mock keyboard so it doesn't show, reducing the chance of flakes due to AA.
         NativeUiUtils.enableMockedKeyboard();
         mVrTestRule.loadUrl(mVrBrowserTestFramework.getUrlForFile("generic_text_entry_page"),
@@ -410,9 +395,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testUrlBarHovering() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         testUrlBarHoveringImpl(false);
     }
 
@@ -424,9 +406,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testUrlBarHoveringIncognito() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         mVrBrowserTestFramework.openIncognitoTab("about:blank");
         testUrlBarHoveringImpl(true);
     }
@@ -465,9 +444,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testOverflowMenuHovering() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         testOverflowMenuHoveringImpl(false);
     }
 
@@ -479,9 +455,6 @@ public class VrBrowserNativeUiTest {
     @LargeTest
     @Feature({"Browser", "RenderTest"})
     public void testOverflowMenuHoveringIncognito() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         mVrBrowserTestFramework.openIncognitoTab("about:blank");
         testOverflowMenuHoveringImpl(true);
     }
@@ -539,9 +512,6 @@ public class VrBrowserNativeUiTest {
     @MediumTest
     @Feature({"Browser", "RenderTest"})
     public void testSuggestionHovering() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
-
         // Input some text to get suggestions.
         NativeUiUtils.enableMockedKeyboard();
         NativeUiUtils.clickElementAndWaitForUiQuiescence(UserFriendlyElementName.URL, new PointF());
@@ -616,8 +586,6 @@ public class VrBrowserNativeUiTest {
     @MediumTest
     @Feature({"Browser", "RenderTest"})
     public void testScrollResizing() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
         mVrBrowserTestFramework.loadFileAndAwaitInitialization(
                 "test_content_resizing_does_not_affect_webpage", PAGE_LOAD_TIMEOUT_S);
         mVrBrowserTestFramework.executeStepAndWait("stepGetInitialDimensions()");
@@ -637,8 +605,6 @@ public class VrBrowserNativeUiTest {
     @Feature({"Browser", "RenderTest"})
     public void testOverflowAndKeyboardFollowContentQuad()
             throws InterruptedException, TimeoutException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
         mVrTestRule.loadUrl(mVrBrowserTestFramework.getUrlForFile("generic_text_entry_page"),
                 PAGE_LOAD_TIMEOUT_S);
         // Drag the content quad up and to the left.
@@ -683,8 +649,6 @@ public class VrBrowserNativeUiTest {
     @Feature({"Browser", "RenderTest"})
     public void testRepositionBarDoesNotAppearWithPermissionPromptVisible()
             throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
         mVrBrowserTestFramework.loadFileAndAwaitInitialization(
                 "2d_permission_page", PAGE_LOAD_TIMEOUT_S);
         NativeUiUtils.enableMockedInput();
@@ -805,8 +769,6 @@ public class VrBrowserNativeUiTest {
     @MediumTest
     @Feature({"Browser", "RenderTest"})
     public void testSecurityTokenOnHttp() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
         mVrTestRule.getEmbeddedTestServerRule().setServerUsesHttps(false);
         testSecurityTokenImpl("security_token_http");
     }
@@ -818,8 +780,6 @@ public class VrBrowserNativeUiTest {
     @MediumTest
     @Feature({"Browser", "RenderTest"})
     public void testSecurityTokenOnHttps() throws InterruptedException, IOException {
-        // Render tests need to ensure that they are always run on the same port.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
         testSecurityTokenImpl("security_token_https");
     }
 
@@ -831,9 +791,6 @@ public class VrBrowserNativeUiTest {
     @MediumTest
     @Feature({"Browser", "RenderTest"})
     public void testSecurityTokenOnHttpsBadCertificate() throws InterruptedException, IOException {
-        // The test framework ensures that the server defaults to https, but we still need to ensure
-        // the proper port, and to set the invalid cert.
-        mVrTestRule.getEmbeddedTestServerRule().setServerPort(XrTestFramework.SERVER_PORT);
         mVrTestRule.getEmbeddedTestServerRule().setCertificateType(ServerCertificate.CERT_EXPIRED);
         testSecurityTokenImpl("security_token_https_bad_cert");
     }

chrome/android/javatests/src/org/chromium/chrome/browser/vr/XrTestFramework.java

@@ -400,6 +400,9 @@ public abstract class XrTestFramework {
 
         // WebXr requires HTTPS, so configure the server to by default use it.
         mRule.getEmbeddedTestServerRule().setServerUsesHttps(true);
+        // Tests that use RenderTestRule need a static port, as the port shows up in the URL. It
+        // doesn't hurt to use the same port in non-RenderTests, so set here.
+        mRule.getEmbeddedTestServerRule().setServerPort(SERVER_PORT);
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/vr/jsdialog/VrBrowserJavaScriptModalDialogTest.java

@@ -7,7 +7,6 @@ package org.chromium.chrome.browser.vr.jsdialog;
 import static org.chromium.chrome.browser.vr.XrTestFramework.PAGE_LOAD_TIMEOUT_S;
 import static org.chromium.chrome.browser.vr.XrTestFramework.POLL_TIMEOUT_LONG_MS;
 import static org.chromium.chrome.browser.vr.XrTestFramework.POLL_TIMEOUT_SHORT_MS;
-import static org.chromium.chrome.browser.vr.XrTestFramework.SERVER_PORT;
 import static org.chromium.chrome.test.util.ChromeRestriction.RESTRICTION_TYPE_VIEWER_DAYDREAM;
 
 import android.support.test.filters.MediumTest;
@@ -55,8 +54,6 @@ public class VrBrowserJavaScriptModalDialogTest {
     @Before
     public void setUp() {
         mActivity = mActivityTestRule.getActivity();
-        // Need to ensure that the SERVER_PORT used is fixed for RenderTests
-        mActivityTestRule.getEmbeddedTestServerRule().setServerPort(SERVER_PORT);
         mVrBrowserTestFramework = new VrBrowserTestFramework(mActivityTestRule);
         mVrBrowserTestFramework.loadFileAndAwaitInitialization(
                 "2d_permission_page", PAGE_LOAD_TIMEOUT_S);