Android: Better error messaging for some UnsatisfiedLinkErrors.

A common cause of UnsatisfiedLinkErrors is that the native library
hasn't been loaded yet. It's difficult to tell the difference between
this and other UnsatisfiedLinkErrors.

This CL adds generated code that queries LibraryLoader to determine
library loading status and throws an explicit error when a native
method is called before the library has been loaded.

Bug: 1022938
Change-Id: I63980990abaef16da3847a05eda01cb116779560
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1976905Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729461}

base/BUILD.gn

@@ -3224,9 +3224,13 @@ if (is_android) {
   java_library("jni_java") {
     supports_android = true
     sources = [
+      "android/java/src/org/chromium/base/JniException.java",
       "android/java/src/org/chromium/base/JniStaticTestMocker.java",
+      "android/java/src/org/chromium/base/NativeLibraryLoadedStatus.java",
       "android/java/src/org/chromium/base/annotations/NativeMethods.java",
     ]
+    srcjar_deps = [ ":base_build_config_gen" ]
+    jar_excluded_patterns = [ "*/BuildConfig.class" ]
   }
 
   android_library("base_java") {

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.base;
+
+/**
+ *  Error when calling native methods.
+ */
+public class JniException extends RuntimeException {
+    public JniException(String msg) {
+        super(msg);
+    }
+}

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

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.base;
+
+/**
+ * Exposes native library loading status.
+ */
+public class NativeLibraryLoadedStatus {
+    /**
+     * Interface for querying native method availability.
+     */
+    public interface NativeLibraryLoadedStatusProvider {
+        boolean areMainDexNativeMethodsReady();
+        boolean areNativeMethodsReady();
+    }
+
+    private static NativeLibraryLoadedStatusProvider sProvider;
+
+    public static void checkLoaded(boolean isMainDex) {
+        // Necessary to make sure all of these calls are stripped in release builds.
+        if (!BuildConfig.DCHECK_IS_ON) return;
+
+        if (sProvider == null) return;
+
+        boolean nativeMethodsReady = isMainDex ? sProvider.areMainDexNativeMethodsReady()
+                                               : sProvider.areNativeMethodsReady();
+        if (!nativeMethodsReady) {
+            throw new JniException("Native method called before the native library was ready.");
+        }
+    }
+
+    public static void setProvider(NativeLibraryLoadedStatusProvider statusProvider) {
+        sProvider = statusProvider;
+    }
+
+    public static NativeLibraryLoadedStatusProvider getProviderForTesting() {
+        return sProvider;
+    }
+}

base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java

@@ -25,6 +25,8 @@ import org.chromium.base.ContextUtils;
 import org.chromium.base.FileUtils;
 import org.chromium.base.JNIUtils;
 import org.chromium.base.Log;
+import org.chromium.base.NativeLibraryLoadedStatus;
+import org.chromium.base.NativeLibraryLoadedStatus.NativeLibraryLoadedStatusProvider;
 import org.chromium.base.StreamUtil;
 import org.chromium.base.StrictModeContext;
 import org.chromium.base.TraceEvent;
@@ -32,6 +34,7 @@ import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.MainDex;
 import org.chromium.base.annotations.NativeMethods;
+import org.chromium.base.annotations.RemovableInRelease;
 import org.chromium.base.compat.ApiHelperForM;
 import org.chromium.base.metrics.CachedMetrics;
 import org.chromium.base.metrics.RecordHistogram;
@@ -242,6 +245,23 @@ public class LibraryLoader {
         return NativeLibraries.sEnableLinkerTests;
     }
 
+    @RemovableInRelease
+    public void enableJniChecks() {
+        if (!BuildConfig.DCHECK_IS_ON) return;
+
+        NativeLibraryLoadedStatus.setProvider(new NativeLibraryLoadedStatusProvider() {
+            @Override
+            public boolean areMainDexNativeMethodsReady() {
+                return mLoadState >= LoadState.MAIN_DEX_LOADED;
+            }
+
+            @Override
+            public boolean areNativeMethodsReady() {
+                return isInitialized();
+            }
+        });
+    }
+
     /**
      * Return if library is already loaded successfully by the zygote.
      */

base/android/javatests/src/org/chromium/base/library_loader/EarlyNativeTest.java

@@ -7,11 +7,15 @@ package org.chromium.base.library_loader;
 import android.content.pm.ApplicationInfo;
 import android.support.test.filters.SmallTest;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.BuildConfig;
+import org.chromium.base.JniException;
+import org.chromium.base.NativeLibraryLoadedStatus;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.MainDex;
 import org.chromium.base.annotations.NativeMethods;
@@ -21,7 +25,7 @@ import org.chromium.base.test.util.CallbackHelper;
 import java.util.concurrent.TimeoutException;
 
 /**
- * Tests for {@link LibraryLoader#ensureMainDexInitialized}.
+ * Tests for early JNI initialization.
  */
 @RunWith(BaseJUnit4ClassRunner.class)
 @JNINamespace("base")
@@ -36,6 +40,11 @@ public class EarlyNativeTest {
         mEnsureMainDexInitializedFinished = new CallbackHelper();
     }
 
+    @After
+    public void tearDown() {
+        NativeLibraryLoadedStatus.setProvider(null);
+    }
+
     private class TestLibraryLoader extends LibraryLoader {
         @Override
         protected void loadMainDexAlreadyLocked(ApplicationInfo appInfo, boolean inZygote) {
@@ -78,6 +87,7 @@ public class EarlyNativeTest {
     private void doTestFullInitializationDoesntBlockMainDexInitialization(final boolean initialize)
             throws Exception {
         final TestLibraryLoader loader = new TestLibraryLoader();
+        loader.enableJniChecks();
         loader.setLibraryProcessType(LibraryProcessType.PROCESS_BROWSER);
         final Thread t1 = new Thread(() -> {
             if (initialize) {
@@ -110,4 +120,42 @@ public class EarlyNativeTest {
     public void testLoadDoesntBlockMainDexInitialization() throws Exception {
         doTestFullInitializationDoesntBlockMainDexInitialization(false);
     }
+
+    @Test
+    @SmallTest
+    public void testNativeMethodsReadyAfterLibraryInitialized() {
+        LibraryLoader.getInstance().enableJniChecks();
+
+        Assert.assertFalse(
+                NativeLibraryLoadedStatus.getProviderForTesting().areMainDexNativeMethodsReady());
+        Assert.assertFalse(
+                NativeLibraryLoadedStatus.getProviderForTesting().areNativeMethodsReady());
+
+        LibraryLoader.getInstance().ensureMainDexInitialized();
+        Assert.assertTrue(
+                NativeLibraryLoadedStatus.getProviderForTesting().areMainDexNativeMethodsReady());
+        Assert.assertFalse(
+                NativeLibraryLoadedStatus.getProviderForTesting().areNativeMethodsReady());
+
+        LibraryLoader.getInstance().ensureInitialized();
+        Assert.assertTrue(
+                NativeLibraryLoadedStatus.getProviderForTesting().areMainDexNativeMethodsReady());
+        Assert.assertTrue(
+                NativeLibraryLoadedStatus.getProviderForTesting().areNativeMethodsReady());
+    }
+
+    @Test
+    @SmallTest
+    public void testNativeMethodsNotReadyThrows() {
+        LibraryLoader.getInstance().enableJniChecks();
+
+        try {
+            // Test is a no-op if dcheck isn't on.
+            if (BuildConfig.DCHECK_IS_ON) {
+                EarlyNativeTestJni.get().isCommandLineInitialized();
+                Assert.fail("Using JNI before the library is loaded should throw an exception.");
+            }
+        } catch (JniException e) {
+        }
+    }
 }

base/android/jni_generator/java/src/org/chromium/jni_generator/JniProcessor.java

@@ -21,6 +21,7 @@ import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
 
 import org.chromium.base.JniStaticTestMocker;
+import org.chromium.base.NativeLibraryLoadedStatus;
 import org.chromium.base.annotations.CheckDiscard;
 import org.chromium.base.annotations.MainDex;
 import org.chromium.base.annotations.NativeMethods;
@@ -67,8 +68,10 @@ public class JniProcessor extends AbstractProcessor {
     private static final Class<NativeMethods> JNI_STATIC_NATIVES_CLASS = NativeMethods.class;
     private static final Class<MainDex> MAIN_DEX_CLASS = MainDex.class;
     private static final Class<CheckDiscard> CHECK_DISCARD_CLASS = CheckDiscard.class;
-    private static final String CHECK_DISCARD_CRBUG = "crbug.com/993421";
+    private static final Class<NativeLibraryLoadedStatus> JNI_STATUS_CLASS =
+            NativeLibraryLoadedStatus.class;
 
+    private static final String CHECK_DISCARD_CRBUG = "crbug.com/993421";
     private static final String NATIVE_WRAPPER_CLASS_POSTFIX = "Jni";
 
     // The native class name and package name used in debug.
@@ -394,6 +397,7 @@ public class JniProcessor extends AbstractProcessor {
                     throw new UnsupportedOperationException($noMockExceptionString);
                 }
             }
+            NativeLibraryLoadedStatus.checkLoaded($isMainDex)
             return new {classname}Jni();
         }
          */
@@ -402,6 +406,7 @@ public class JniProcessor extends AbstractProcessor {
                                 + "The current configuration requires all native "
                                 + "implementations to have a mock instance.",
                         nativeInterfaceType);
+
         MethodSpec instanceGetter =
                 MethodSpec.methodBuilder("get")
                         .addModifiers(Modifier.PUBLIC, Modifier.STATIC)
@@ -416,6 +421,8 @@ public class JniProcessor extends AbstractProcessor {
                                 noMockExceptionString)
                         .endControlFlow()
                         .endControlFlow()
+                        .addStatement("$T.$N($L)", ClassName.get(JNI_STATUS_CLASS), "checkLoaded",
+                                isMainDex)
                         .addStatement("return new $N()", name)
                         .build();
 

chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java

@@ -144,6 +144,7 @@ public class ChromeApplication extends Application {
                 ProductConfig.COMPRESSED_LOCALES, ProductConfig.UNCOMPRESSED_LOCALES);
         LibraryLoader.getInstance().setLinkerImplementation(
                 ProductConfig.USE_CHROMIUM_LINKER, ProductConfig.USE_MODERN_LINKER);
+        LibraryLoader.getInstance().enableJniChecks();
 
         if (isBrowserProcess) {
             TraceEvent.end("ChromeApplication.attachBaseContext");

content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellApplication.java

@@ -31,6 +31,7 @@ public class ContentShellApplication extends Application {
         boolean isBrowserProcess = !ContextUtils.getProcessName().contains(":");
         ContextUtils.initApplicationContext(this);
         ResourceBundle.setNoAvailableLocalePaks();
+        LibraryLoader.getInstance().enableJniChecks();
         LibraryLoader.getInstance().setLibraryProcessType(isBrowserProcess
                         ? LibraryProcessType.PROCESS_BROWSER
                         : LibraryProcessType.PROCESS_CHILD);