Changes caching logic of mojo java apps

Previously we would extract all necessary files every time we ran the
app. This is obviously unnecessary for any bundled apps. Now we
extract only as necessary.

R=ben@chromium.org, jcivelli@chromium.org
BUG=none
TEST=none

Committed: https://crrev.com/128f7a0181634a89a35f681baab1d086d100e377
Cr-Commit-Position: refs/heads/master@{#330824}

Review URL: https://codereview.chromium.org/1149813002

Cr-Commit-Position: refs/heads/master@{#330940}

mojo/runner/android/android_handler.cc

@@ -15,6 +15,8 @@
 #include "mojo/public/c/system/main.h"
 #include "mojo/runner/android/run_android_application_function.h"
 #include "mojo/runner/native_application_support.h"
+#include "mojo/util/filename_util.h"
+#include "url/gurl.h"
 
 using base::android::AttachCurrentThread;
 using base::android::ScopedJavaLocalRef;
@@ -34,15 +36,17 @@ namespace {
 void RunAndroidApplication(JNIEnv* env,
                            jobject j_context,
                            const base::FilePath& app_path,
-                           jint j_handle) {
+                           jint j_handle,
+                           bool is_cached_app) {
   InterfaceRequest<Application> application_request =
       MakeRequest<Application>(MakeScopedHandle(MessagePipeHandle(j_handle)));
 
   // Load the library, so that we can set the application context there if
   // needed.
   // TODO(vtl): We'd use a ScopedNativeLibrary, but it doesn't have .get()!
-  base::NativeLibrary app_library =
-      LoadNativeApplication(app_path, shell::NativeApplicationCleanup::DELETE);
+  base::NativeLibrary app_library = LoadNativeApplication(
+      app_path, is_cached_app ? shell::NativeApplicationCleanup::DONT_DELETE
+                              : shell::NativeApplicationCleanup::DELETE);
   if (!app_library)
     return;
 
@@ -69,6 +73,57 @@ void RunAndroidApplication(JNIEnv* env,
   base::UnloadNativeLibrary(app_library);
 }
 
+// Returns true if |url| denotes a cached app. If true |app_dir| is set to the
+// path of the directory for the app and |path_to_mojo| the path of the app's
+// .mojo file.
+bool IsCachedApp(JNIEnv* env,
+                 const GURL& url,
+                 base::FilePath* app_dir,
+                 base::FilePath* path_to_mojo) {
+  ScopedJavaLocalRef<jstring> j_local_apps_dir =
+      Java_AndroidHandler_getLocalAppsDir(env, GetApplicationContext());
+  const base::FilePath local_apps_fp(
+      ConvertJavaStringToUTF8(env, j_local_apps_dir.obj()));
+  const std::string local_apps(util::FilePathToFileURL(local_apps_fp).spec());
+  const std::string response_url(GURL(url).spec());
+  if (response_url.size() <= local_apps.size() ||
+      local_apps.compare(0u, local_apps.size(), response_url, 0u,
+                         local_apps.size()) != 0) {
+    return false;
+  }
+
+  const std::string mojo_suffix(".mojo");
+  // app_rel_path is either something like html_viewer/html_viewer.mojo, or
+  // html_viewer.mojo, depending upon whether the app has a package.
+  const std::string app_rel_path(response_url.substr(local_apps.size() + 1));
+  const size_t slash_index = app_rel_path.find('/');
+  if (slash_index != std::string::npos) {
+    const std::string tail =
+        app_rel_path.substr(slash_index + 1, std::string::npos);
+    const std::string head = app_rel_path.substr(0, slash_index);
+    if (head.find('/') != std::string::npos ||
+        tail.size() <= mojo_suffix.size() ||
+        tail.compare(tail.size() - mojo_suffix.size(), tail.size(),
+                     mojo_suffix) != 0) {
+      return false;
+    }
+    *app_dir = local_apps_fp.Append(head);
+    *path_to_mojo = app_dir->Append(tail);
+    return true;
+  }
+  if (app_rel_path.find('/') != std::string::npos ||
+      app_rel_path.size() <= mojo_suffix.size() ||
+      app_rel_path.compare(app_rel_path.size() - mojo_suffix.size(),
+                           mojo_suffix.size(), mojo_suffix) != 0) {
+    return false;
+  }
+
+  *app_dir = local_apps_fp.Append(
+      app_rel_path.substr(0, app_rel_path.size() - mojo_suffix.size()));
+  *path_to_mojo = local_apps_fp.Append(app_rel_path);
+  return true;
+}
+
 }  // namespace
 
 AndroidHandler::AndroidHandler() : content_handler_factory_(this) {
@@ -81,13 +136,30 @@ void AndroidHandler::RunApplication(
     InterfaceRequest<Application> application_request,
     URLResponsePtr response) {
   JNIEnv* env = AttachCurrentThread();
+  RunAndroidApplicationFn run_android_application_fn = &RunAndroidApplication;
+  if (!response->url.is_null()) {
+    base::FilePath internal_app_path;
+    base::FilePath path_to_mojo;
+    if (IsCachedApp(env, GURL(response->url), &internal_app_path,
+                    &path_to_mojo)) {
+      ScopedJavaLocalRef<jstring> j_internal_app_path(
+          ConvertUTF8ToJavaString(env, internal_app_path.value()));
+      ScopedJavaLocalRef<jstring> j_path_to_mojo(
+          ConvertUTF8ToJavaString(env, path_to_mojo.value()));
+      Java_AndroidHandler_bootstrapCachedApp(
+          env, GetApplicationContext(), j_path_to_mojo.obj(),
+          j_internal_app_path.obj(),
+          application_request.PassMessagePipe().release().value(),
+          reinterpret_cast<jlong>(run_android_application_fn));
+      return;
+    }
+  }
   ScopedJavaLocalRef<jstring> j_archive_path =
       Java_AndroidHandler_getNewTempArchivePath(env, GetApplicationContext());
   base::FilePath archive_path(
       ConvertJavaStringToUTF8(env, j_archive_path.obj()));
 
   common::BlockingCopyToFile(response->body.Pass(), archive_path);
-  RunAndroidApplicationFn run_android_application_fn = &RunAndroidApplication;
   Java_AndroidHandler_bootstrap(
       env, GetApplicationContext(), j_archive_path.obj(),
       application_request.PassMessagePipe().release().value(),

mojo/runner/android/apk/src/org/chromium/mojo/shell/Bootstrap.java

@@ -22,14 +22,16 @@ public class Bootstrap implements Runnable {
     private final File mApplicationNativeLibrary;
     private final int mHandle;
     private final long mRunApplicationPtr;
+    private final boolean mIsCachedApp;
 
     public Bootstrap(Context context, File bootstrapNativeLibrary, File applicationNativeLibrary,
-            Integer handle, Long runApplicationPtr) {
+            Integer handle, Long runApplicationPtr, Boolean isCachedApp) {
         mContext = context;
         mBootstrapNativeLibrary = bootstrapNativeLibrary;
         mApplicationNativeLibrary = applicationNativeLibrary;
         mHandle = handle;
         mRunApplicationPtr = runApplicationPtr;
+        mIsCachedApp = isCachedApp;
     }
 
     @Override
@@ -37,9 +39,9 @@ public class Bootstrap implements Runnable {
         System.load(mBootstrapNativeLibrary.getAbsolutePath());
         System.load(mApplicationNativeLibrary.getAbsolutePath());
         nativeBootstrap(mContext, mApplicationNativeLibrary.getAbsolutePath(), mHandle,
-                mRunApplicationPtr);
+                mRunApplicationPtr, mIsCachedApp);
     }
 
     native void nativeBootstrap(Context context, String libraryPath, int handle,
-            long runApplicationPtr);
+            long runApplicationPtr, boolean isCachedApp);
 }

mojo/runner/android/apk/src/org/chromium/mojo/shell/FileHelper.java

@@ -7,7 +7,8 @@ package org.chromium.mojo.shell;
 import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
-import android.util.Log;
+
+import org.chromium.base.Log;
 
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
@@ -15,7 +16,6 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
-import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -35,63 +35,88 @@ class FileHelper {
     // Prefix used when naming timestamp files.
     private static final String TIMESTAMP_PREFIX = "asset_timestamp-";
 
+    /**
+     * Used to indicate the type of destination file that should be created.
+     */
+    public enum FileType {
+        TEMPORARY,
+        PERMANENT,
+    }
+
+    public enum ArchiveType {
+        /**
+         * The archive was created for a content handler (contains the mojo escape sequence).
+         */
+        CONTENT_HANDLER,
+        NORMAL,
+    }
+
     /**
      * Looks for a timestamp file on disk that indicates the version of the APK that the resource
      * assets were extracted from. Returns null if a timestamp was found and it indicates that the
-     * resources match the current APK. Otherwise returns a String that represents the filename of a
-     * timestamp to create.
+     * resources match the current APK. Otherwise returns the file to create.
      */
-    private static String checkAssetTimestamp(Context context, File outputDir) {
+    private static File findAssetTimestamp(Context context, File outputDir) {
         PackageManager pm = context.getPackageManager();
         PackageInfo pi = null;
 
         try {
             pi = pm.getPackageInfo(context.getPackageName(), 0);
         } catch (PackageManager.NameNotFoundException e) {
-            return TIMESTAMP_PREFIX;
         }
 
         if (pi == null) {
-            return TIMESTAMP_PREFIX;
+            return new File(outputDir, TIMESTAMP_PREFIX);
         }
 
-        String expectedTimestamp = TIMESTAMP_PREFIX + pi.versionCode + "-" + pi.lastUpdateTime;
-
-        String[] timestamps = outputDir.list(new FilenameFilter() {
-            @Override
-            public boolean accept(File dir, String name) {
-                return name.startsWith(TIMESTAMP_PREFIX);
+        final File expectedTimestamp =
+                new File(outputDir, TIMESTAMP_PREFIX + pi.versionCode + "-" + pi.lastUpdateTime);
+        return expectedTimestamp.exists() ? null : expectedTimestamp;
     }
-        });
 
-        if (timestamps.length != 1) {
-            // If there's no timestamp, nuke to be safe as we can't tell the age of the files.
-            // If there's multiple timestamps, something's gone wrong so nuke.
-            return expectedTimestamp;
+    /**
+     * Invoke prior to extracting any assets into {@code directory}. If necessary deletes all the
+     * files in the specified directory. The return value must be supplied to {@link
+     *createTimestampIfNecessary}.
+     *
+     * @param directory directory assets will be extracted to
+     * @return non-null if a file with the specified name needs to be created after assets have
+     * been extracted.
+     */
+    public static File prepareDirectoryForAssets(Context context, File directory) {
+        final File timestamp = findAssetTimestamp(context, directory);
+        if (timestamp == null) {
+            return null;
         }
-
-        if (!expectedTimestamp.equals(timestamps[0])) {
-            return expectedTimestamp;
+        for (File child : directory.listFiles()) {
+            deleteRecursively(child);
         }
-
-        // Timestamp file is already up-to date.
-        return null;
+        return timestamp;
     }
 
-    public static File extractFromAssets(Context context, String assetName, File outputDirectory,
-            boolean useTempFile) throws IOException, FileNotFoundException {
-        String timestampToCreate = null;
-        if (!useTempFile) {
-            timestampToCreate = checkAssetTimestamp(context, outputDirectory);
-            if (timestampToCreate != null) {
-                for (File child : outputDirectory.listFiles()) {
-                    deleteRecursively(child);
+    /**
+     * Creates a file used as a timestamp. The supplied file comes from {@link
+     *prepareDirectoryForAssets}.
+     *
+     * @param timestamp path of file to create, or null if a file does not need to be created
+     */
+    public static void createTimestampIfNecessary(File timestamp) {
+        if (timestamp == null) {
+            return;
         }
+        try {
+            timestamp.createNewFile();
+        } catch (IOException e) {
+            // In the worst case we don't write a timestamp, so we'll re-extract the asset next
+            // time.
+            Log.w(TAG, "Failed to write asset timestamp!");
         }
     }
 
+    public static File extractFromAssets(Context context, String assetName, File outputDirectory,
+            FileType fileType) throws IOException, FileNotFoundException {
         File outputFile;
-        if (useTempFile) {
+        if (fileType == FileType.TEMPORARY) {
             // Make the original filename part of the temp file name.
             // TODO(ppi): do we need to sanitize the suffix?
             String suffix = "-" + assetName;
@@ -111,36 +136,48 @@ class FileHelper {
         } finally {
             inputStream.close();
         }
-
-        if (timestampToCreate != null) {
-            try {
-                new File(outputDirectory, timestampToCreate).createNewFile();
-            } catch (IOException e) {
-                // In the worst case we don't write a timestamp, so we'll re-extract the asset next
-                // time.
-                Log.w(TAG, "Failed to write asset timestamp!");
-            }
-        }
-
         return outputFile;
     }
 
     /**
      * Extracts the file of the given extension from the archive. Throws FileNotFoundException if no
      * matching file is found.
+     *
+     * @return path of extracted file
      */
-    static File extractFromArchive(File archive, String suffixToMatch,
-            File outputDirectory) throws IOException, FileNotFoundException {
-        ZipInputStream zip = new ZipInputStream(new BufferedInputStream(new FileInputStream(
-                archive)));
+    static File extractFromArchive(File archive, String suffixToMatch, File outputDirectory,
+            FileType fileType, ArchiveType archiveType) throws IOException, FileNotFoundException {
+        if (!outputDirectory.exists() && !outputDirectory.mkdirs()) {
+            Log.e(TAG, "extractFromArchive unable to create directory "
+                            + outputDirectory.getAbsolutePath());
+            throw new FileNotFoundException();
+        }
+
+        BufferedInputStream inputStream = new BufferedInputStream(new FileInputStream(archive));
+        if (archiveType == ArchiveType.CONTENT_HANDLER) {
+            int currentChar;
+            do {
+                currentChar = inputStream.read();
+            } while (currentChar != -1 && currentChar != '\n');
+            if (currentChar == -1) {
+                throw new FileNotFoundException();
+            }
+            inputStream = new BufferedInputStream(inputStream);
+        }
+        ZipInputStream zip = new ZipInputStream(inputStream);
         ZipEntry entry;
         while ((entry = zip.getNextEntry()) != null) {
             if (entry.getName().endsWith(suffixToMatch)) {
+                // TODO(sky): sanitize name.
+                final String name = new File(entry.getName()).getName();
+                File extractedFile;
                 // Make the original filename part of the temp file name.
-                // TODO(ppi): do we need to sanitize the suffix?
-                String suffix = "-" + new File(entry.getName()).getName();
-                File extractedFile = File.createTempFile(TEMP_FILE_PREFIX, suffix,
-                        outputDirectory);
+                if (fileType == FileType.TEMPORARY) {
+                    final String suffix = "-" + name;
+                    extractedFile = File.createTempFile(TEMP_FILE_PREFIX, suffix, outputDirectory);
+                } else {
+                    extractedFile = new File(outputDirectory, name);
+                }
                 writeStreamToFile(zip, extractedFile);
                 zip.close();
                 return extractedFile;

mojo/runner/android/apk/src/org/chromium/mojo/shell/ShellMain.java

@@ -27,7 +27,8 @@ import java.util.List;
 public class ShellMain {
     private static final String TAG = "ShellMain";
 
-    // Directory where applications bundled with the shell will be extracted.
+    // Directory where applications cached with the shell will be extracted.
+    // TODO(sky): rename this to CACHED_APP_DIRECTORY.
     private static final String LOCAL_APP_DIRECTORY = "local_apps";
     // The mojo_shell library is also an executable run in forked processes when running
     // multi-process.
@@ -72,9 +73,13 @@ public class ShellMain {
         if (sInitialized) return;
         File localAppsDir = getLocalAppsDir(applicationContext);
         try {
+            final File timestamp =
+                    FileHelper.prepareDirectoryForAssets(applicationContext, localAppsDir);
             for (String assetPath : getAssetsList(applicationContext)) {
-                FileHelper.extractFromAssets(applicationContext, assetPath, localAppsDir, false);
+                FileHelper.extractFromAssets(
+                        applicationContext, assetPath, localAppsDir, FileHelper.FileType.PERMANENT);
             }
+            FileHelper.createTimestampIfNecessary(timestamp);
             File mojoShell = new File(applicationContext.getApplicationInfo().nativeLibraryDir,
                     MOJO_SHELL_EXECUTABLE);
 
@@ -111,7 +116,7 @@ public class ShellMain {
         nativeAddApplicationURL(url);
     }
 
-    private static File getLocalAppsDir(Context context) {
+    static File getLocalAppsDir(Context context) {
         return context.getDir(LOCAL_APP_DIRECTORY, Context.MODE_PRIVATE);
     }
 
@@ -128,7 +133,7 @@ public class ShellMain {
      * Initializes the native system. This API should be called only once per process.
      **/
     private static native void nativeInit(Context context, String mojoShellPath,
-            String[] parameters, String bundledAppsDirectory, String tmpDir);
+            String[] parameters, String cachedAppsDirectory, String tmpDir);
 
     private static native boolean nativeStart();
 

mojo/runner/android/bootstrap.cc

@@ -17,12 +17,13 @@ void Bootstrap(JNIEnv* env,
                jobject j_context,
                jstring j_native_library_path,
                jint j_handle,
-               jlong j_run_application_ptr) {
+               jlong j_run_application_ptr,
+               jboolean is_cached_app) {
   base::FilePath app_path(
       base::android::ConvertJavaStringToUTF8(env, j_native_library_path));
   RunAndroidApplicationFn run_android_application_fn =
       reinterpret_cast<RunAndroidApplicationFn>(j_run_application_ptr);
-  run_android_application_fn(env, j_context, app_path, j_handle);
+  run_android_application_fn(env, j_context, app_path, j_handle, is_cached_app);
 }
 
 bool RegisterBootstrapJni(JNIEnv* env) {

mojo/runner/android/run_android_application_function.h

@@ -19,7 +19,8 @@ namespace runner {
 typedef void (*RunAndroidApplicationFn)(JNIEnv* env,
                                         jobject j_context,
                                         const base::FilePath& app_path,
-                                        jint j_handle);
+                                        jint j_handle,
+                                        bool is_cached_app);
 
 }  // namespace runner
 }  // namespace mojo

mojo/runner/android/tests/src/org/chromium/mojo/shell/ShellTestBase.java

@@ -31,7 +31,8 @@ public class ShellTestBase {
         AssetManager manager = context.getResources().getAssets();
         for (String asset : manager.list("")) {
             if (asset.endsWith(".mojo")) {
-                FileHelper.extractFromAssets(context, asset, outputDirectory, false);
+                FileHelper.extractFromAssets(
+                        context, asset, outputDirectory, FileHelper.FileType.PERMANENT);
             }
         }
 

mojo/runner/url_resolver.cc

@@ -114,10 +114,16 @@ GURL URLResolver::ResolveMojoURL(const GURL& mojo_url) const {
   if (mojo_base_url_.SchemeIsFile()) {
     const GURL url_with_directory(
         mojo_base_url_.Resolve(base_url.host() + "/"));
-    const base::FilePath file_path(util::UrlToFilePath(url_with_directory));
-    if (base::DirectoryExists(file_path))
+    const base::FilePath dir(util::UrlToFilePath(url_with_directory));
+    if (base::DirectoryExists(dir)) {
+      const std::string mojo_file_name(base_url.host() + ".mojo");
+      const base::FilePath mojo_path =
+          dir.Append(base::FilePath::FromUTF8Unsafe(mojo_file_name));
+      // Only use the directory if the .mojo exists in the directory.
+      if (base::PathExists(mojo_path))
         return url_with_directory.Resolve(base_url.host() + ".mojo" + query);
     }
+  }
   return mojo_base_url_.Resolve(base_url.host() + ".mojo" + query);
 }
 

mojo/runner/url_resolver_unittest.cc

@@ -159,13 +159,23 @@ TEST_F(URLResolverTest, PreferDirectory) {
   EXPECT_EQ(util::FilePathToFileURL(tmp_dir.path()).spec() + "/foo.mojo",
             mapped_url.spec());
 
-  // With a directory |mojo:foo| maps to path/foo/foo.mojo.
+  // With an empty directory |mojo:foo| maps to path/foo.mojo.
   const base::FilePath foo_file_path(
       tmp_dir.path().Append(FILE_PATH_LITERAL("foo")));
   ASSERT_TRUE(base::CreateDirectory(foo_file_path));
   const GURL mapped_url_with_dir = resolver.ResolveMojoURL(GURL("mojo:foo"));
-  EXPECT_EQ(util::FilePathToFileURL(tmp_dir.path()).spec() + "/foo/foo.mojo",
+  EXPECT_EQ(util::FilePathToFileURL(tmp_dir.path()).spec() + "/foo.mojo",
             mapped_url_with_dir.spec());
+
+  // When foo.mojo exists in the directory (path/foo/foo.mojo), then it should
+  // be picked up.
+  // With an empty directory |mojo:foo| maps to path/foo/foo.mojo.
+  ASSERT_EQ(1,
+            base::WriteFile(foo_file_path.Append(FILE_PATH_LITERAL("foo.mojo")),
+                            "a", 1));
+  const GURL mapped_url_in_dir = resolver.ResolveMojoURL(GURL("mojo:foo"));
+  EXPECT_EQ(util::FilePathToFileURL(tmp_dir.path()).spec() + "/foo/foo.mojo",
+            mapped_url_in_dir.spec());
 }
 
 }  // namespace