Fix webview memory leak when keyboard was shown

Android framework does not seem to release ResultReceiver passed in
InputMethodManager#showSoftInput() after use. As a result, WebView
will not be garbage collected after it's removed, if keyboard shows up
on the screen once.

ResultReceiver cannot be avoided since we want to scroll to the editable
node only after input method window shows up.

This is a fix to weak-reference CVC object (and thus WebView as well) from
ResultReceiver. Unfortunately, ResultReceiver will still be leaked,
but the leak can be significantly reduced.

A test is added to AwContentsGarbageCollectionTest to test that references
to WebView can be removed even when showSoftInput() is called before
garbage collection.

Also, I've manually tested that the new test can catch the memory leak
issue when we strong-reference CVC.

BUG=595613

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

Cr-Commit-Position: refs/heads/master@{#381891}

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

@@ -4,13 +4,14 @@
 
 package org.chromium.android_webview.test;
 
+import static org.chromium.base.test.util.ScalableTimeout.scaleTimeout;
+
 import android.content.Context;
 import android.content.ContextWrapper;
+import android.os.ResultReceiver;
 import android.test.suitebuilder.annotation.LargeTest;
 import android.test.suitebuilder.annotation.SmallTest;
 
-import static org.chromium.base.test.util.ScalableTimeout.scaleTimeout;
-
 import org.chromium.android_webview.AwContents;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.test.util.Feature;
@@ -60,7 +61,7 @@ public class AwContentsGarbageCollectionTest extends AwTestBase {
     }
 
     private static class GcTestDependencyFactory extends TestDependencyFactory {
-        private StrongRefTestContext mContext;
+        private final StrongRefTestContext mContext;
 
         public GcTestDependencyFactory(StrongRefTestContext context) {
             mContext = context;
@@ -103,6 +104,42 @@ public class AwContentsGarbageCollectionTest extends AwTestBase {
         gcAndCheckAllAwContentsDestroyed();
     }
 
+    @DisableHardwareAccelerationForTest
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testHoldKeyboardResultReceiver() throws Throwable {
+        gcAndCheckAllAwContentsDestroyed();
+
+        TestAwContentsClient client = new TestAwContentsClient();
+        AwTestContainerView containerViews[] = new AwTestContainerView[MAX_IDLE_INSTANCES + 1];
+        ResultReceiver resultReceivers[] = new ResultReceiver[MAX_IDLE_INSTANCES + 1];
+        for (int i = 0; i < containerViews.length; i++) {
+            final AwTestContainerView containerView = createAwTestContainerViewOnMainSync(client);
+            containerViews[i] = containerView;
+            loadUrlAsync(containerView.getAwContents(), "about:blank");
+            // When we call showSoftInput(), we pass a ResultReceiver object as a parameter.
+            // Android framework will hold the object reference until the matching
+            // ResultReceiver in InputMethodService (IME app) gets garbage-collected.
+            // WebView object wouldn't get gc'ed once OSK shows up because of this.
+            // It is difficult to show keyboard and wait until input method window shows up.
+            // Instead, we simply emulate Android's behavior by keeping strong references.
+            // See crbug.com/595613 for details.
+            resultReceivers[i] = runTestOnUiThreadAndGetResult(new Callable<ResultReceiver>() {
+                @Override
+                public ResultReceiver call() throws Exception {
+                    return containerView.getContentViewCore().getNewShowKeyboardReceiver();
+                }
+            });
+        }
+
+        for (int i = 0; i < containerViews.length; i++) {
+            containerViews[i] = null;
+        }
+        containerViews = null;
+        removeAllViews();
+        gcAndCheckAllAwContentsDestroyed();
+    }
+
     @DisableHardwareAccelerationForTest
     @SmallTest
     @Feature({"AndroidWebView"})

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

@@ -361,6 +361,29 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Screen
         }
     }
 
+    /**
+     * {@ResultReceiver} passed in InputMethodManager#showSoftInput}. We need this to scroll to the
+     * editable node at the right timing, which is after input method window shows up.
+     */
+    private static class ShowKeyboardResultReceiver extends ResultReceiver {
+
+        // Unfortunately, ResultReceiver used in showSoftInput() will be leaked. We minimize
+        // the leak by weak referencing CVC and therefore WebView object.
+        private final WeakReference<ContentViewCore> mContentViewCore;
+
+        public ShowKeyboardResultReceiver(ContentViewCore contentViewCore, Handler handler) {
+            super(handler);
+            mContentViewCore = new WeakReference<>(contentViewCore);
+        }
+
+        @Override
+        public void onReceiveResult(int resultCode, Bundle resultData) {
+            ContentViewCore contentViewCore = mContentViewCore.get();
+            if (contentViewCore == null) return;
+            contentViewCore.onShowKeyboardReceiveResult(resultCode);
+        }
+    }
+
     /**
      * Interface that consumers of {@link ContentViewCore} must implement to allow the proper
      * dispatching of view methods through the containing view.
@@ -585,6 +608,10 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Screen
     // The client that implements Contextual Search functionality, or null if none exists.
     private ContextualSearchClient mContextualSearchClient;
 
+    // NOTE: This object will not be released by Android framework until the matching
+    // ResultReceiver in the InputMethodService (IME app) gets gc'ed.
+    private ShowKeyboardResultReceiver mShowKeyboardResultReceiver;
+
     /**
      * @param webContents The {@link WebContents} to find a {@link ContentViewCore} of.
      * @return            A {@link ContentViewCore} that is connected to {@code webContents} or
@@ -739,26 +766,7 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Screen
 
                     @Override
                     public ResultReceiver getNewShowKeyboardReceiver() {
-                        return new ResultReceiver(new Handler()) {
-                            @Override
-                            public void onReceiveResult(int resultCode, Bundle resultData) {
-                                if (resultCode == InputMethodManager.RESULT_SHOWN) {
-                                    // If OSK is newly shown, delay the form focus until
-                                    // the onSizeChanged (in order to adjust relative to the
-                                    // new size).
-                                    // TODO(jdduke): We should not assume that onSizeChanged will
-                                    // always be called, crbug.com/294908.
-                                    getContainerView().getWindowVisibleDisplayFrame(
-                                            mFocusPreOSKViewportRect);
-                                } else if (hasFocus() && resultCode
-                                        == InputMethodManager.RESULT_UNCHANGED_SHOWN) {
-                                    // If the OSK was already there, focus the form immediately.
-                                    if (mWebContents != null) {
-                                        mWebContents.scrollFocusedEditableNodeIntoView();
-                                    }
-                                }
-                            }
-                        };
+                        return ContentViewCore.this.getNewShowKeyboardReceiver();
                     }
                 });
     }
@@ -3307,6 +3315,36 @@ public class ContentViewCore implements AccessibilityStateChangeListener, Screen
         mContextualSearchClient = contextualSearchClient;
     }
 
+    /**
+     * Call this when we get result from ResultReceiver passed in calling showSoftInput().
+     * @param resultCode The result of showSoftInput() as defined in InputMethodManager.
+     */
+    public void onShowKeyboardReceiveResult(int resultCode) {
+        if (resultCode == InputMethodManager.RESULT_SHOWN) {
+            // If OSK is newly shown, delay the form focus until
+            // the onSizeChanged (in order to adjust relative to the
+            // new size).
+            // TODO(jdduke): We should not assume that onSizeChanged will
+            // always be called, crbug.com/294908.
+            getContainerView().getWindowVisibleDisplayFrame(
+                    mFocusPreOSKViewportRect);
+        } else if (hasFocus() && resultCode == InputMethodManager.RESULT_UNCHANGED_SHOWN) {
+            // If the OSK was already there, focus the form immediately.
+            if (mWebContents != null) {
+                mWebContents.scrollFocusedEditableNodeIntoView();
+            }
+        }
+    }
+
+    @VisibleForTesting
+    public ResultReceiver getNewShowKeyboardReceiver() {
+        if (mShowKeyboardResultReceiver == null) {
+            // Note: the returned object will get leaked by Android framework.
+            mShowKeyboardResultReceiver = new ShowKeyboardResultReceiver(this, new Handler());
+        }
+        return mShowKeyboardResultReceiver;
+    }
+
     private native long nativeInit(WebContents webContents, ViewAndroidDelegate viewAndroidDelegate,
             long windowAndroidPtr, HashSet<Object> retainedObjectSet);
     private static native ContentViewCore nativeFromWebContentsAndroid(WebContents webContents);