Enable icu_use_data_file_flag on Android

0. Roll icu to r249466 to copy Android-specific icudtl.dat
1. Turn it in build/common.gypi by default except for android webview build. Move it inside L2 var dict and pull it up to the top level var dict to make 'Google Chrome' build on Android happy. 
2. Add an entry for icudtl.dat to chrome/chrome_android_paks.gypi so that
it's copied to assets directory for Chrome and related targets. 
3. Extract icudtl.dat from the asset and copy to DIR_ANDROID_APP_DATA and make it world-readable so that child processes (sandboxed with uid) can read it. 
4. Add icudtl.dat to MANDATORY_PAKS list in various Android build targets 

Google Chrome has #4 but in a separate CL ( https://chrome-internal-review.googlesource.com/#/c/155554/ ). That CL will land before this CL. 

TBR=avi@chromium.org
BUG=72633
TEST=base_unittests:*Convers*, net_unittests:*IDN*, browser tests on all platforms.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253938 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 139 insertions, 25 deletions

diff --git a/DEPS b/DEPS
index e8ee780..6fe49f5 100644
--- a/DEPS
+++ b/DEPS
@@ -99,7 +99,7 @@ deps = {
     "/external/w3c/csswg-test.git@8c415e3215a203fa3a22dbdd1799279fdf44c81e",
 
   "src/third_party/icu":
-    "/trunk/deps/third_party/icu46@246118",
+    "/trunk/deps/third_party/icu46@249466",
 
   "src/third_party/libexif/sources":
     "/trunk/deps/third_party/libexif/sources@146817",
diff --git a/android_webview/android_webview_tests.gypi b/android_webview/android_webview_tests.gypi
index 5ad85fe..9eadf38 100644
--- a/android_webview/android_webview_tests.gypi
+++ b/android_webview/android_webview_tests.gypi
@@ -22,6 +22,13 @@
           '<(PRODUCT_DIR)/android_webview_apk/assets/asset_icon.png',
           '<(PRODUCT_DIR)/android_webview_apk/assets/full_screen_video_test.html',
         ],
+        'conditions': [
+          ['icu_use_data_file_flag==1', {
+            'additional_input_paths': [
+              '<(PRODUCT_DIR)/icudtl.dat',
+            ],
+          }],
+        ],
       },
       'copies': [
         {
@@ -31,6 +38,13 @@
             '<(java_in_dir)/assets/asset_icon.png',
             '<(java_in_dir)/assets/full_screen_video_test.html',
           ],
+	  'conditions': [
+            ['icu_use_data_file_flag==1', {
+              'files': [
+                '<(PRODUCT_DIR)/icudtl.dat',
+              ],
+	    }],
+          ],
         },
       ],
       'includes': [ '../build/java_apk.gypi' ],
diff --git a/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellApplication.java b/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellApplication.java
index 6f92ba3..f6488f4 100644
--- a/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellApplication.java
+++ b/android_webview/test/shell/src/org/chromium/android_webview/shell/AwShellApplication.java
@@ -18,7 +18,7 @@ public class AwShellApplication extends Application {
     private static final String TAG = "AwShellApplication";
     /** The minimum set of .pak files the test runner needs. */
     private static final String[] MANDATORY_PAKS = {
-        "webviewchromium.pak", "en-US.pak"
+        "webviewchromium.pak", "en-US.pak", "icudtl.dat"
     };
 
     @Override
diff --git a/base/i18n/icu_util.cc b/base/i18n/icu_util.cc
index 5fa37f8..bbb73f9 100644
--- a/base/i18n/icu_util.cc
+++ b/base/i18n/icu_util.cc
@@ -94,6 +94,8 @@ bool InitializeICU() {
 #if defined(OS_WIN)
     // The data file will be in the same directory as the current module.
     bool path_ok = PathService::Get(base::DIR_MODULE, &data_path);
+#elif defined(OS_ANDROID)
+    bool path_ok = PathService::Get(base::DIR_ANDROID_APP_DATA, &data_path);
 #else
     // For now, expect the data file to be alongside the executable.
     // This is sufficient while we work on unit tests, but will eventually
diff --git a/build/android/pylib/gtest/setup.py b/build/android/pylib/gtest/setup.py
index 3f2340f..5805e74 100644
--- a/build/android/pylib/gtest/setup.py
+++ b/build/android/pylib/gtest/setup.py
@@ -123,7 +123,7 @@ def _GenerateDepsDirUsingIsolate(suite_name):
       '--config-variable', 'OS', 'android',
       '--config-variable', 'chromeos', '0',
       '--config-variable', 'component', 'static_library',
-      '--config-variable', 'icu_use_data_file_flag', '0',
+      '--config-variable', 'icu_use_data_file_flag', '1',
       '--config-variable', 'use_openssl', '0',
   ]
   assert not cmd_helper.RunCmd(isolate_cmd)
diff --git a/build/common.gypi b/build/common.gypi
index 2735ac8..01e7b07 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -820,6 +820,15 @@
           'remoting%': 0,
           'enable_printing%': 0,
         }],
