Add UMA for testing whether device supports memory mapping APK files with executable permissions.

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

Index: base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
diff --git a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
index 91e53678707f17b3b96a8b8bacaebe6ab07818de..ea8fcd95d448fec21f4f44f4bcbeb2829934cc3e 100644
--- a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
+++ b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
@@ -155,6 +155,7 @@ public class LibraryLoader {
                 if (useChromiumLinker) {
                     // Load libraries using the Chromium linker.
                     Linker.prepareLibraryLoad();
+                    Linker.testLoadFromApk(context.getApplicationInfo().sourceDir);
 

[picksi1, 2014/10/10 10:52:17]

"testLoadFromApk" makes this look like a unit-test and doesn't imply the UMA
stuff (assuming this is the top level call). Can we rename this to (something
like) generateDirectAPKLoadingUMAstats?

[rmcilroy, 2014/10/10 11:02:31]

I would agree that test is perhaps wrong, but would rather we kept UMA out of
the name since we might eventually use the same function to check whether we
want to use the fallback or not.

Maybe call it checkLoadLibraryFromAPK or similar?

[petrcermak, 2014/10/10 14:20:13]

Done. Renamed to "checkApkMemoryMappingSupport".

[rmcilroy, 2014/10/10 16:26:07]

Sorry, I still think this is the wrong name since it's checking for mapping as
executable. I would still call this checkLibraryLoadFromApkSupport.

                     for (String library : NativeLibraries.LIBRARIES) {
                         String zipfile = null;
@@ -295,6 +296,8 @@ public class LibraryLoader {
         if (Linker.isUsed()) {
             nativeRecordChromiumAndroidLinkerHistogram(Linker.loadAtFixedAddressFailed(),
                     SysUtils.isLowEndDevice());
+            nativeRecordChromiumAndroidLinkerLoadFromApkSucceededHistogram(
+                    Linker.loadFromApkSucceeded());

[rmcilroy, 2014/10/10 10:27:05]

I think we could just put as an argument in
nativeRecordChromiumAndroidLinkerHistogram rather than adding a new method.

[petrcermak, 2014/10/10 14:20:14]

Done.

         }

[picksi1, 2014/10/10 10:52:17]

On first reading this code I didn't know if loadFromApkSucceeded() was returning
a true/false value, or implementing a behaviour that needs to happen when
loading was successful (like a callback or something). Can we rename it to make
it clear if it's a Getter or a callback ... Or could we return an enum that this
code can check directly e.g. loadFromApkStatus()==SUCCESS? Is loadFromAPK really
a binary value? Can it fail for different reasons?

[petrcermak, 2014/10/10 14:20:13]

Removed method.

 
         nativeRecordNativeLibraryHack(sNativeLibraryHackWasUsed);
@@ -317,6 +320,11 @@ public class LibraryLoader {
             boolean loadedAtFixedAddressFailed,
             boolean isLowMemoryDevice);
 
+    // Method called to record statistics about the support of memory mapping APK files with
+    // executable permissions.
+    private static native void nativeRecordChromiumAndroidLinkerLoadFromApkSucceededHistogram(
+            boolean loadFromApkSucceeded);
+
     // Get the version of the native library. This is needed so that we can check we
     // have the right version before initializing the (rest of the) JNI.
     private static native String nativeGetVersionNumber();