aw: Avoid leaking WindowAndroid in tests

Overall goal here is to ensure WindowAndroid instances are destroyed at
the end of instrumentation tests, but there is change in behavior for
production code.

Eager destroy WindowAndroid in AwContents. When the destroy runnable for
all AwContents that uses a WindowAndroid runs. Need to ensure at this
time that the WindowAndroid is removed from the global map as well. This
works because WindowAndroid lifetime and destroy runnable lifetime are
essentially the same.

Then in AwActivityTestRule, keep track of all AwContents created in
instrumentation tests in a list of weak references, and destroy at the
end of the test. Calling AwContents.destroy multiple is ok, so this
should be safe.

Need specific handling for AwUncaughtExceptionTest because it renders
the UI thread useless at the end of the test. Instead synchronously
destroy the AwContents in the test body itself.

Bug: 1081250
Change-Id: Ic07e52c7b21bf1300942e8b0a7cf178b8e3858b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2205879
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771078}

android_webview/java/src/org/chromium/android_webview/AwContents.java

@@ -520,11 +520,13 @@ public class AwContents implements SmartClipProvider {
                 long nativeAwContents, WindowAndroidWrapper windowAndroid) {
             mNativeAwContents = nativeAwContents;
             mWindowAndroid = windowAndroid;
+            mWindowAndroid.incrementRefFromDestroyRunnable();
         }
 
         @Override
         public void run() {
             AwContentsJni.get().destroy(mNativeAwContents);
+            mWindowAndroid.decrementRefFromDestroyRunnable();
         }
     }
 
@@ -1131,6 +1133,11 @@ public class AwContents implements SmartClipProvider {
         private final WindowAndroid mWindowAndroid;
         private final CleanupReference mCleanupReference;
 
+        // This ref-counts is used only to destroy WindowAndroid eagerly
+        // when AwContents is destroyed. The CleanupReference is still used
+        // if a Wrapper is created without any AwContents.
+        private int mRefFromAwContentsDestroyRunnable;
+
         private static final class DestroyRunnable implements Runnable {
             private final WindowAndroid mWindowAndroid;
             private DestroyRunnable(WindowAndroid windowAndroid) {
@@ -1153,6 +1160,26 @@ public class AwContents implements SmartClipProvider {
         public WindowAndroid getWindowAndroid() {
             return mWindowAndroid;
         }
+
+        public void incrementRefFromDestroyRunnable() {
+            mRefFromAwContentsDestroyRunnable++;
+        }
+
+        public void decrementRefFromDestroyRunnable() {
+            assert mRefFromAwContentsDestroyRunnable > 0;
+            mRefFromAwContentsDestroyRunnable--;
+            maybeCleanupEarly();
+        }
+
+        private void maybeCleanupEarly() {
+            if (mRefFromAwContentsDestroyRunnable != 0) return;
+
+            Context context = mWindowAndroid.getContext().get();
+            if (context != null && sContextWindowMap.get(context) != this) return;
+
+            mCleanupReference.cleanupNow();
+            if (context != null) sContextWindowMap.remove(context);
+        }
     }
     private static WeakHashMap<Context, WindowAndroidWrapper> sContextWindowMap;
 
@@ -1441,7 +1468,8 @@ public class AwContents implements SmartClipProvider {
     /**
      * Deletes the native counterpart of this object.
      */
-    private void destroyNatives() {
+    @VisibleForTesting
+    public void destroyNatives() {
         if (mCleanupReference != null) {
             assert mNativeAwContents != 0;
 

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

@@ -37,6 +37,9 @@ import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.net.test.util.TestWebServer;
 
 import java.lang.annotation.Annotation;
+import java.lang.ref.WeakReference;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Callable;
@@ -70,6 +73,8 @@ public class AwActivityTestRule extends ActivityTestRule<AwTestRunnerActivity> {
     // The browser context needs to be a process-wide singleton.
     private AwBrowserContext mBrowserContext;
 
+    private List<WeakReference<AwContents>> mAwContentsDestroyedInTearDown = new ArrayList<>();
+
     public AwActivityTestRule() {
         super(AwTestRunnerActivity.class, /* initialTouchMode */ false, /* launchActivity */ false);
     }
@@ -82,6 +87,7 @@ public class AwActivityTestRule extends ActivityTestRule<AwTestRunnerActivity> {
             public void evaluate() throws Throwable {
                 setUp();
                 base.evaluate();
+                tearDown();
             }
         }, description);
     }
@@ -95,6 +101,20 @@ public class AwActivityTestRule extends ActivityTestRule<AwTestRunnerActivity> {
         }
     }
 
+    public void tearDown() {
+        if (!needsAwContentsCleanup()) return;
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            for (WeakReference<AwContents> awContentsRef : mAwContentsDestroyedInTearDown) {
+                AwContents awContents = awContentsRef.get();
+                if (awContents == null) continue;
+                awContents.destroy();
+            }
+        });
+        // Flush the UI queue since destroy posts again to UI thread.
+        TestThreadUtils.runOnUiThreadBlocking(() -> { mAwContentsDestroyedInTearDown.clear(); });
+    }
+
     public AwTestRunnerActivity launchActivity() {
         if (getActivity() == null) {
             return launchActivity(null);
@@ -130,6 +150,14 @@ public class AwActivityTestRule extends ActivityTestRule<AwTestRunnerActivity> {
         return true;
     }
 
+    /**
+     * Override this to return false if test doesn't need all AwContents to be
+     * destroyed explicitly after the test.
+     */
+    public boolean needsAwContentsCleanup() {
+        return true;
+    }
+
     public void createAwBrowserContext() {
         if (mBrowserContext != null) {
             throw new AndroidRuntimeException("There should only be one browser context.");
@@ -376,6 +404,7 @@ public class AwActivityTestRule extends ActivityTestRule<AwTestRunnerActivity> {
                 testContainerView.getNativeDrawFunctorFactory(), awContentsClient, awSettings,
                 testDependencyFactory);
         testContainerView.initialize(awContents);
+        mAwContentsDestroyedInTearDown.add(new WeakReference<>(awContents));
         return testContainerView;
     }
 

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

@@ -43,6 +43,13 @@ public class AwUncaughtExceptionTest {
         public boolean needsBrowserProcessStarted() {
             return false;
         }
+        @Override
+        public boolean needsAwContentsCleanup() {
+            // State of VM might be hosed after throwing and not catching exceptions.
+            // Do not assume it is safe to destroy AwContents by posting to the UI thread.
+            // Instead explicitly destroy any AwContents created in this test.
+            return false;
+        }
     };
 
     private class BackgroundThread extends Thread {
@@ -178,6 +185,7 @@ public class AwUncaughtExceptionTest {
             mContentsClient = new TestAwContentsClient() {
                 @Override
                 public boolean shouldOverrideUrlLoading(AwWebResourceRequest request) {
+                    mAwContents.destroyNatives();
                     throw new RuntimeException(msg);
                 }
             };