+
+	# By default, use ICU data file (icudtl.dat) on all platforms
+	# except when building Android WebView.
+	# TODO(jshin): Handle 'use_system_icu' on Linux (Chromium).
+        ['android_webview_build==0', {
+          'icu_use_data_file_flag%' : 1,
+        }, {
+          'icu_use_data_file_flag%' : 0,
+        }],
       ],
 
       # Set this to 1 to enable use of concatenated impulse responses
@@ -969,6 +978,7 @@
     'use_libjpeg_turbo%': '<(use_libjpeg_turbo)',
     'use_system_libjpeg%': '<(use_system_libjpeg)',
     'android_webview_build%': '<(android_webview_build)',
+    'icu_use_data_file_flag%': '<(icu_use_data_file_flag)',
     'gyp_managed_install%': 0,
     'create_standalone_apk%': 1,
     'enable_app_list%': '<(enable_app_list)',
@@ -1264,9 +1274,6 @@
     # IPC fuzzer is disabled by default.
     'enable_ipc_fuzzer%': 0,
 
-    # Whether or not to use "icu*.dat" file for ICU data.
-    # Do not use it by default.
-    'icu_use_data_file_flag%': 0,
 
     # Force disable libstdc++ debug mode.
     'disable_glibcxx_debug%': 0,
@@ -1308,7 +1315,6 @@
       }],
       ['OS=="win"', {
         'windows_driver_kit_path%': '$(WDK_DIR)',
-        'icu_use_data_file_flag%': 1,
       }],
       ['os_posix==1 and OS!="mac" and OS!="ios"', {
         'conditions': [
@@ -1331,21 +1337,11 @@
             # can use breakpad for these builds.
             'release_unwind_tables%': 0,
           }],
-          # TODO(jungshik): Turn this on on Android.
-          # For ChromeOS, this should be turned on in chromeos-chrome.ebuild
-          # file as well by adding icu_use_data_file_flag=1 to BUILD_DEFINES.
-          ['OS!="android"', {
-            'icu_use_data_file_flag%': 1,
-          }],
         ],
       }],  # os_posix==1 and OS!="mac" and OS!="ios"
-      ['OS=="mac"', {
-        'icu_use_data_file_flag%': 1,
-      }],  # os=="mac"
       ['OS=="ios"', {
         'disable_nacl%': 1,
         'enable_background%': 0,
-        'icu_use_data_file_flag%': 1,
         'use_system_libxml%': 1,
         'use_system_sqlite%': 1,
         'locales==': [
diff --git a/chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellApplication.java b/chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellApplication.java
index 5fcdba7..2f4eb44 100644
--- a/chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellApplication.java
+++ b/chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellApplication.java
@@ -22,10 +22,17 @@ import java.util.ArrayList;
  */
 public class ChromiumTestShellApplication extends ChromiumApplication {
     private static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "chromiumtestshell";
+    /**
+     * icudtl.dat provides ICU (i18n library) with all the necessary data
+     * for its operation. We used to link the data statically to our binary,
+     * but don't do that any more and need to install along with pak files.
+     * See src/third_party/icu/README.chromium.
+     */
     private static final String[] CHROME_MANDATORY_PAKS = {
         "en-US.pak",
         "resources.pak",
         "chrome_100_percent.pak",
+        "icudtl.dat",
     };
     private static final String COMMAND_LINE_FILE =
             "/data/local/tmp/chromium-testshell-command-line";
diff --git a/chrome/chrome_android_paks.gypi b/chrome/chrome_android_paks.gypi
index 7937f2f..622ff61f 100644
--- a/chrome/chrome_android_paks.gypi
+++ b/chrome/chrome_android_paks.gypi
@@ -100,6 +100,16 @@
       '<(chrome_android_pak_output_folder)/zh-CN.pak',
       '<(chrome_android_pak_output_folder)/zh-TW.pak',
     ],
+    'conditions': [
+      ['icu_use_data_file_flag==1', {
+        'chrome_android_pak_input_resources': [
+          '<(PRODUCT_DIR)/icudtl.dat',
+	],
+        'chrome_android_pak_output_resources': [
+          '<(chrome_android_pak_output_folder)/icudtl.dat',
+	],
+      }],
+    ],
   },
 }
 
diff --git a/content/content.gyp b/content/content.gyp
index b921672..7ef6737 100644
--- a/content/content.gyp
+++ b/content/content.gyp
@@ -504,6 +504,22 @@
           ],
           'includes': [ 'content_jni.gypi' ],
         },
