Android: move to CVC onShow() / onHide() for child process binding management.

Existing process management logic adjusts child process bindings in response to
ContentViewCore onAttachedToWindow() / onDetachedFromWindow() calls. This
causes some inconveniences: views in the embedding application sometimes are
detached for short-lived animations; but more importantly, we now have
embedders that do not alter the view hierarchy at all, preventing any
management of their renderer bindings

This patch moves to onShow() / onHide() calls instead. These are called by the
embedding applications independently from the view hierarchy changes.

In order to keep the most recently used renderer strongly bound during the
background period (as it was the case under onAttachedToWindow() /
onDetachedFromWindow() calls), explicit logic is introduced in the
BindingManager.

Also, the delay for removing strong bindings is brought from 5s to 1s as we no
longer face the interference of views detaching for short-lived animations (as
it was the case when we were relying on onAttachedToWindow() /
onDetachedFromWindow()).

BUG=317963

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237333 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 73 insertions, 19 deletions

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/TabBase.java b/chrome/android/java/src/org/chromium/chrome/browser/TabBase.java
index 5bf4fac..f809409 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/TabBase.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/TabBase.java
@@ -704,6 +704,7 @@ public abstract class TabBase implements NavigationClient {
     /** This is currently called when committing a pre-rendered page. */
     @CalledByNative
     private void swapWebContents(final long newWebContents) {
+        if (mContentViewCore != null) mContentViewCore.onHide();
         destroyContentView(false);
         NativePage previousNativePage = mNativePage;
         mNativePage = null;
diff --git a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
index d8ce233..8ba084b 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
@@ -221,9 +221,9 @@ public class ChildProcessLauncher {
         // Delay of 1 second used when removing the initial oom binding of a process.
         private static final long REMOVE_INITIAL_BINDING_DELAY_MILLIS = 1 * 1000;
 
-        // Delay of 5 second used when removing temporary strong binding of a process (only on
+        // Delay of 1 second used when removing temporary strong binding of a process (only on
         // non-low-memory devices).
-        private static final long DETACH_AS_ACTIVE_HIGH_END_DELAY_MILLIS = 5 * 1000;
+        private static final long DETACH_AS_ACTIVE_HIGH_END_DELAY_MILLIS = 1 * 1000;
 
         // Map from pid to the count of oom bindings bound for the service. Should be accessed with
         // mCountLock.
@@ -234,6 +234,10 @@ public class ChildProcessLauncher {
         // one renderer process at a time is oom bound. Should be accessed with mCountLock.
         private int mLastOomPid = -1;
 
+        // Pid of the renderer that we bound with a strong binding for the background period. Equals
+        // -1 when the embedder is in foreground.
+        private int mBoundForBackgroundPeriodPid = -1;
+
         // Should be acquired before binding or unbinding the connections and modifying state
         // variables: mOomBindingCount and mLastOomPid.
         private final Object mCountLock = new Object();
@@ -387,6 +391,39 @@ public class ChildProcessLauncher {
                 return mOomBindingCount.get(pid) > 0;
             }
         }
+
+        /**
+         * Called when the embedding application is sent to background. We want to maintain a strong
+         * binding on the most recently used renderer while the embedder is in background, to
+         * indicate the relative importance of the renderer to system oom killer.
+         *
+         * The embedder needs to ensure that:
+         *  - every onBroughtToForeground() is followed by onSentToBackground()
+         *  - pairs of consecutive onBroughtToForeground() / onSentToBackground() calls do not
+         *    overlap
+         */
+        void onSentToBackground() {
+            assert mBoundForBackgroundPeriodPid == -1;
+            // mLastOomPid can be -1 at this point as the embedding application could be used in
+            // foreground without spawning any renderers.
+            if (mLastOomPid >= 0) {
+                bindAsHighPriority(mLastOomPid);
+                mBoundForBackgroundPeriodPid = mLastOomPid;
+            }
+        }
+
+        /**
+         * Called when the embedding application is brought to foreground. This will drop the strong
+         * binding kept on the main renderer during the background period, so the embedder should
+         * make sure that this is called after the regular strong binding is attached for the
+         * foreground session.
+         */
+        void onBroughtToForeground() {
+            if (mBoundForBackgroundPeriodPid >= 0) {
+                unbindAsHighPriority(mBoundForBackgroundPeriodPid);
+                mBoundForBackgroundPeriodPid = -1;
+            }
+        }
     }
 
     private static BindingManager sBindingManager = new BindingManager();
@@ -401,6 +438,20 @@ public class ChildProcessLauncher {
     }
 
     /**
+     * Called when the embedding application is sent to background.
+     */
+    public static void onSentToBackground() {
+        sBindingManager.onSentToBackground();
+    }
+
+    /**
+     * Called when the embedding application is brought to foreground.
+     */
+    public static void onBroughtToForeground() {
+        sBindingManager.onBroughtToForeground();
+    }
+
+    /**
      * Returns the child process service interface for the given pid. This may be called on
      * any thread, but the caller must assume that the service can disconnect at any time. All
      * service calls should catch and handle android.os.RemoteException.
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index e24c48b..05e5d0b 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -366,7 +366,7 @@ public class ContentViewCore
     // Native pointer to C++ ContentViewCoreImpl object which will be set by nativeInit().
     private long mNativeContentViewCore = 0;
 
-    private boolean mAttachedToWindow = false;
+    private boolean mInForeground = false;
 
     private ContentViewGestureHandler mContentViewGestureHandler;
     private GestureStateListener mGestureStateListener;
@@ -1516,6 +1516,17 @@ public class ContentViewCore
      * To be called when the ContentView is shown.
      */
     public void onShow() {
+        assert mNativeContentViewCore != 0;
+        if (!mInForeground) {
+            int pid = nativeGetCurrentRenderProcessId(mNativeContentViewCore);
+            ChildProcessLauncher.getBindingManager().bindAsHighPriority(pid);
+            // Normally the initial binding is removed in onRenderProcessSwap(), but it is possible
+            // to construct WebContents and spawn the renderer before passing it to ContentViewCore.
+            // In this case there will be no onRenderProcessSwap() call and the initial binding will
+            // be removed here.
+            ChildProcessLauncher.getBindingManager().removeInitialBinding(pid);
+        }
+        mInForeground = true;
         nativeOnShow(mNativeContentViewCore);
         setAccessibilityState(mAccessibilityManager.isEnabled());
     }
@@ -1524,6 +1535,12 @@ public class ContentViewCore
      * To be called when the ContentView is hidden.
      */
     public void onHide() {
+        assert mNativeContentViewCore != 0;
+        if (mInForeground) {
+            int pid = nativeGetCurrentRenderProcessId(mNativeContentViewCore);
+            ChildProcessLauncher.getBindingManager().unbindAsHighPriority(pid);
+        }
+        mInForeground = false;
         hidePopupDialog();
         setInjectedAccessibility(false);
         nativeOnHide(mNativeContentViewCore);
@@ -1583,16 +1600,6 @@ public class ContentViewCore
      */
     @SuppressWarnings("javadoc")
     public void onAttachedToWindow() {
-        mAttachedToWindow = true;
-        if (mNativeContentViewCore != 0) {
-            int pid = nativeGetCurrentRenderProcessId(mNativeContentViewCore);
-            ChildProcessLauncher.getBindingManager().bindAsHighPriority(pid);
-            // Normally the initial binding is removed in onRenderProcessSwap(), but it is possible
-            // to construct WebContents and spawn the renderer before passing it to ContentViewCore.
-            // In this case there will be no onRenderProcessSwap() call and the initial binding will
-            // be removed here.
-            ChildProcessLauncher.getBindingManager().removeInitialBinding(pid);
-        }
         setAccessibilityState(mAccessibilityManager.isEnabled());
     }
 
@@ -1601,11 +1608,6 @@ public class ContentViewCore
      */
     @SuppressWarnings("javadoc")
     public void onDetachedFromWindow() {
-        mAttachedToWindow = false;
-        if (mNativeContentViewCore != 0) {
-            int pid = nativeGetCurrentRenderProcessId(mNativeContentViewCore);
-            ChildProcessLauncher.getBindingManager().unbindAsHighPriority(pid);
-        }
         setInjectedAccessibility(false);
         hidePopupDialog();
         mZoomControlsDelegate.dismissZoomPicker();
@@ -2645,7 +2647,7 @@ public class ContentViewCore
     @SuppressWarnings("unused")
     @CalledByNative
     private void onRenderProcessSwap(int oldPid, int newPid) {
-        if (mAttachedToWindow && oldPid != newPid) {
+        if (mInForeground && oldPid != newPid) {
             ChildProcessLauncher.getBindingManager().unbindAsHighPriority(oldPid);
             ChildProcessLauncher.getBindingManager().bindAsHighPriority(newPid);
         }