🐸 Reland of Check for 0-length files in ResourceExtractor

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

Index: base/android/java/src/org/chromium/base/ResourceExtractor.java
diff --git a/base/android/java/src/org/chromium/base/ResourceExtractor.java b/base/android/java/src/org/chromium/base/ResourceExtractor.java
index 458e266b48c98f15e8d4d559058ec9312a663e2d..2854b02dd41d514d87dbafd9ccab7c09aa2cf803 100644
--- a/base/android/java/src/org/chromium/base/ResourceExtractor.java
+++ b/base/android/java/src/org/chromium/base/ResourceExtractor.java
@@ -6,6 +6,7 @@ package org.chromium.base;
 
 import android.annotation.TargetApi;
 import android.content.Context;
+import android.content.SharedPreferences;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.os.AsyncTask;
@@ -18,7 +19,6 @@ import org.chromium.base.annotations.SuppressFBWarnings;
 
 import java.io.File;
 import java.io.FileOutputStream;
-import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -37,6 +37,7 @@ public class ResourceExtractor {
     private static final String ICU_DATA_FILENAME = "icudtl.dat";
     private static final String V8_NATIVES_DATA_FILENAME = "natives_blob.bin";
     private static final String V8_SNAPSHOT_DATA_FILENAME = "snapshot_blob.bin";
+    private static final String APP_VERSION_PREF = "org.chromium.base.ResourceExtractor.Version";
 
     private static ResourceEntry[] sResourcesToExtract = new ResourceEntry[0];
 
@@ -71,21 +72,15 @@ public class ResourceExtractor {
                 while ((count = is.read(buffer, 0, BUFFER_SIZE)) != -1) {
                     os.write(buffer, 0, count);
                 }
-                os.flush();
-
-                // Ensure something reasonable was written.
-                if (outFile.length() == 0) {
-                    throw new IOException(outFile + " extracted with 0 length!");
-                }
             } finally {
                 try {
-                    if (is != null) {
-                        is.close();
-                    }
-                } finally {
                     if (os != null) {
                         os.close();
                     }
+                } finally {
+                    if (is != null) {
+                        is.close();
+                    }
                 }
             }
         }
@@ -97,24 +92,29 @@ public class ResourceExtractor {
                 return;
             }
 
-            String timestampFile = null;
             beginTraceSection("checkPakTimeStamp");
-            try {
-                timestampFile = checkPakTimestamp(outputDir);
-            } finally {
-                endTraceSection();
-            }
-            if (timestampFile != null) {
+            long curAppVersion = getApkVersion();
+            SharedPreferences sharedPrefs = ContextUtils.getAppSharedPreferences();
+            long prevAppVersion = sharedPrefs.getLong(APP_VERSION_PREF, 0);
+            boolean versionChanged = curAppVersion != prevAppVersion;
+            endTraceSection();
+
+            if (versionChanged) {
                 deleteFiles();
+                // Use the version only to see if files should be deleted, not to skip extraction.
+                // We've seen files be corrupted, so always attempt extraction.
+                // http://crbug.com/606413
+                sharedPrefs.edit().putLong(APP_VERSION_PREF, curAppVersion).apply();
             }
 
             beginTraceSection("WalkAssets");
             byte[] buffer = new byte[BUFFER_SIZE];
             try {
-                // Extract all files that don't already exist.
                 for (ResourceEntry entry : sResourcesToExtract) {
                     File output = new File(outputDir, entry.extractedFileName);
-                    if (output.exists()) {
+                    // TODO(agrieve): It would be better to check that .length == expectedLength.
+                    //     http://crbug.com/606413
+                    if (output.length() != 0) {
                         continue;
                     }
                     beginTraceSection("ExtractResource");
@@ -137,17 +137,6 @@ public class ResourceExtractor {
             } finally {
                 endTraceSection(); // WalkAssets
             }
-
-            // Finished, write out a timestamp file if we need to.
-            if (timestampFile != null) {
-                try {
-                    new File(outputDir, timestampFile).createNewFile();
-                } catch (IOException e) {
-                    // Worst case we don't write a timestamp, so we'll re-extract the resource
-                    // paks next start up.
-                    Log.w(TAG, "Failed to write resource pak timestamp!");
-                }
-            }
         }
 
         @Override
@@ -182,49 +171,16 @@ public class ResourceExtractor {
             }
         }
 
-        // Looks for a timestamp file on disk that indicates the version of the APK that
-        // the resource paks 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.
-        // Note that we do this to avoid adding a BroadcastReceiver on
-        // android.content.Intent#ACTION_PACKAGE_CHANGED as that causes process churn
-        // on (re)installation of *all* APK files.
-        private String checkPakTimestamp(File outputDir) {
-            final String timestampPrefix = "pak_timestamp-";
+        /** Returns a number that is different each time the apk changes. */
+        private long getApkVersion() {
             PackageManager pm = mContext.getPackageManager();
-            PackageInfo pi = null;
-
             try {
-                pi = pm.getPackageInfo(mContext.getPackageName(), 0);
+                // More appropriate would be versionCode, but it doesn't change while developing.
+                PackageInfo pi = pm.getPackageInfo(mContext.getPackageName(), 0);
+                return pi.lastUpdateTime;
             } catch (PackageManager.NameNotFoundException e) {
-                return timestampPrefix;
+                throw new RuntimeException(e);
             }
-
-            if (pi == null) {
-                return timestampPrefix;
-            }
-
-            String expectedTimestamp = timestampPrefix + pi.versionCode + "-" + pi.lastUpdateTime;
-
-            String[] timestamps = outputDir.list(new FilenameFilter() {
-                @Override
-                public boolean accept(File dir, String name) {
-                    return name.startsWith(timestampPrefix);
-                }
-            });
-
-            if (timestamps == null || 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;
-            }
-
-            if (!expectedTimestamp.equals(timestamps[0])) {
-                return expectedTimestamp;
-            }
-
-            // timestamp file is already up-to date.
-            return null;
         }
 
         @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR2)