+        {
+          'target_name': 'content_icudata',
+          'type': 'none',
+          'conditions': [
+            ['icu_use_data_file_flag==1', {
+              'copies': [
+                {
+                  'destination': '<(PRODUCT_DIR)/content_shell/assets',
+                  'files': [
+                    '<(PRODUCT_DIR)/icudtl.dat',
+                  ],
+                },
+              ],
+            }],
+          ],
+        },
       ],
     }],  # OS == "android"
   ],
diff --git a/content/content_shell.gypi b/content/content_shell.gypi
index 548a2b3..0576ea7 100644
--- a/content/content_shell.gypi
+++ b/content/content_shell.gypi
@@ -1042,6 +1042,7 @@
           'target_name': 'content_shell_apk',
           'type': 'none',
           'dependencies': [
+            'content.gyp:content_icudata',
             'content.gyp:content_java',
             'content_java_test_support',
             'content_shell_java',
@@ -1062,6 +1063,13 @@
             'additional_input_paths': ['<(PRODUCT_DIR)/content_shell/assets/content_shell.pak'],
             'asset_location': '<(PRODUCT_DIR)/content_shell/assets',
             'extra_native_libs': ['<(SHARED_LIB_DIR)/libosmesa.so'],
+            'conditions': [
+              ['icu_use_data_file_flag==1', {
+                'additional_input_paths': [
+                  '<(PRODUCT_DIR)/icudtl.dat',
+                ],
+              }],
+            ],
           },
           'conditions': [
             ['android_webview_build==0', {
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 5daf221..137089f 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -1420,6 +1420,7 @@
           'target_name': 'content_browsertests_apk',
           'type': 'none',
           'dependencies': [
+            'content.gyp:content_icudata',
             'content.gyp:content_java',
             'content_browsertests',
             'content_java_test_support',
@@ -1432,6 +1433,13 @@
             'native_lib_target': 'libcontent_browsertests',
             'additional_input_paths': ['<(PRODUCT_DIR)/content_shell/assets/content_shell.pak'],
             'asset_location': '<(PRODUCT_DIR)/content_shell/assets',
+            'conditions': [
+              ['icu_use_data_file_flag==1', {
+                'additional_input_paths': [
+                  '<(PRODUCT_DIR)/icudtl.dat',
+                ],
+              }],
+            ],
           },
           'includes': [ '../build/java_apk.gypi' ],
         },
@@ -1453,6 +1461,7 @@
           'type': 'none',
           'dependencies': [
             'chromium_android_linker_test',
+            'content.gyp:content_icudata',
             'content.gyp:content_java',
             'content_shell_java',
           ],
@@ -1465,6 +1474,13 @@
             'asset_location': '<(PRODUCT_DIR)/content_shell/assets',
             'use_chromium_linker': '1',
             'enable_chromium_linker_tests': '1',
+            'conditions': [
+              ['icu_use_data_file_flag==1', {
+                'additional_input_paths': [
+                  '<(PRODUCT_DIR)/icudtl.dat',
+                ],
+              }],
+            ],
           },
           'includes': [ '../build/java_apk.gypi' ],
         },
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 23aacca..51c7d43 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,7 @@ public class ResourceExtractor {
                 p.append(currentLanguage);
                 p.append("(-\\w+)?\\.pak");
             }
+
             Pattern paksToInstall = Pattern.compile(p.toString());
 
             AssetManager manager = mContext.getResources().getAssets();
