branch 2357: Fix leak from android Context

The Activity, which is normally the Context instance, holds the view
tree, which in turns can prevent native side from garbage collected. So
native code cannot hold strongly to the Context object.

There are two instances where Context is leaking:

1) Currently native holds ContentViewAndroidDelegate which holds
the Context. Stop holding the Context and just use get the context
for the container view, which is already held weakly.

This means acquireAnchorView can return null when the java peer
has been garbage collected, but native side has not been destroyed
yet. This is never a problem in chrome. This happens to not be a
problem in webview, because all callers of acquireAnchorView already
guarantees the ContentViewCore is still alive.

2) CaptionManager uses the Context (instead of Application Context)
and never unregisters it in WebView. This keeps a dangling reference
all the way to the Context used to register in Android framework
code.

In this CL, only fix the webview leak by using the Application Context.
There is still the problem that the CaptionManager is unregistered
which will be fixed in the future.

Modify the webview gc test to catch this case.

BUG=478719, 469803

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

Cr-Commit-Position: refs/heads/master@{#325961}
(cherry picked from commit 157d438)
TBR=boliu@chromium.org

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

Cr-Commit-Position: refs/branch-heads/2357@{#176}
Cr-Branched-From: 59d4494-refs/heads/master@{#323860}

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

@@ -4,6 +4,8 @@
 
 package org.chromium.android_webview.test;
 
+import android.content.Context;
+import android.content.ContextWrapper;
 import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.graphics.Canvas;
@@ -151,6 +153,45 @@ public void setAwContentsStrongRef(AwContents awContents) {
         }
     }
 
+    private TestDependencyFactory mOverridenFactory;
+
+    @Override
+    protected TestDependencyFactory createTestDependencyFactory() {
+        if (mOverridenFactory == null) {
+            return new TestDependencyFactory();
+        } else {
+            return mOverridenFactory;
+        }
+    }
+
+    @SuppressFBWarnings("URF_UNREAD_FIELD")
+    private static class StrongRefTestContext extends ContextWrapper {
+        private AwContents mAwContents;
+        public void setAwContentsStrongRef(AwContents awContents) {
+            mAwContents = awContents;
+        }
+
+        public StrongRefTestContext(Context context) {
+            super(context);
+        }
+    }
+
+    private static class GcTestDependencyFactory extends TestDependencyFactory {
+        private StrongRefTestContext mContext;
+
+        public GcTestDependencyFactory(StrongRefTestContext context) {
+            mContext = context;
+        }
+
+        @Override
+        public AwTestContainerView createAwTestContainerView(
+                AwTestRunnerActivity activity, boolean allowHardwareAcceleration) {
+            if (activity != mContext.getBaseContext()) fail();
+            return new AwTestContainerView(mContext, allowHardwareAcceleration);
+        }
+    }
+
+    // TODO(boliu): Refactor this test into its own file, then into separate tests.
     @DisableHardwareAccelerationForTest
     @LargeTest
     @Feature({"AndroidWebView"})
@@ -169,59 +210,69 @@ public Boolean call() {
                 return AwContents.getNativeInstanceCount() <= maxIdleInstances;
             }
         });
