Optimize RTL check in ICU to avoid mmap access.

base/i18n/rtl.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/i18n/rtl.h"
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
-#include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "third_party/icu/source/common/unicode/locid.h"
 #include "third_party/icu/source/common/unicode/uchar.h"
 #include "third_party/icu/source/common/unicode/uscript.h"
 #include "third_party/icu/source/i18n/unicode/coll.h"
 
 namespace {
 
@@ skipping 99 matching lines @@
   g_icu_text_direction = UNKNOWN_DIRECTION;
 }
 
 bool IsRTL() {
   return ICUIsRTL();
 }
 
 bool ICUIsRTL() {
   if (g_icu_text_direction == UNKNOWN_DIRECTION) {
     const icu::Locale& locale = icu::Locale::getDefault();
-    g_icu_text_direction = GetTextDirectionForLocale(locale.getName());
+    g_icu_text_direction = GetTextDirectionForLocaleInStartUp(locale.getName());
   }
   return g_icu_text_direction == RIGHT_TO_LEFT;
 }
 
+TextDirection GetTextDirectionForLocaleInStartUp(const char* locale_name) {
+  // This list needs to be updated if we add more RTL locales.
+  static const char* kRTLLocales[] = {
+    "ar",
+    "fa",
+    "he",
+    "iw",
+    "ur"
+  };
+  for (const char* rtl_locale : kRTLLocales) {
+    if (StartsWith(locale_name, rtl_locale, CompareCase::SENSITIVE))

[jungshik at Google, 2015/08/11 20:38:29]

One more thing I forgot to mention. Sorry that I didn't mention it in the first
round. 

Instead of StartsWith, you have to get the language code (chopping |locale_name|
at the 1st '_' or '-')  and compare the result against the list. That way, you
can defend against a case where 3-letter language code (for some languages,
there's no 2-letter code and we have to use 3-letter codes) happens to starts
with one of the above. 

[jungshik at Google, 2015/08/11 20:41:59]

Because the only 3-letter language code in the current list of Chrome's UI
languages is 'fil' and it's not a prefix match for any of the above. So, it's
theoeretical.  So, if you really think it can affect perf, you can comment this
and use StartsWith, but I guess perf impact would be really tiny if you do the
right thing. 

[danduong, 2015/08/11 21:03:16]

That was definitely an oversight on my part. Thanks for the catch!

+      return RIGHT_TO_LEFT;
+  }
+  return LEFT_TO_RIGHT;
+}
+
 TextDirection GetTextDirectionForLocale(const char* locale_name) {
-  const std::string group_name =
-      FieldTrialList::FindFullName("LightSpeed");
-  // StartsWith allows flexibility for this experiment to apply to multiple
-  // group names. To start, this will apply to AvoidMMapOnStartup.
-  if (StartsWith(group_name, "AvoidMMap", CompareCase::SENSITIVE)) {
-    static const char kEnglishLocale[] = "en_";
-    if (StartsWith(locale_name, kEnglishLocale, CompareCase::SENSITIVE))
-      return LEFT_TO_RIGHT;
-  }
   UErrorCode status = U_ZERO_ERROR;
   ULayoutType layout_dir = uloc_getCharacterOrientation(locale_name, &status);
   DCHECK(U_SUCCESS(status));
   // Treat anything other than RTL as LTR.
   return (layout_dir != ULOC_LAYOUT_RTL) ? LEFT_TO_RIGHT : RIGHT_TO_LEFT;
 }
 
 TextDirection GetFirstStrongCharacterDirection(const string16& text) {
   const UChar* string = text.c_str();
   size_t length = text.length();
@@ skipping 241 matching lines @@
       begin == kRightToLeftOverride)
     ++begin_index;
   size_t end_index = text.length() - 1;
   if (text[end_index] == kPopDirectionalFormatting)
     --end_index;
   return text.substr(begin_index, end_index - begin_index + 1);
 }
 
 }  // namespace i18n
 }  // namespace base