Library loading fallback.

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

Index: base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java
diff --git a/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java b/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java
index 58198aed8c4874352e343cad7c6effa93d3e1681..9e80ac4ef552a815acd13b577172b48c896f24f2 100644
--- a/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java
+++ b/base/android/java/src/org/chromium/base/library_loader/LibraryLoaderHelper.java
@@ -10,13 +10,33 @@ import android.content.pm.ApplicationInfo;
 import android.os.Build;
 import android.util.Log;
 
+import java.io.BufferedOutputStream;
+import java.io.Closeable;
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
 import java.util.zip.ZipFile;
 
+import javax.annotation.Nullable;
+
+/**
+ * Class representing an exception which occured during the fallback process.
+ */
+class FallbackException extends Exception {
+    public FallbackException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public FallbackException(String message) {
+        this(message, null);
+    }
+}
+
 /**
  * The class provides helper functions to extract native libraries from APK,
  * and load libraries from there.
@@ -29,6 +49,14 @@ public class LibraryLoaderHelper {
 
     private static final String LIB_DIR = "lib";
 
+    // Fallback process constants.
+    private static final String FALLBACK_DIR = "fallback";

[rmcilroy, 2014/10/29 17:20:43]

Could we try integrating this with the other fallback mechanism (pulling it out
into the same directory, etc.)?  I would like to avoid having two completely
independent fallback mechanisms then we will have no clear idea where the
library file ended up depending on the fallback which was taken.

[petrcermak, 2014/10/30 19:55:07]

I refactored the code and removed duplication where possible. As discussed
offline, it is better to keep the (existing) workaround and (new) fallback in
separate directories. Fortunately, it seems that we don't actually need
Context.MODE_WORLD_READABLE and can use Context.MODE_PRIVATE (as in the
workaround).

+    private static final String FALLBACK_PREFIX = "fallback-";
+    private static final String TEMP_PREFIX = "tmp-";
+    private static final String LIBRARY_PREFIX = "lib";
+    private static final String LIBRARY_EXTENSION = ".so";
+    private static final int BUFFER_SIZE = 4096;
+
     /**
      * One-way switch becomes true if native libraries were unpacked
      * from APK.
@@ -71,7 +99,7 @@ public class LibraryLoaderHelper {
      * recreate one to the new directory. However, on some devices (e.g. Sony Xperia),
      * the symlink was updated, but fails to extract new native libraries from
      * the new apk.
-
+     *
      * We make the following changes to alleviate the issue:
      *  1) name the native library with apk version code, e.g.,
      *     libchrome.1750.136.so, 1750.136 is Chrome version number;
@@ -250,4 +278,240 @@ public class LibraryLoaderHelper {
             Log.e(TAG, "Failed to remove old libs, ", e);
         }
     }
+
+    // Log an error and throw an UnsatisfiedLinkError.
+    private static void throwUnsatisfiedLinkError(
+            String fallbackLibFilename, String zipFilename, String library, String reason,
+            @Nullable Throwable e) {
+        String errorMessage = "Unable to load fallback library " + fallbackLibFilename +
+                " for library " + library + " in " + zipFilename + ", " + reason;
+        if (e != null) {
+            Log.e(TAG, errorMessage, e);
+        } else {
+            Log.e(TAG, errorMessage);
+        }
+        throw new UnsatisfiedLinkError(errorMessage);
+    }
+
+    // Open a given zip file.
+    private static ZipFile openZipFile(String zipFilename) throws FallbackException {
+        Log.i(TAG, "Opening zip file " + zipFilename);
+        try {
+            return new ZipFile(zipFilename);
+        } catch (ZipException e) {
+            throw new FallbackException("unable to open the zip file", e);
+        } catch (IOException e) {
+            throw new FallbackException("unable to open the zip file", e);
+        }
+    }
+
+    // Find the library in the zip file.
+    private static ZipEntry findZipEntry(ZipFile zip, String library) throws FallbackException {
+        String libFilenameInZip = Linker.getLibraryFilenameInZipFile(library);
+        if (libFilenameInZip.equals("")) {
+            throw new FallbackException("could not retrieve library filename in zip file");
+        }
+
+        Log.i(TAG, "Searching for " + libFilenameInZip + " inside the zip file");
+        try {
+            return zip.getEntry(libFilenameInZip);
+        } catch (IllegalStateException e) {
+            throw new FallbackException("did not find library in zip file", e);
+        }
+    }
+
+    // Close a closable and log a warning if it fails.
+    private static void close(Closeable closable, String name) {
+        try {
+            closable.close();
+        } catch (IOException e) {
+            // Warn and ignore.
+            Log.w(TAG, "IO exception when closing " + name, e);
+        }
+    }
+
+    // Delete a file and log it.
+    private static void deleteFile(File file) {
+        if (file != null && file.delete()) {
+            Log.i(TAG, "Deleted file " + file);
+        } else {
+            Log.w(TAG, "Failed to delete file " + file);
+        }
+    }
+
+    // Determine whether it is necessary to generate the fallback library.
+    private static boolean needToGenerateFallback(
+            File fallbackLib, File zipFile, ZipEntry entry) {
+        // Check if the fallback library already exists.
+        if (!fallbackLib.exists()) {
+            Log.i(TAG, "Fallback file does not exist yet");
+            return true;
+        }
+
+        // Check last modification dates.
+        long zipTime = zipFile.lastModified();
+        long fallbackLibTime = fallbackLib.lastModified();
+        if (zipTime > fallbackLibTime) {
+            Log.i(TAG, "Not using existing fallback file because " +
+                    "the APK file " + zipFile.getPath() +
+                    " (timestamp=" + zipTime + ") is newer than " +
+                    "the fallback library " + fallbackLib.getPath() +
+                    "(timestamp=" + fallbackLibTime + ")");
+            return true;
+        }
+
+        // Check file sizes.
+        long entrySize = entry.getSize();
+        long fallbackLibSize = fallbackLib.length();
+        if (fallbackLibSize != entrySize) {
+            Log.i(TAG, "Not using existing fallback file because " +
+                    "the library in the APK " + zipFile.getPath() +
+                    " (" + entrySize + "B) has a different size than " +
+                    "the fallback library " + fallbackLib.getPath() +
+                    "(" + fallbackLibSize + "B)");
+            return true;
+        }
+
+        Log.i(TAG, "Reusing existing fallback file");
+        return false;
+    }
+
+    // Remove any "tmp-" and "fallback-" libraries which currently exist.
+    private static void removeExistingFallbackFiles(File fallbackDir) {
+        Log.i(TAG, "Removing existing fallback files in " + fallbackDir);
+        for (File f : fallbackDir.listFiles()) {
+            String name = f.getName();
+            if (name.endsWith(LIBRARY_EXTENSION) &&
+                    (name.startsWith(FALLBACK_PREFIX + LIBRARY_PREFIX) ||
+                     name.startsWith(TEMP_PREFIX + LIBRARY_PREFIX))) {

[picksi1, 2014/10/30 10:48:07]

If this were me I'd probably write a function that takes the File and figures
out if it is a fallback file (as it is long and not necessarily clear why it is
doing the checks it is), e.g.

if (isAFallbackFile(f)) {...}

It also keeps knowledge of the name out of this function.

But this would bloat the code & I know brevity is desirable too!

[petrcermak, 2014/10/30 19:55:07]

I removed this code.

+                deleteFile(f);
+            }
+        }
+    }
+
+    // Copy the library from the zipfile into a temporary file.
+    private static File copyLibraryFromZipFile(ZipFile zip, ZipEntry entry,
+            File fallbackDir, String library) throws FallbackException {
+        // Locate the library inside the zip file.
+        InputStream in = null;
+        try {
+            in = zip.getInputStream(entry);
+        } catch (IOException e) {
+            throw new FallbackException(
+                    "IO exception when locating library in the zip file", e);
+        }
+
+        // Temporary file used while the fallback library file is incomplete.
+        File tmpFile = new File(fallbackDir, TEMP_PREFIX + library);
+
+        // Open the output file.
+        OutputStream out = null;
+        try {
+            out = new BufferedOutputStream(new FileOutputStream(tmpFile));
+        } catch (FileNotFoundException e) {
+            close(in, "input stream");
+            throw new FallbackException("failed to create temporary file", e);
+        } catch (SecurityException e) {
+            close(in, "input stream");
+            throw new FallbackException(
+                    "security exception when creating temporary file", e);
+        }
+
+        // Copy the library from the zip file into the temporary file.
+        Log.i(TAG, "Copying " + library + " from " + zip.getName() + " to " + tmpFile);
+        try {
+            byte[] buffer = new byte[BUFFER_SIZE];
+            int len;
+            while ((len = in.read(buffer)) != -1) {
+                out.write(buffer, 0, len);
+            }
+        } catch (IOException e) {
+            deleteFile(tmpFile);
+            throw new FallbackException(
+                    "failed to copy the library into temporary file", e);
+        } finally {
+            close(in, "input stream");
+            close(out, "output stream");
+        }
+
+        return tmpFile;
+    }
+
+    // Set up file permissions and move the temporary file to the fallback file.
+    private static void setUpFallbackLibrary(File tmpLib, File fallbackLib) throws FallbackException {
+        Log.i(TAG, "Setting file permissions for " + tmpLib);
+        if (!tmpLib.setReadable(/* readable */ true, /* ownerOnly */ false)) {
+            throw new FallbackException("failed to chmod +r the temporary file");
+        }
+        if (!tmpLib.setExecutable(/* executable */ true, /* ownerOnly */ false)) {
+            throw new FallbackException("failed to chmod +x the temporary file");
+        }
+        Log.i(TAG, "Renaming " + tmpLib + " to " + fallbackLib);
+        if (!tmpLib.renameTo(fallbackLib)) {
+            throw new FallbackException("failed to rename the temporary file");
+        }
+    }
+
+    /**
+     * Copy a library from a zip file to the application's private directory.
+     * This is used as a fallback when we are unable to load the library
+     * directly from the zip file.
+     *
+     * @param context The context the code is running in.
+     * @param zipFilename Filename of the zip file containing the library.
+     * @param library Platform specific library name (e.g. libfoo.so).
+     * @return name of the fallback copy of the library.
+     */
+    public static String buildFallbackLibrary(
+            Context context, String zipFilename, String library) {
+        // Get the fallback directory and file.
+        String fallbackDirFilename = context.getDir(
+                FALLBACK_DIR, Context.MODE_WORLD_READABLE).getPath();
+        File fallbackDir = new File(fallbackDirFilename);
+        File fallbackLib = new File(fallbackDir, FALLBACK_PREFIX + library);
+        String fallbackLibFilename = fallbackLib.getPath();
+
+        // The zip process takes tens of milliseconds.
+        long before = System.currentTimeMillis();
+

[picksi1, 2014/10/30 10:48:07]

should 'before' and 'after' be called startTime & endTime?

[petrcermak, 2014/10/30 19:55:07]

ditto.

+        try {
+            ZipFile zip = openZipFile(zipFilename);

[rmcilroy, 2014/10/29 17:20:44]

Why can't we just use unpackLibrariesOnce (possibly adapted somewhat) rather
than adding yet more code to pull the libraries out of the zip file?

[petrcermak, 2014/10/30 19:55:07]

I refactored the code and merged them where I could (see unpackLibraries).

+            File tmpLib = null;
+

[picksi1, 2014/10/30 10:48:07]

should "tmpLib" be called tempLibraryFile?

[petrcermak, 2014/10/30 19:55:06]

ditto (removed code).

+            // Copy the zipped library into a temporary file (if necessary).
+            try {
+                ZipEntry entry = findZipEntry(zip, library);

[picksi1, 2014/10/30 10:48:07]

The name 'entry' doesn't help understand what this is.

[petrcermak, 2014/10/30 19:55:07]

I renamed it to "packedLib".

+                if (needToGenerateFallback(fallbackLib, new File(zipFilename), entry)) {
+                    removeExistingFallbackFiles(fallbackDir);
+                    tmpLib = copyLibraryFromZipFile(zip, entry, fallbackDir, library);
+                }
+            } finally {
+                close(zip, "zip file");
+            }
+
+            // Modify permissions of the temporary file and rename it to fallback library.

[picksi1, 2014/10/30 10:48:07]

Is the above comment correct?

[petrcermak, 2014/10/30 19:55:07]

ditto (but I think that it was).

+            if (tmpLib != null) {
+                try {
+                    setUpFallbackLibrary(tmpLib, fallbackLib);
+                } finally {
+                    if (tmpLib.exists()) {
+                        deleteFile(tmpLib);
+                    }
+                }
+            }
+
+            // Check that the fallback library exists.
+            if (!fallbackLib.exists()) {
+                throw new FallbackException("fallback library was not created");
+            }
+        } catch (FallbackException e) {
+            throwUnsatisfiedLinkError(fallbackLibFilename, zipFilename, library,
+                    e.getMessage(), e.getCause());
+        } finally {
+            long after = System.currentTimeMillis();
+            Log.i(TAG, "Building fallback library took " + (after - before) + "ms");

[rmcilroy, 2014/10/29 17:20:43]

There is no point logging this I don't think (other than sanity checking during
your development).

[petrcermak, 2014/10/30 19:55:07]

Done.

+        }
+
+        return fallbackLibFilename;
+    }
 }