Deduplicate Monochrome locale .paks

ui/base/resource/resource_bundle.cc

Index: ui/base/resource/resource_bundle.cc
diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc
index bf53a1314e39afe87aa4569ac7fa5577eb2c88a4..1c1a0d77e7f59264e039d6bdca0945c2d11dd1e7 100644
--- a/ui/base/resource/resource_bundle.cc
+++ b/ui/base/resource/resource_bundle.cc
@@ -248,6 +248,17 @@ ResourceBundle& ResourceBundle::GetSharedInstance() {
   return *g_shared_instance_;
 }
 
+void ResourceBundle::LoadSecondaryLocaleDataWithPakFileRegion(
+    base::File pak_file,
+    const base::MemoryMappedFile::Region& region) {
+  std::unique_ptr<DataPack> data_pack(new DataPack(SCALE_FACTOR_100P));

[sadrul, 2017/07/04 17:01:15]

auto data_pack = base::MakeUnique...

[F, 2017/07/10 18:56:34]

Done.

+  if (!data_pack->LoadFromFileRegion(std::move(pak_file), region)) {
+    NOTREACHED() << "failed to load secondary pak file";
+    return;
+  }
+  g_shared_instance_->secondary_locale_resources_data_ = std::move(data_pack);

[sadrul, 2017/07/04 17:01:15]

Why do you need to use |g_shared_instance| here?

[F, 2017/07/10 18:56:34]

Oops. Thanks! Done

+}
+
 #if !defined(OS_ANDROID)
 bool ResourceBundle::LocaleDataPakExists(const std::string& locale) {
   return !GetLocaleFilePath(locale, true).empty();
@@ -389,6 +400,7 @@ void ResourceBundle::LoadTestResources(const base::FilePath& path,
 
 void ResourceBundle::UnloadLocaleResources() {
   locale_resources_data_.reset();
+  secondary_locale_resources_data_.reset();
 }
 
 void ResourceBundle::OverrideLocalePakForTest(const base::FilePath& pak_path) {
@@ -550,20 +562,27 @@ base::string16 ResourceBundle::GetLocalizedString(int message_id) {
   }
 
   base::StringPiece data;
+  ResourceHandle::TextEncodingType encoding =
+      locale_resources_data_->GetTextEncodingType();
   if (!locale_resources_data_->GetStringPiece(static_cast<uint16_t>(message_id),
                                               &data)) {
-    // Fall back on the main data pack (shouldn't be any strings here except in
-    // unittests).
-    data = GetRawDataResource(message_id);
-    if (data.empty()) {
-      NOTREACHED() << "unable to find resource: " << message_id;
-      return base::string16();
+    if (secondary_locale_resources_data_.get() &&
+        secondary_locale_resources_data_->GetStringPiece(
+            static_cast<uint16_t>(message_id), &data)) {
+      // Fall back on the secondary locale pak if it exists.
+      encoding = secondary_locale_resources_data_->GetTextEncodingType();
+    } else {
+      // Fall back on the main data pack (shouldn't be any strings here except
+      // in unittests).
+      data = GetRawDataResource(message_id);
+      if (data.empty()) {
+        NOTREACHED() << "unable to find resource: " << message_id;
+        return base::string16();
+      }

[sadrul, 2017/07/04 17:01:14]

This looks backwards. Shouldn't you look up in the main data pack before going
to the secondary data pack?

[F, 2017/07/10 18:56:34]

Before the change, Chrome would look for string resources in main locale data
pack first, and main resource data pack next. With the introduction of secondary
locale data pack, Chrome would look for string resources in main locale data
pack first, secondary locale data pack next, and then the main resource data
pack. The main resource data pack is just a fallback, currently only used for
tests. It should not contain any string resources used by Chrome.

     }
   }
 
   // Strings should not be loaded from a data pack that contains binary data.
-  ResourceHandle::TextEncodingType encoding =
-      locale_resources_data_->GetTextEncodingType();
   DCHECK(encoding == ResourceHandle::UTF16 || encoding == ResourceHandle::UTF8)
       << "requested localized string from binary pack file";
 
@@ -588,6 +607,11 @@ base::RefCountedMemory* ResourceBundle::LoadLocalizedResourceBytes(
             static_cast<uint16_t>(resource_id), &data) &&
         !data.empty()) {
       return new base::RefCountedStaticMemory(data.data(), data.length());
+    } else if (secondary_locale_resources_data_.get() &&

[sadrul, 2017/07/04 17:01:14]

Don't need else.

[F, 2017/07/10 18:56:34]

Done.

+        secondary_locale_resources_data_->GetStringPiece(
+            static_cast<uint16_t>(resource_id), &data) &&
+        !data.empty()) {
+      return new base::RefCountedStaticMemory(data.data(), data.length());
     }
   }
   // Release lock_scope and fall back to main data pack.