[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..9e80d28e0779782d6046bb442a90108dcc73c5e1 100644
--- a/base/android/java/src/org/chromium/base/ApplicationStatus.java
+++ b/base/android/java/src/org/chromium/base/ApplicationStatus.java
@@ -64,6 +64,18 @@ public class ApplicationStatus {
     private static ApplicationStateListener sNativeApplicationStateListener;
 
     /**
+     * If true means we are running in an activity lifecycle independent mode,
+     * see {@link #initializeActivityIndependent(Application, Context)} for more details.
+     */
+    private static boolean sActivityLifecycleIndependentMode = false;
+
+    /**
+     * Context corresponding to sApplication. Note this context can be different from sApplication
+     * iself so use sApplicationContext instead of casting sApplication.
+     */
+    private static Context sApplicationContext;
+
+    /**
      * A map of which observers listen to state changes from which {@link Activity}.
      */
     private static final Map<Activity, ActivityInfo> sActivityInfo =
@@ -109,28 +121,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 +165,49 @@ public class ApplicationStatus {
     }
 
     /**
+     * Initializes the activity status for a specified application.
+     * The initialization should happen before any activity is created.
+     *
+     * @param application The application whose status you wish to monitor.
+     */
+    public static void initialize(BaseChromiumApplication application) {
+        sApplicationContext = application.getApplicationContext();
+        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?
+                    }
+                });
+    }
+
+
+    /**
+     * Initializes ApplicationStatus in "ActivityLifecycleIndependentMode". This mode is
+     * different from the regular initialization (@see initialize) in the following ways:
+     *   1. Currently doesn't listen to focus changes (hence sActivity is always null).
+     *   2. Allows to specify a custom applicationContext and an android application object.
+     *   3. Allows to call this method at any time during lifetime of the application.
+     */
+    public static void initializeActivityIndependent(Application application,
+            Context applicationContext) {
+        sActivityLifecycleIndependentMode = true;
+        sApplicationContext = applicationContext;
+
+        initializeLifecycleCallbacks(application);
+    }
+
+    /**
      * Must be called by the main activity when it changes state.
      *
      * @param activity Current activity.
@@ -176,18 +216,25 @@ public class ApplicationStatus {
     private static void onStateChange(Activity activity, int newState) {
         if (activity == null) throw new IllegalArgumentException("null activity is not supported");
 
-        if (sActivity == null
-                || newState == ActivityState.CREATED
-                || newState == ActivityState.RESUMED
-                || newState == ActivityState.STARTED) {
-            sActivity = activity;
-        }
-
         int oldApplicationState = getStateForApplication();
 
-        if (newState == ActivityState.CREATED) {
-            assert !sActivityInfo.containsKey(activity);
-            sActivityInfo.put(activity, new ActivityInfo());
+        if (sActivityLifecycleIndependentMode) {

[boliu, 2015/10/02 15:51:32]

since my non-inline comment were ignored twice, inline this time..

What's the story for chromium listening to the lifetime events of every single
activity in the app, instead of just the ones that has webview created in them?
Do we believe this is ok?

Also does all listeners deal well with say, not seeing the CREATE state of an
activity?

[timvolodine, 2015/10/08 16:13:55]

Bo, I didn't ignore your comments and I thought I answered the first one
already. Basically I think the webview-specific 'application status' could be
implemented in android_webview on top of this class. This could use the
AwWebViewLifecycleObserver for example.
Also it appears some functionality like NetworkChangeNotifier would not work out
of the box with an 'ApplicationStatus' that would depend on straightforward
webview creation (see crrev.com/1297943003/).
Regarding missing the ActivityState.CREATED is not really a problem as it is
followed by STARTED and RESUMED which will make sure the ApplicationState is set
accordingly. The listeners would then correctly receive application state
changes.

However I did some further experimentation regarding the possibility of missing
all of the CREATED, STARTED, RESUMED activity signals which appears to be
possible using this somewhat far-fetched snippet.

@Override
protected void onResume() {
    super.onResume();
    setContentView(R.layout.activity_webview_browser);
}

Which means in that particular case that on startup webview will think the
Application state is HAS_DESTROYED_ACTIVITIES (sort of default). To address this
possibility I think we can set sCachedApplicationState to HAS_RUNNING_ACTIVITIES
inside the initializeActivityIndependent() method. The assumption is that when
webview is initialized the Activity is either running or will be running very
soon. wdyt?

[boliu, 2015/10/08 18:01:30]

This doesn't really answer whether this patch is safe, ie what kind of thing
does observers do right now that could have a negative impact to apps? It's
probably ok, but want to check.

Regarding webview specific implementation, I was thinking ApplicationStatus
becomes an interface, and there's the default implementation chrome uses, and
then there's the webview-specific implementation that webview sets on init.
By this example, You mean that something _real important_ happens in onResume,
right? Do we already know nothing _real important_ happens in onCreate/onStart?
This is a question for base owner. But my thought is that's a bad idea since
that's by definition inconsistent, since sActivity is still null.

My thought would be directly insert the activity used to create a webview, as
soon as the webview is inserted. Then you can transition to a non-destroyed but
still consistent state, and maybe also send fake notifications like onCreate so
the observers are consistent too.

[timvolodine, 2015/10/09 15:46:36]

yeah, I have a list of classes using it, need to have a closer look.
yes, but I don't think it is essential here. We can always make an interface and
set implementations instead of sActivityLifecycleIndependentMode boolean. The
question of correct initialization still stands.
setContentView triggers AwBrowserProcess.start and hence
ApplicationStatus.initializeActivityIndependent() but at this stage no activity
state changes will be observed as it happens after CREATED,STARTED,RESUMED have
already happened. If this happens in onStart, or onCreate there is no such
problem.
Hmm, the problem is that at initialization of ApplicationStatus we don't really
have an Activity available to init with. Maybe this can be resolved using
ActivityThread but that seems a bit hacky (but maybe the only option?). Also,
making ApplicationStatus depend on actual webview creation (i.e. AwContents) is
not exactly the same and would actually be something different than
'ApplicationStatus'.

[boliu, 2015/10/09 15:58:57]

Ahh ok. That can definitely happen with real apps too (ie they create webview
much later after activity has been active, think which only creates webview when
you open a thread)
Right. I mean supply the activity in AwContents constructor, which is after
initializeActivityIndependent. The state will be DESTROYED immediately after
initializeActivityIndependent. But presumably AwContents constructor will be
called soon, and if that has an activity, then give that to ApplicationStatus to
track.

[timvolodine, 2015/10/09 16:50:36]

Right, I was actually trying to avoid this scenario as the constructor of
AwContents would be able to trigger an ApplicationState change which can result
in a concurrency issue with NetworkChangeNotifier (which listens to
ApplicationStatus). This is basically crbug.com/520088, but with this approach I
see only one potential problem now, i.e. if network state changes between lets
say AwBrowserProcess.start and AwContents contructor, so this is very close..

[timvolodine, 2015/10/09 16:52:38]

Actually, regarding your earlier comment about inconsistency of setting
sCachedApplicationState: currently we have an invariant that
sActivityLifecycleIndependentMode implies sActivity == null. So there is no
inconsistency w.r.t. sActivity.

[boliu, 2015/10/09 19:29:11]

Good point. Which means that deserves a huge comment, probably next to
sActivity, that it can be null at these "unexpected" times. And make sure all
callers can handle null.

[boliu, 2015/10/09 19:29:11]

I skimmed through the bug and still don't quite understand the issue wrt to
changes here. Can this be explained simplier terms for someone who don't know
the network stack, or should I just fire up cs look at all the code mentioned in
https://code.google.com/p/chromium/issues/detail?id=520088#c27 ?

[timvolodine, 2015/10/12 12:35:43]

Yeah, it's a long story I'll try to summarize. There is apparently a concurrency
issue with the host_resolver which happens when we enable NetworkChangeNotifier
when the AwContents constructor is called. Basically the NETWORK_CHANGE event
propagates via multiple post tasks induces a DNS_CHANGE event and forces the
host_resolver to abort pending jobs. Seemingly this happens because an app (i.e.
twitter) tries to load a page shortly after creation of webview and ends up
thinking that network has changed during page load (or more precisely
host_resolution). So given current code it seems that activating
NetworkChangeNotifier at the time of webview creation is not a great idea.

[timvolodine, 2015/10/12 12:35:43]

It's even stronger than that, what I meant by invariant is that sActivity is
always null (i.e. at anytime) if we are in sActivityLifecycleIndependentMode.
I've added some comments regarding this in getLastTrackedFocusedActivity().
Classes that use it do handle 'null' as it is what it currently returns in
webview case.

+            assert sActivity == null;
+            if (!sActivityInfo.containsKey(activity)) {
+                sActivityInfo.put(activity, new ActivityInfo());
+            }
+        } else {
+            if (sActivity == null
+                    || newState == ActivityState.CREATED
+                    || newState == ActivityState.RESUMED
+                    || newState == ActivityState.STARTED) {
+                sActivity = activity;
+            }
+
+            if (newState == ActivityState.CREATED) {
+                assert !sActivityInfo.containsKey(activity);
+                sActivityInfo.put(activity, new ActivityInfo());
+            }
         }
 
         // Invalidate the cached application state.
@@ -233,8 +280,12 @@ public class ApplicationStatus {
     /**
      * @return The most recent focused {@link Activity} tracked by this class.  Being focused means
      *         out of all the activities tracked here, it has most recently gained window focus.
+     *         Note that in 'sActivityLifecycleIndependentMode' the returned Activity is currently
+     *         always null (see crbug.com/538175).
      */
     public static Activity getLastTrackedFocusedActivity() {
+        // sActivityLifecycleIndependentMode implies sActivity == null
+        assert !sActivityLifecycleIndependentMode || sActivity == null;
         return sActivity;
     }
 
@@ -253,7 +304,7 @@ public class ApplicationStatus {
      * @return The {@link Context} for the {@link Application}.
      */
     public static Context getApplicationContext() {
-        return sApplication != null ? sApplication.getApplicationContext() : null;
+        return sApplicationContext;
     }
 
     /**