Enable icu_use_data_file_flag on Android

content/public/android/java/src/org/chromium/content/browser/ResourceExtractor.java

Index: content/public/android/java/src/org/chromium/content/browser/ResourceExtractor.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/ResourceExtractor.java b/content/public/android/java/src/org/chromium/content/browser/ResourceExtractor.java
index 23aacca28c93749e67b9eb09ac3c5ab44aa938e9..e2aad6eabfde3db719c634a3721c174cd6c33382 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ResourceExtractor.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ResourceExtractor.java
@@ -36,6 +36,7 @@ public class ResourceExtractor {
     private static final String LOGTAG = "ResourceExtractor";
     private static final String LAST_LANGUAGE = "Last language";
     private static final String PAK_FILENAMES = "Pak filenames";
+    private static final String ICU_DATA_FILENAME = "icudtl.dat";
 
     private static String[] sMandatoryPaks = null;
 
@@ -96,6 +97,10 @@ public class ResourceExtractor {
                 p.append(currentLanguage);
                 p.append("(-\\w+)?\\.pak");
             }
+
+            if (p.length() > 0) p.append('|');
+            p.append(ICU_DATA_FILENAME);
+
             Pattern paksToInstall = Pattern.compile(p.toString());
 
             AssetManager manager = mContext.getResources().getAssets();
@@ -109,7 +114,8 @@ public class ResourceExtractor {
                     if (!paksToInstall.matcher(file).matches()) {
                         continue;
                     }
-                    File output = new File(mOutputDir, file);
+                    boolean isICUData = file.equals(ICU_DATA_FILENAME);
+                    File output = new File(isICUData ? mAppDataDir : mOutputDir, file);
                     if (output.exists()) {
                         continue;
                     }
@@ -135,7 +141,12 @@ public class ResourceExtractor {
                             throw new IOException(file + " extracted with 0 length!");
                         }
 
-                        filenames.add(file);
+                        if (!isICUData) {
+                            filenames.add(file);
+                        } else {
+                            // icudata needs to be accessed by a renderer process.
+                            output.setReadable(true, false);
+                        }
                     } finally {
                         try {
                             if (is != null) {
@@ -223,6 +234,7 @@ public class ResourceExtractor {
 
     private final Context mContext;
     private ExtractTask mExtractTask;
+    private final File mAppDataDir;
     private final File mOutputDir;
 
     private static ResourceExtractor sInstance;
@@ -263,11 +275,12 @@ public class ResourceExtractor {
 
     private ResourceExtractor(Context context) {
         mContext = context;
+        mAppDataDir = getAppDataDirFromContext(mContext);
         mOutputDir = getOutputDirFromContext(mContext);
     }
 
     public void waitForCompletion() {
-        if (shouldSkipPakExtraction()) {
+        if (shouldSkipExtraction()) {
             return;
         }
 
@@ -295,7 +308,7 @@ public class ResourceExtractor {
             return;
         }
 
-        if (shouldSkipPakExtraction()) {
+        if (shouldSkipExtraction()) {
             return;
         }
 
@@ -303,11 +316,18 @@ public class ResourceExtractor {
         mExtractTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
     }
 
+    public static File getAppDataDirFromContext(Context context) {
+        return new File(PathUtils.getDataDirectory(context.getApplicationContext()));
+    }
     public static File getOutputDirFromContext(Context context) {
-        return new File(PathUtils.getDataDirectory(context.getApplicationContext()), "paks");
+        return new File(getAppDataDirFromContext(context), "paks");
     }
 
     public static void deleteFiles(Context context) {
+        File icudata = new File(getAppDataDirFromContext(context), ICU_DATA_FILENAME);
+        if (icudata.exists() && !icudata.delete()) {
+            Log.w(LOGTAG, "Unable to remove the icudata " + icudata.getName());
+        }
         File dir = getOutputDirFromContext(context);
         if (dir.exists()) {
             File[] files = dir.listFiles();
@@ -321,8 +341,21 @@ public class ResourceExtractor {
 
     /**
      * Pak extraction not necessarily required by the embedder; we allow them to skip
-     * this process if they call setMandatoryPaksToExtract with a single empty String.
+     * this process if they call setMandatoryPaksToExtract with a single empty String and the
+     * icudata is absent. icudtl.dat will be present only when icu_use_data_file_flag == 1, which is
+     * false for the WebView build.

[benm (inactive), 2014/02/10 21:30:45]

i think that if an app using WebView had bundled a file called icudtl.dat that
we'd now extract this to it's data directory?

Also, I think that looking into the AssetManager might be slow (although I don't
have data to back that up), and this happens on the critical app start up path.
Would it be better to set a flag to say if we should extract ICU data, like we
set the mandatory paks? Maybe we just have MANDATORY_RESOURCES that combines the
needed PAKs and other files? we could still do the check on the filename when we
extract above to put the ICU file into the correct place.

[jungshik at Google, 2014/02/10 22:10:31]

Although that's not likely, I agree that we'd better be on the safe side. 
I'm less worried about the performance than about your 1st concern (although I'm
just a novice here and I may as well completely wrong) given that line 116 uses
AssetManager to list all the files in APK. 

There are 5 files specifying MANDATORY_PAKS ( http://goo.gl/KgHvhZ ). Are they
all non-webview stadalone targets? Then, I have to add icudtl.dat to all 5 of
them. 

[benm (inactive), 2014/02/10 22:28:32]

I think that the other use will only happen in the case that PAK extraction is
actually required, which should be only the first time after a new install or
update. I think this new check will happen on every start up regardless?
Yes, they are all non-WebView. They all look like test runners - do they all
require the ICU data file? 

[jungshik at Google, 2014/02/10 23:41:46]

That's what I expect us to do, but my reading of the current code (lines 106 ~
125) is that every time it starts, AssetManager is called to get the list of all
the files in assets/, match the list against 'pattern' and check their existence
in the target directory (if exist / already copied/extreacted, do not
copy/extract). 
Am I misreading it? 
Anything that renders web pages need the icudata unless we use the 'system icu'
(ICU from Android), which is the case of WebView. ICU is used also in the
UI/browser process. 

BTW, is this also non-webview?  

android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellApplication.java

      */
+    private boolean shouldSkipExtraction() {
+        AssetManager manager = mContext.getResources().getAssets();
+        String[] files = null;
+        try {
+            files = manager.list(ICU_DATA_FILENAME);
+        } catch (IOException e) {
+            Log.w(LOGTAG, ICU_DATA_FILENAME + " is not found.");
+        }
+        return (files == null) && shouldSkipPakExtraction();
+    }
+
     private static boolean shouldSkipPakExtraction() {
         // Must call setMandatoryPaksToExtract before beginning resource extraction.
         assert sMandatoryPaks != null;