Fire SystemMonitor::{RESUME,SUSPEND}_EVENT on Android.

This improves SystemMonitor on Android by firing RESUME/SUSPEND events when the
Android main activity goes to foreground/background.
This lets the Android port use the same code path as other platforms to support
the close idle network connections functionnality.

SUSPEND events are now fired 1 minute after the main activity goes to
background.

Additionnally this CL cleans up ActivityStatus used by the Java side
SystemMonitor class. ActivityStatus was suffering from various refactorings and
was providing a counter intuitive interface with the Listener/StateListener
duality.
In particular it was possible to call SystemMonitor.onPause/Resume() as opposed
to onStateChange(). This could lead to SystemMonitor.getActivity() returning
null. The issue was raised while this CL was prepared.

BUG=164495

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

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

8 files changed, 72 insertions, 170 deletions

diff --git a/base/android/java/src/org/chromium/base/ActivityStatus.java b/base/android/java/src/org/chromium/base/ActivityStatus.java
index b80c149..e728870 100644
--- a/base/android/java/src/org/chromium/base/ActivityStatus.java
+++ b/base/android/java/src/org/chromium/base/ActivityStatus.java
@@ -25,24 +25,23 @@ public class ActivityStatus {
     // Current main activity, or null if none.
     private static Activity sActivity;
 
-    // Current main activity's state. This can be set even if sActivity
-    // is null, to simplify unit testing.
+    // Current main activity's state. This can be set even if sActivity is null, to simplify unit
+    // testing.
     private static int sActivityState;
 
-    // Current activity instance, or null.
-    private static ActivityStatus sInstance;
+    private static final ArrayList<StateListener> sStateListeners = new ArrayList<StateListener>();
 
-    // List of pause/resume listeners.
-    private final ArrayList<Listener> mListeners;
-
-    // List of state listeners.
-    private final ArrayList<StateListener> mStateListeners;
-
-    protected ActivityStatus() {
-        mListeners = new ArrayList<Listener>();
-        mStateListeners = new ArrayList<StateListener>();
+    // Use this interface to listen to all state changes.
+    public interface StateListener {
+        /**
+         * Called when the activity's state changes.
+         * @param newState New activity state.
+         */
+        public void onActivityStateChange(int newState);
     }
 
+    private ActivityStatus() {}
+
     /**
      * Must be called by the main activity when it changes state.
      * @param activity Current activity.
@@ -53,76 +52,18 @@ public class ActivityStatus {
             sActivity = activity;
         }
         sActivityState = newState;
-        getInstance().changeState(newState);
-
-        if (newState == DESTROYED) {
-            sActivity = null;
-        }
-    }
-
-    private void changeState(int newState) {
-        for (StateListener listener : mStateListeners) {
+        for (StateListener listener : sStateListeners) {
             listener.onActivityStateChange(newState);
         }
-        if (newState == PAUSED || newState == RESUMED) {
-            boolean paused = (newState == PAUSED);
-            for (Listener listener : mListeners) {
-                listener.onActivityStatusChanged(paused);
-            }
-        }
-    }
-
-    // This interface can only be used to listen to PAUSED and RESUMED
-    // events. Deprecated, new code should use StateListener instead.
-    // TODO(digit): Remove once all users have switched to StateListener.
-    @Deprecated
-    public interface Listener {
-        /**
-         * Called when the activity is paused or resumed only.
-         * @param isPaused true if the activity is paused, false if not.
-         */
-        public void onActivityStatusChanged(boolean isPaused);
-    }
-
-    // Use this interface to listen to all state changes.
-    public interface StateListener {
-        /**
-         * Called when the activity's state changes.
-         * @param newState New activity state.
-         */
-        public void onActivityStateChange(int newState);
-    }
-
-    /**
-     * Returns the current ActivityStatus instance.
-     */
-    public static ActivityStatus getInstance() {
-        // Can only be called on the UI thread.
-        assert Looper.myLooper() == Looper.getMainLooper();
-        if (sInstance == null) {
-            sInstance = new ActivityStatus();
+        if (newState == DESTROYED) {
+            sActivity = null;
         }
-        return sInstance;
-    }
-
-    /**
-     * Indicates that the parent activity was paused.
-     */
-    public void onPause() {
-       changeState(PAUSED);
-    }
-
-    /**
-     * Indicates that the parent activity was resumed.
-     */
-    public void onResume() {
-        changeState(RESUMED);
     }
 
     /**
      * Indicates that the parent activity is currently paused.
      */
-    public boolean isPaused() {
+    public static boolean isPaused() {
         return sActivityState == PAUSED;
     }
 
@@ -137,35 +78,15 @@ public class ActivityStatus {
      * Returns the current main application activity's state.
      */
     public static int getState() {
-        // To simplify unit testing, don't check sActivity for null.
         return sActivityState;
     }
 
     /**
-     * Registers the given pause/resume listener to receive activity
-     * status updates. Use registerStateListener() instead.
-     * @param listener Listener to receive status updates.
-     */
-    public void registerListener(Listener listener) {
-        mListeners.add(listener);
-    }
-
-    /**
-     * Unregisters the given pause/resume listener from receiving activity
-     * status updates. Use unregisterStateListener() instead.
-     * @param listener Listener that doesn't want to receive status updates.
-     */
-    public void unregisterListener(Listener listener) {
-        mListeners.remove(listener);
-    }
-
-    /**
      * Registers the given listener to receive activity state changes.
      * @param listener Listener to receive state changes.
      */
     public static void registerStateListener(StateListener listener) {
-        ActivityStatus status = getInstance();
-        status.mStateListeners.add(listener);
+        sStateListeners.add(listener);
     }
 
     /**
@@ -173,7 +94,6 @@ public class ActivityStatus {
      * @param listener Listener that doesn't want to receive state changes.
      */
     public static void unregisterStateListener(StateListener listener) {
-        ActivityStatus status = getInstance();
-        status.mStateListeners.remove(listener);
+        sStateListeners.remove(listener);
     }
 }
diff --git a/base/android/java/src/org/chromium/base/ChromiumActivity.java b/base/android/java/src/org/chromium/base/ChromiumActivity.java
index f857e12..65f5ce9 100644
--- a/base/android/java/src/org/chromium/base/ChromiumActivity.java
+++ b/base/android/java/src/org/chromium/base/ChromiumActivity.java
@@ -7,9 +7,8 @@ package org.chromium.base;
 import android.app.Activity;
 import android.os.Bundle;
 
-// All Chromium main activities should extend this class.
-// This allows various sub-systems to properly react to activity
-// state changes.
+// All Chromium main activities should extend this class. This allows various sub-systems to
+// properly react to activity state changes.
 public class ChromiumActivity extends Activity {
 
   @Override
@@ -27,18 +26,18 @@ public class ChromiumActivity extends Activity {
   @Override
   protected void onResume() {
     super.onResume();
-    ActivityStatus.onStateChange(this, ActivityStatus.STARTED);
+    ActivityStatus.onStateChange(this, ActivityStatus.RESUMED);
   }
 
   @Override
   protected void onPause() {
-    ActivityStatus.onStateChange(this, ActivityStatus.STARTED);
+    ActivityStatus.onStateChange(this, ActivityStatus.PAUSED);
     super.onPause();
   }
 
   @Override
   protected void onStop() {
-    ActivityStatus.onStateChange(this, ActivityStatus.STARTED);
+    ActivityStatus.onStateChange(this, ActivityStatus.STOPPED);
     super.onStop();
   }
 
diff --git a/base/android/java/src/org/chromium/base/SystemMonitor.java b/base/android/java/src/org/chromium/base/SystemMonitor.java
index f43f115..30f61a6 100644
--- a/base/android/java/src/org/chromium/base/SystemMonitor.java
+++ b/base/android/java/src/org/chromium/base/SystemMonitor.java
@@ -4,10 +4,12 @@
 
 package org.chromium.base;
 
+import android.app.Activity;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.os.BatteryManager;
+import android.os.Handler;
 import android.os.Looper;
 
 
@@ -15,22 +17,35 @@ import android.os.Looper;
  * Integrates native SystemMonitor with the java side.
  */
 @JNINamespace("base::android")
-public class SystemMonitor {
+public class SystemMonitor implements ActivityStatus.StateListener {
+    private static final long SUSPEND_DELAY_MS = 1 * 60 * 1000;  // 1 minute.
     private static SystemMonitor sInstance;
 
     private boolean mIsBatteryPower;
+    private final Handler mHandler = new Handler(Looper.getMainLooper());
+
+    // Asynchronous task used to fire the "paused" event to the native side 1 minute after the main
+    // activity transitioned to the "paused" state. This event is not sent immediately because it
+    // would be too aggressive. An Android activity can be in the "paused" state quite often. This
+    // can happen when a dialog window shows up for instance.
+    private static final Runnable sSuspendTask = new Runnable() {
+            @Override
+            public void run() {
+                nativeOnMainActivitySuspended();
+            }
+        };
 
     public static void createForTests(Context context) {
-        // Applications will create this once the
-        // JNI side has been fully wired up both sides.
-        // For tests, we just need native -> java, that is,
-        // we don't need to notify java -> native on creation.
+        // Applications will create this once the JNI side has been fully wired up both sides. For
+        // tests, we just need native -> java, that is, we don't need to notify java -> native on
+        // creation.
         sInstance = new SystemMonitor();
     }
 
     public static void create(Context context) {
         if (sInstance == null) {
             sInstance = new SystemMonitor();
+            ActivityStatus.registerStateListener(sInstance);
             IntentFilter ifilter = new IntentFilter(Intent.ACTION_BATTERY_CHANGED);
             Intent batteryStatusIntent = context.registerReceiver(null, ifilter);
             onBatteryChargingChanged(batteryStatusIntent);
@@ -42,9 +57,8 @@ public class SystemMonitor {
 
     public static void onBatteryChargingChanged(Intent intent) {
         if (sInstance == null) {
-            // We may be called by the framework intent-filter before being
-            // fully initialized. This is not a problem, since our constructor
-            // will check for the state later on.
+            // We may be called by the framework intent-filter before being fully initialized. This
+            // is not a problem, since our constructor will check for the state later on.
             return;
         }
         int chargePlug = intent.getIntExtra(BatteryManager.EXTRA_PLUGGED, -1);
@@ -54,11 +68,23 @@ public class SystemMonitor {
         nativeOnBatteryChargingChanged();
     }
 
+    @Override
+    public void onActivityStateChange(int newState) {
+        if (newState == ActivityStatus.RESUMED) {
+            // Remove the callback from the message loop in case it hasn't been executed yet.
+            mHandler.removeCallbacks(sSuspendTask);
+            nativeOnMainActivityResumed();
+        } else if (newState == ActivityStatus.PAUSED) {
+            mHandler.postDelayed(sSuspendTask, SUSPEND_DELAY_MS);
+        }
+    }
+
     @CalledByNative
     private static boolean isBatteryPower() {
         return sInstance.mIsBatteryPower;
     }
 
     private static native void nativeOnBatteryChargingChanged();
-
+    private static native void nativeOnMainActivitySuspended();
+    private static native void nativeOnMainActivityResumed();
 }
diff --git a/base/system_monitor/system_monitor_android.cc b/base/system_monitor/system_monitor_android.cc
index 4e1a16b..f29b7b0 100644
--- a/base/system_monitor/system_monitor_android.cc
+++ b/base/system_monitor/system_monitor_android.cc
@@ -10,11 +10,18 @@ namespace base {
 namespace android {
 
 // Native implementation of SystemMonitor.java.
-void OnBatteryChargingChanged(JNIEnv* env,
-                              jclass clazz) {
+void OnBatteryChargingChanged(JNIEnv* env, jclass clazz) {
   SystemMonitor::Get()->ProcessPowerMessage(SystemMonitor::POWER_STATE_EVENT);
 }
 
+void OnMainActivityResumed(JNIEnv* env, jclass clazz) {
+  SystemMonitor::Get()->ProcessPowerMessage(SystemMonitor::RESUME_EVENT);
+}
+
+void OnMainActivitySuspended(JNIEnv* env, jclass clazz) {
+  SystemMonitor::Get()->ProcessPowerMessage(SystemMonitor::SUSPEND_EVENT);
+}
+
 }  // namespace android
 
 bool SystemMonitor::IsBatteryPower() {
diff --git a/build/android/findbugs_filter/findbugs_known_bugs.txt b/build/android/findbugs_filter/findbugs_known_bugs.txt
index ce0bf4e..b46bfd3 100644
--- a/build/android/findbugs_filter/findbugs_known_bugs.txt
+++ b/build/android/findbugs_filter/findbugs_known_bugs.txt
@@ -100,6 +100,7 @@ M D UuF: Unused public or protected field: org.chromium.content.browser.JavaBrid
 M M IS: Inconsistent synchronization of org.chromium.content.browser.SandboxedProcessConnection.mPID; locked 66% of time  Unsynchronized access at SandboxedProcessConnection.java
 M M IS: Inconsistent synchronization of org.chromium.content.browser.SandboxedProcessConnection.mService; locked 55% of time  Unsynchronized access at SandboxedProcessConnection.java
 M M IS: Inconsistent synchronization of org.chromium.content.browser.SandboxedProcessConnection.mServiceConnectComplete; locked 60% of time  Unsynchronized access at SandboxedProcessConnection.java
+M M LI: Incorrect lazy initialization and update of static field org.chromium.base.SystemMonitor.sInstance in org.chromium.base.SystemMonitor.create(Context)  At SystemMonitor.java
 M M LI: Incorrect lazy initialization and update of static field org.chromium.content.browser.ContentVideoView.sContentVideoView in org.chromium.content.browser.ContentVideoView.createContentVideoView(int)  At ContentVideoView.java
 M M LI: Incorrect lazy initialization and update of static field org.chromium.net.test.util.TestWebServer.sReasons in org.chromium.net.test.util.TestWebServer.createResponse(int)  At TestWebServer.java
 M M LI: Incorrect lazy initialization of static field org.chromium.net.NetworkChangeNotifier.sInstance in org.chromium.net.NetworkChangeNotifier.createInstance(Context, int)  At NetworkChangeNotifier.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ProcessUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/ProcessUtils.java
index 2a9f1f3..419a033 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ProcessUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ProcessUtils.java
@@ -6,6 +6,8 @@ package org.chromium.chrome.browser;
 
 /**
  * A set of utility methods related to the various Chrome processes.
+ * TODO(pliard): Remove this class when JavaScript timers toggling is handled directly on the native
+ * side by using the system monitor.
  */
 public class ProcessUtils {
     // To prevent this class from being instantiated.
@@ -21,15 +23,5 @@ public class ProcessUtils {
         nativeToggleWebKitSharedTimers(suspend);
     }
 
-    /**
-     * We have keep-alives enabled for network connections as without it some routers will
-     * kill the connection, causing web pages to hang. This call closes such
-     * idle-but-kept-alive connections.
-     */
-    public static void closeIdleConnections() {
-        nativeCloseIdleConnections();
-    }
-
     private static native void nativeToggleWebKitSharedTimers(boolean suspend);
-    private static native void nativeCloseIdleConnections();
 }
diff --git a/chrome/browser/android/process_utils.cc b/chrome/browser/android/process_utils.cc
index 916b0c3..f308926 100644
--- a/chrome/browser/android/process_utils.cc
+++ b/chrome/browser/android/process_utils.cc
@@ -8,39 +8,12 @@
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/render_messages.h"
-#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "jni/ProcessUtils_jni.h"
-#include "net/http/http_network_session.h"
-#include "net/http/http_transaction_factory.h"
-#include "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_context_getter.h"
 
 namespace {
 
-void CloseIdleConnectionsForProfile(
-    scoped_refptr<net::URLRequestContextGetter> context_getter) {
-  DCHECK(context_getter.get());
-  if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)) {
-    content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
-                           base::Bind(&CloseIdleConnectionsForProfile,
-                                      context_getter));
-    return;
-  }
-
-  net::URLRequestContext* context = context_getter->GetURLRequestContext();
-  if (!context)
-    return;
-  net::HttpNetworkSession* session =
-      context->http_transaction_factory()->GetSession();
-  if (session)
-    session->CloseIdleConnections();
-}
-
 // Only accessed from the JNI thread by ToggleWebKitSharedTimers() which is
 // implemented below.
 base::LazyInstance<std::vector<int /* process id */> > g_suspended_processes =
@@ -81,23 +54,6 @@ static void ToggleWebKitSharedTimers(JNIEnv* env,
   }
 }
 
-static void CloseIdleConnections(JNIEnv* env, jclass obj) {
-  // Iterate through all loaded profiles (and their associated incognito
-  // profiles if created), and close the idle connections associated with each
-  // one.
-  std::vector<Profile*> profiles(
-      g_browser_process->profile_manager()->GetLoadedProfiles());
-  for (std::vector<Profile*>::iterator i = profiles.begin();
-      i != profiles.end(); i++) {
-    Profile* profile = *i;
-    CloseIdleConnectionsForProfile(profile->GetRequestContext());
-    if (profile->HasOffTheRecordProfile()) {
-      CloseIdleConnectionsForProfile(
-          profile->GetOffTheRecordProfile()->GetRequestContext());
-    }
-  }
-}
-
 bool RegisterProcessUtils(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
diff --git a/content/public/android/java/src/org/chromium/content/browser/LocationProvider.java b/content/public/android/java/src/org/chromium/content/browser/LocationProvider.java
index 6efcb95..d1c5e5a 100644
--- a/content/public/android/java/src/org/chromium/content/browser/LocationProvider.java
+++ b/content/public/android/java/src/org/chromium/content/browser/LocationProvider.java
@@ -49,6 +49,7 @@ class LocationProvider {
             mContext = context;
         }
 
+        @Override
         public void onActivityStateChange(int state) {
             if (state == ActivityStatus.PAUSED) {
                 mShouldRunAfterActivityResume = mIsRunning;
@@ -71,7 +72,7 @@ class LocationProvider {
                 ActivityStatus.registerStateListener(this);
             }
             mIsGpsEnabled = gpsEnabled;
-            if (ActivityStatus.getInstance().isPaused()) {
+            if (ActivityStatus.isPaused()) {
                 mShouldRunAfterActivityResume = true;
             } else {
                 unregisterFromLocationUpdates();