-        for (int i = 0; i < repetitions; ++i) {
-            for (int j = 0; j < concurrentInstances; ++j) {
-                final StrongRefTestAwContentsClient client = new StrongRefTestAwContentsClient();
-                AwTestContainerView view = createAwTestContainerViewOnMainSync(client);
-                // Embedding app can hold onto a strong ref to the WebView from either WebViewClient
-                // or WebChromeClient. That should not prevent WebView from gc-ed. We simulate that
-                // behavior by making the equivalent change here, have AwContentsClient hold a
-                // strong ref to the AwContents object.
-                client.setAwContentsStrongRef(view.getAwContents());
-                loadUrlAsync(view.getAwContents(), "about:blank");
-            }
-            assertTrue(AwContents.getNativeInstanceCount() >= concurrentInstances);
-            assertTrue(AwContents.getNativeInstanceCount() <= (i + 1) * concurrentInstances);
-            runTestOnUiThread(new Runnable() {
-                @Override
-                public void run() {
-                    getActivity().removeAllViews();
+
+        try {
+            for (int i = 0; i < repetitions; ++i) {
+                for (int j = 0; j < concurrentInstances; ++j) {
+                    final StrongRefTestAwContentsClient client =
+                            new StrongRefTestAwContentsClient();
+                    StrongRefTestContext context = new StrongRefTestContext(getActivity());
+                    mOverridenFactory = new GcTestDependencyFactory(context);
+                    AwTestContainerView view = createAwTestContainerViewOnMainSync(client);
+                    mOverridenFactory = null;
+                    // Embedding app can hold onto a strong ref to the WebView from either
+                    // WebViewClient or WebChromeClient. That should not prevent WebView from
+                    // gc-ed. We simulate that behavior by making the equivalent change here,
+                    // have AwContentsClient hold a strong ref to the AwContents object.
+                    client.setAwContentsStrongRef(view.getAwContents());
+                    context.setAwContentsStrongRef(view.getAwContents());
+                    loadUrlAsync(view.getAwContents(), "about:blank");
                 }
-            });
-        }
+                assertTrue(AwContents.getNativeInstanceCount() >= concurrentInstances);
+                assertTrue(AwContents.getNativeInstanceCount() <= (i + 1) * concurrentInstances);
+                runTestOnUiThread(new Runnable() {
+                    @Override
+                    public void run() {
+                        getActivity().removeAllViews();
+                    }
+                });
+            }
 
-        Runtime.getRuntime().gc();
+            Runtime.getRuntime().gc();
 
-        Criteria criteria = new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                try {
-                    return runTestOnUiThreadAndGetResult(new Callable<Boolean>() {
-                        @Override
-                        public Boolean call() {
-                            return AwContents.getNativeInstanceCount() <= maxIdleInstances;
-                        }
-                    });
-                } catch (Exception e) {
-                    return false;
+            Criteria criteria = new Criteria() {
+                @Override
+                public boolean isSatisfied() {
+                    try {
+                        return runTestOnUiThreadAndGetResult(new Callable<Boolean>() {
+                            @Override
+                            public Boolean call() {
+                                return AwContents.getNativeInstanceCount() <= maxIdleInstances;
+                            }
+                        });
+                    } catch (Exception e) {
+                        return false;
+                    }
+                }
+            };
+
+            // Depending on a single gc call can make this test flaky. It's possible
+            // that the WebView still has transient references during load so it does not get
+            // gc-ed in the one gc-call above. Instead call gc again if exit criteria fails to
+            // catch this case.
+            final long timeoutBetweenGcMs = scaleTimeout(1000);
+            for (int i = 0; i < 15; ++i) {
+                if (CriteriaHelper.pollForCriteria(criteria, timeoutBetweenGcMs, CHECK_INTERVAL)) {
+                    break;
+                } else {
+                    Runtime.getRuntime().gc();
                 }
             }
-        };
-
-        // Depending on a single gc call can make this test flaky. It's possible
-        // that the WebView still has transient references during load so it does not get
-        // gc-ed in the one gc-call above. Instead call gc again if exit criteria fails to
-        // catch this case.
-        final long timeoutBetweenGcMs = scaleTimeout(1000);
-        for (int i = 0; i < 15; ++i) {
-            if (CriteriaHelper.pollForCriteria(criteria, timeoutBetweenGcMs, CHECK_INTERVAL)) {
-                break;
-            } else {
-                Runtime.getRuntime().gc();
-            }
-        }
 
-        assertTrue(criteria.isSatisfied());
+            assertTrue(criteria.isSatisfied());
+        } finally {
+            mOverridenFactory = null;
+        }
     }
 
     @SmallTest

content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java

@@ -177,7 +177,6 @@ public Position(float x, float y, float width, float height) {
             }
         }
 
-        private final Context mContext;
         private final RenderCoordinates mRenderCoordinates;
 
         /**
@@ -194,21 +193,18 @@ public Position(float x, float y, float width, float height) {
          */
         private Map<View, Position> mAnchorViews = new LinkedHashMap<View, Position>();
 
-        ContentViewAndroidDelegate(
-                Context context, ViewGroup containerView, RenderCoordinates renderCoordinates) {
-            mContext = context;
+        ContentViewAndroidDelegate(ViewGroup containerView, RenderCoordinates renderCoordinates) {
             mRenderCoordinates = renderCoordinates;
             mCurrentContainerView = new WeakReference<>(containerView);
         }
 
         @Override
         public View acquireAnchorView() {
-            View anchorView = new View(mContext);
-            mAnchorViews.put(anchorView, null);
             ViewGroup containerView = mCurrentContainerView.get();
-            if (containerView != null) {
-                containerView.addView(anchorView);
-            }
+            if (containerView == null) return null;
+            View anchorView = new View(containerView.getContext());
+            mAnchorViews.put(anchorView, null);
+            containerView.addView(anchorView);
             return anchorView;
         }
 
@@ -233,7 +229,8 @@ private void doSetAnchorViewPosition(
             }
             assert view.getParent() == containerView;
 
-            float scale = (float) DeviceDisplayInfo.create(mContext).getDIPScale();
+            float scale =
+                    (float) DeviceDisplayInfo.create(containerView.getContext()).getDIPScale();
 
             // The anchor view should not go outside the bounds of the ContainerView.
             int leftMargin = Math.round(x * scale);
@@ -853,8 +850,7 @@ public void initialize(ViewGroup containerView, InternalAccessDelegate internalD
 
     @VisibleForTesting
     void createContentViewAndroidDelegate() {
-        mViewAndroidDelegate =
-                new ContentViewAndroidDelegate(mContext, mContainerView, mRenderCoordinates);
+        mViewAndroidDelegate = new ContentViewAndroidDelegate(mContainerView, mRenderCoordinates);
     }
 
     @VisibleForTesting

content/public/android/java/src/org/chromium/content/browser/accessibility/captioning/KitKatCaptioningBridge.java

@@ -60,8 +60,9 @@ public void onUserStyleChanged(CaptioningManager.CaptionStyle userStyle) {
      */
     public KitKatCaptioningBridge(ContentViewCore contenViewCore) {
         mCaptioningChangeDelegate = new CaptioningChangeDelegate(contenViewCore);
-        mCaptioningManager = (CaptioningManager) contenViewCore.getContext().getSystemService(
-                Context.CAPTIONING_SERVICE);
+        mCaptioningManager = (CaptioningManager) contenViewCore.getContext()
+                                     .getApplicationContext()
+                                     .getSystemService(Context.CAPTIONING_SERVICE);
         mCaptioningManager.addCaptioningChangeListener(mCaptioningChangeListener);
         syncToDelegate();
     }