@@ -109,7 +111,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 +138,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 +231,7 @@ public class ResourceExtractor {
 
     private final Context mContext;
     private ExtractTask mExtractTask;
+    private final File mAppDataDir;
     private final File mOutputDir;
 
     private static ResourceExtractor sInstance;
@@ -263,6 +272,7 @@ public class ResourceExtractor {
 
     private ResourceExtractor(Context context) {
         mContext = context;
+        mAppDataDir = getAppDataDirFromContext(mContext);
         mOutputDir = getOutputDirFromContext(mContext);
     }
 
@@ -303,17 +313,32 @@ 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");
     }
 
+    /**
+     * Pak files (UI strings and other resources) should be updated along with
+     * Chrome. A version mismatch can lead to a rather broken user experience.
+     * The ICU data (icudtl.dat) is less version-sensitive, but still can
+     * lead to malfunction/UX misbehavior. So, we regard failing to update them
+     * as an error.
+     */
     public static void deleteFiles(Context context) {
+        File icudata = new File(getAppDataDirFromContext(context), ICU_DATA_FILENAME);
+        if (icudata.exists() && !icudata.delete()) {
+            Log.e(LOGTAG, "Unable to remove the icudata " + icudata.getName());
+        }
         File dir = getOutputDirFromContext(context);
         if (dir.exists()) {
             File[] files = dir.listFiles();
             for (File file : files) {
                 if (!file.delete()) {
-                    Log.w(LOGTAG, "Unable to remove existing resource " + file.getName());
+                    Log.e(LOGTAG, "Unable to remove existing resource " + file.getName());
                 }
             }
         }
diff --git a/content/shell/android/browsertests_apk/src/org/chromium/content_browsertests_apk/ContentBrowserTestsApplication.java b/content/shell/android/browsertests_apk/src/org/chromium/content_browsertests_apk/ContentBrowserTestsApplication.java
index 22a70e0..f9b7993 100644
--- a/content/shell/android/browsertests_apk/src/org/chromium/content_browsertests_apk/ContentBrowserTestsApplication.java
+++ b/content/shell/android/browsertests_apk/src/org/chromium/content_browsertests_apk/ContentBrowserTestsApplication.java
@@ -11,7 +11,10 @@ import org.chromium.content.browser.ResourceExtractor;
 
 public class ContentBrowserTestsApplication extends Application {
 
-    private static final String[] MANDATORY_PAK_FILES = new String[] {"content_shell.pak"};
+    private static final String[] MANDATORY_PAK_FILES = new String[] {
+        "content_shell.pak",
+        "icudtl.dat"
+    };
     private static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "content_shell";
 
     @Override
diff --git a/content/shell/android/linker_test_apk/src/org/chromium/chromium_linker_test_apk/ChromiumLinkerTestApplication.java b/content/shell/android/linker_test_apk/src/org/chromium/chromium_linker_test_apk/ChromiumLinkerTestApplication.java
index 252198a..89e7362 100644
--- a/content/shell/android/linker_test_apk/src/org/chromium/chromium_linker_test_apk/ChromiumLinkerTestApplication.java
+++ b/content/shell/android/linker_test_apk/src/org/chromium/chromium_linker_test_apk/ChromiumLinkerTestApplication.java
@@ -14,7 +14,16 @@ import org.chromium.content.browser.ResourceExtractor;
  */
 public class ChromiumLinkerTestApplication extends Application {
 
-    private static final String[] MANDATORY_PAK_FILES = new String[] {"content_shell.pak"};
+    /**
+     * icudtl.dat provides ICU (i18n library) with all the data for its
+     * operation. We use to link it statically to our binary, but not any more
+     * so that we have to install it along with other mandatory pak files.
+     * See src/third_party/icu/README.chromium.
+     */
+    private static final String[] MANDATORY_PAK_FILES = new String[] {
+        "content_shell.pak",
+        "icudtl.dat"
+    };
     private static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "chromium_linker_test";
 
     @Override
diff --git a/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellApplication.java b/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellApplication.java
index 43ded48..b068ea0 100644
--- a/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellApplication.java
+++ b/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellApplication.java
@@ -13,8 +13,16 @@ import org.chromium.content.browser.ResourceExtractor;
  * to be shared across the main activity and the child services created.
  */
 public class ContentShellApplication extends ContentApplication {
-
-    private static final String[] MANDATORY_PAK_FILES = new String[] {"content_shell.pak"};
+    /**
+     * icudtl.dat provides ICU (i18n library) with all the data for its
+     * operation. We use to link it statically to our binary, but not any more
+     * so that we have to install it along with other mandatory pak files.
+     * See src/third_party/icu/README.chromium.
+     */
+    private static final String[] MANDATORY_PAK_FILES = new String[] {
+        "content_shell.pak",
+        "icudtl.dat"
+    };
     private static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "content_shell";
 
     @Override