🍈 Unify application context usage.

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

Index: base/android/java/src/org/chromium/base/ContextUtils.java
diff --git a/base/android/java/src/org/chromium/base/ContextUtils.java b/base/android/java/src/org/chromium/base/ContextUtils.java
index 51adcff8f67375a4bf0507fedfeecf72daa07396..3ea54d54c8d7c70bc453a87bd5e624d7a760501c 100644
--- a/base/android/java/src/org/chromium/base/ContextUtils.java
+++ b/base/android/java/src/org/chromium/base/ContextUtils.java
@@ -14,6 +14,7 @@ import org.chromium.base.annotations.JNINamespace;
  */
 @JNINamespace("base::android")
 public class ContextUtils {
+    private static final String TAG = "ContextUtils";
     private static Context sApplicationContext;
 
     /**
@@ -34,24 +35,29 @@ public class ContextUtils {
     }
 
     /**
-     * Initialize the Android application context.
+     * Initializes the java application context.
      *
-     * Either this or the native equivalent base::android::InitApplicationContext must be called
-     * once during startup. JNI bindings must have been initialized, as the context is stored on
-     * both sides.
+     * This must be called early on during startup, before native is loaded and before any other
+     * clients make use of the application context through this class.
+     *
+     * @param appContext The application context.
      */
     public static void initApplicationContext(Context appContext) {
         assert appContext != null;
         assert sApplicationContext == null || sApplicationContext == appContext;

[Torne, 2016/04/15 14:22:43]

It seems like you've added a number of places where it will now be the case that
initApplicationContext is called more than once during startup of a particular
binary.

Are you aware that "assert" here basically doesn't mean anything reliably, and
that only a subset of configurations (older Android versions) ever actually
check asserts? For recent OS versions these are all no-ops and so are not in
fact enforcing this.

So, I'm worried that maybe this change is going to introduce cases where we
change the app context from one thing to another - this would especially be a
risk for WebView, because we have to wrap the application context in our own
custom wrapper and we *must* always use the wrapped version, not the original
context.

[Peter Wen, 2016/04/18 18:49:58]

Unfortunately without a base custom application instance initializing this
consistently (and it can't reliably do so in webview or content), we don't have
a consistent initialization path for all processes that we can dump globals like
this one.

I agree that the fewer places we can call this the better, and that the assert
is more just for tests right now.

Since webview doesn't use ChromeApplication, we can just have it set its context
as soon as it creates the wrapper, and make this function only accept the first
setter? :-/

[Torne, 2016/04/19 16:55:58]

I would prefer that every initialisation path guaranteed that this function was
called precisely once, by design, without any special cases on this side. It
doesn't seem like that should be impossible, but it might be a huge pain. :/

         initJavaSideApplicationContext(appContext);
-        nativeInitNativeSideApplicationContext(appContext);
     }
 
     /**
-     * JUnit Robolectric tests run without native code; allow them to set just the Java-side
-     * context. Do not use in configurations that actually run on Android!
+     * Initialize the native Android application context to be the same as the java counter-part.
      */
-    public static void initApplicationContextForJUnitTests(Context appContext) {
+    public static void initApplicationContextForNative() {
+        assert sApplicationContext != null;
+        nativeInitNativeSideApplicationContext(sApplicationContext);
+    }
+
+    @VisibleForTesting
+    public static void initApplicationContextForTests(Context appContext) {
         initJavaSideApplicationContext(appContext);
     }