[Android] Enable ApplicationStatus for WebView.

base/android/java/src/org/chromium/base/ApplicationStatus.java

Index: base/android/java/src/org/chromium/base/ApplicationStatus.java
diff --git a/base/android/java/src/org/chromium/base/ApplicationStatus.java b/base/android/java/src/org/chromium/base/ApplicationStatus.java
index 5035b9cabfb0f182d869d154ced60f290bdcccdc..a317b158fca4db48f6a9a5c1fb1d0b2560a8f6f4 100644
--- a/base/android/java/src/org/chromium/base/ApplicationStatus.java
+++ b/base/android/java/src/org/chromium/base/ApplicationStatus.java
@@ -63,10 +63,13 @@ public class ApplicationStatus {
     /** A lazily initialized listener that forwards application state changes to native. */
     private static ApplicationStateListener sNativeApplicationStateListener;
 
+    /** If true means we are running in 'webview' mode with somewhat different logic **/
+    private static boolean sWebViewMode = false;

[dgn, 2015/09/30 11:02:55]

Could you please add more explanations on how WebView mode is different? Why the
distinction is needed, what would not work, etc. Maybe add checks to throw in
methods not supported in webview mode? If it's only going to be used only for a
single thing, it's maybe better to have a more descriptive/focused name? (it
could be useful beyond webview some day?)

[timvolodine, 2015/10/01 15:49:29]

I've added explanations of differences in the initializeForWebView method and
made this comment point there. Also added some asserts specific to webview mode.

+
     /**
      * A map of which observers listen to state changes from which {@link Activity}.
      */
-    private static final Map<Activity, ActivityInfo> sActivityInfo =
+    private static final ConcurrentHashMap<Activity, ActivityInfo> sActivityInfo =
             new ConcurrentHashMap<Activity, ActivityInfo>();

[boliu, 2015/09/29 21:34:02]

dtrainor: why ConcurrentHashMap? Is this class not single threaded?

Also this honestly scares me for potential of leaks, but ehh...I guess it's fine
if listeners are working fine.

[David Trainor- moved to gerrit, 2015/09/29 23:02:11]

IIRC callers don't have to be on the UI thread.  The InvalidationService uses it
for example (see crbug.com/403951).

[boliu, 2015/09/29 23:21:45]

so this class is partially thread safe? The part about accessing the state of an
activity is thread safe, but the listeners are not o_O

(Base has a global thread safe object that doesn't work on android webview, :( )

[David Trainor- moved to gerrit, 2015/09/30 14:50:48]

Sadly yeah :(.  I think we made the smallest change we had to to fix a bug
before release but this apparently fell off my radar and I didn't follow up to
make the whole class thread safe :(.
?

 
     /**
@@ -109,28 +112,13 @@ public class ApplicationStatus {
 
     /**
      * Initializes the activity status for a specified application.
+     * This method only registers for ActivityLifecycle changes.
      *
      * @param application The application whose status you wish to monitor.
      */
-    public static void initialize(BaseChromiumApplication application) {
+    private static void initializeLifecycleCallbacks(Application application) {
         sApplication = application;
 
-        application.registerWindowFocusChangedListener(
-                new BaseChromiumApplication.WindowFocusChangedListener() {
-                    @Override
-                    public void onWindowFocusChanged(Activity activity, boolean hasFocus) {
-                        if (!hasFocus || activity == sActivity) return;
-
-                        int state = getStateForActivity(activity);
-
-                        if (state != ActivityState.DESTROYED && state != ActivityState.STOPPED) {
-                            sActivity = activity;
-                        }
-
-                        // TODO(dtrainor): Notify of active activity change?
-                    }
-                });
-
         application.registerActivityLifecycleCallbacks(new ActivityLifecycleCallbacks() {
             @Override
             public void onActivityCreated(final Activity activity, Bundle savedInstanceState) {
@@ -168,6 +156,36 @@ public class ApplicationStatus {
     }
 
     /**
+     * Initializes the activity status for a specified application.
+     *
+     * @param application The application whose status you wish to monitor.
+     */
+    public static void initialize(BaseChromiumApplication application) {
+        initializeLifecycleCallbacks(application);
+
+        application.registerWindowFocusChangedListener(
+                new BaseChromiumApplication.WindowFocusChangedListener() {
+                    @Override
+                    public void onWindowFocusChanged(Activity activity, boolean hasFocus) {
+                        if (!hasFocus || activity == sActivity) return;
+
+                        int state = getStateForActivity(activity);
+
+                        if (state != ActivityState.DESTROYED && state != ActivityState.STOPPED) {
+                            sActivity = activity;
+                        }
+
+                        // TODO(dtrainor): Notify of active activity change?
+                    }
+                });
+    }
+
+    public static void initializeForWebView(Application application) {
+        sWebViewMode = true;
+        initializeLifecycleCallbacks(application);

[dgn, 2015/09/30 11:02:55]

This is going to miss a cycle of Created/Resumed events at registration, right?
Shouldn't they be explicitly called instead of using the flag to only simulate
the CREATED event on the first activity? This would allow other stuff to run in
those hooks. But maybe entirely skipping them is better than calling them at the
wrong moment.

[timvolodine, 2015/10/01 15:49:29]

hmm not sure what you mean here. Basically in webview mode the 'created' event
can be skipped because the activity may already have been created before the
webview is started.

+    }
+
+    /**
      * Must be called by the main activity when it changes state.
      *
      * @param activity Current activity.
@@ -185,9 +203,13 @@ public class ApplicationStatus {
 
         int oldApplicationState = getStateForApplication();
 
-        if (newState == ActivityState.CREATED) {
-            assert !sActivityInfo.containsKey(activity);
-            sActivityInfo.put(activity, new ActivityInfo());
+        if (sWebViewMode) {
+            sActivityInfo.putIfAbsent(activity, new ActivityInfo());

[boliu, 2015/09/29 21:34:02]

This creates a new ActivityInfo object even if one is not needed.

[timvolodine, 2015/10/01 15:49:29]

right, my bad, done.

+        } else {
+            if (newState == ActivityState.CREATED) {
+                assert !sActivityInfo.containsKey(activity);
+                sActivityInfo.put(activity, new ActivityInfo());
+            }
         }
 
         // Invalidate the cached application state.