Allow EmbeddedTestServer reuse across batched tests.

Migrates EmbeddedTestServer to a TestRule-based implementation, which
allows it to be used as an @ClassRule that can be reused across batched
tests.

Also removes a bunch of unnecessary locking that probably wasn't
necessary in the first place, and almost certainly isn't necessary now.

Bug: 989569
Change-Id: I1b5454b02251c08cd085eb4c9d70859acc1ed29a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2300327
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789522}

net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServerRule.java

@@ -7,41 +7,38 @@ package org.chromium.net.test;
 import android.content.Context;
 import android.support.test.InstrumentationRegistry;
 
-import androidx.annotation.GuardedBy;
-
-import org.junit.rules.TestWatcher;
+import org.junit.rules.TestRule;
 import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
 
 /**
  * Junit4 rule for starting embedded test server when necessary (i.e. when accessed via
  * {@link #getServer()}), and shutting it down when the test finishes.
  */
-public class EmbeddedTestServerRule extends TestWatcher {
-    private final Object mLock = new Object();
-    @GuardedBy("mLock")
+public class EmbeddedTestServerRule implements TestRule {
     private EmbeddedTestServer mServer;
 
     // The default value of 0 will result in the same behavior as createAndStartServer
     // (auto-selected port).
-    @GuardedBy("mLock")
     private int mServerPort;
 
-    @GuardedBy("mLock")
     private boolean mUseHttps;
 
-    @GuardedBy("mLock")
     @ServerCertificate
     private int mCertificateType = ServerCertificate.CERT_OK;
 
     @Override
-    protected void finished(Description description) {
-        super.finished(description);
-        synchronized (mLock) {
-            if (mServer != null) {
-                mServer.stopAndDestroyServer();
-                mServer = null;
+    public Statement apply(Statement base, Description description) {
+        return new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                try {
+                    base.evaluate();
+                } finally {
+                    if (mServer != null) mServer.stopAndDestroyServer();
+                }
             }
-        }
+        };
     }
 
     /**
@@ -50,16 +47,14 @@ public class EmbeddedTestServerRule extends TestWatcher {
      * @return the test server.
      */
     public EmbeddedTestServer getServer() {
-        synchronized (mLock) {
-            if (mServer == null) {
-                Context context = InstrumentationRegistry.getContext();
-                mServer = mUseHttps
-                        ? EmbeddedTestServer.createAndStartHTTPSServerWithPort(
-                                context, mCertificateType, mServerPort)
-                        : EmbeddedTestServer.createAndStartServerWithPort(context, mServerPort);
-            }
-            return mServer;
+        if (mServer == null) {
+            Context context = InstrumentationRegistry.getContext();
+            mServer = mUseHttps
+                    ? EmbeddedTestServer.createAndStartHTTPSServerWithPort(
+                            context, mCertificateType, mServerPort)
+                    : EmbeddedTestServer.createAndStartServerWithPort(context, mServerPort);
         }
+        return mServer;
     }
 
     public String getOrigin() {
@@ -73,25 +68,19 @@ public class EmbeddedTestServerRule extends TestWatcher {
      * @param port the port to start the server with, or 0 for an automatically selected one.
      */
     public void setServerPort(int port) {
-        synchronized (mLock) {
-            assert mServer == null;
-            mServerPort = port;
-        }
+        assert mServer == null;
+        mServerPort = port;
     }
 
     /** Sets whether to create an HTTPS (vs HTTP) server. */
     public void setServerUsesHttps(boolean useHttps) {
-        synchronized (mLock) {
-            assert mServer == null;
-            mUseHttps = useHttps;
-        }
+        assert mServer == null;
+        mUseHttps = useHttps;
     }
 
     /** Sets what type of certificate the server uses when running as an HTTPS server. */
     public void setCertificateType(@ServerCertificate int certificateType) {
-        synchronized (mLock) {
-            assert mServer == null;
-            mCertificateType = certificateType;
-        }
+        assert mServer == null;
+        mCertificateType = certificateType;
     }
 }