resources: Prevent including the same resource in multiple pack files.

Add code to cause failure at runtime if a resource-pack includes a
resource that is already present in another resource-pack.

BUG=471609

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

Cr-Commit-Position: refs/heads/master@{#329646}

chrome/test/base/javascript_browser_test.cc

@@ -52,11 +52,6 @@ void JavaScriptBrowserTest::SetUpOnMainThread() {
   ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &source_root_directory));
   library_search_paths_.push_back(source_root_directory);
 
-  base::FilePath resources_pack_path;
-  PathService::Get(chrome::FILE_RESOURCES_PACK, &resources_pack_path);
-  ResourceBundle::GetSharedInstance().AddDataPackFromPath(
-      resources_pack_path, ui::SCALE_FACTOR_NONE);
-
   AddLibrary(base::FilePath(kMockJSPath));
   AddLibrary(base::FilePath(kWebUILibraryJS));
 }

components/BUILD.gn

@@ -298,6 +298,7 @@ test("components_unittests") {
     "//content/test:test_support",
     "//net",
     "//ui/base",
+    "//ui/resources:ui_test_pak",
   ]
   data_deps = [ ":components_tests_pak" ]
 
@@ -331,10 +332,6 @@ repack("components_tests_pak") {
   sources = [
     "$root_gen_dir/components/components_resources.pak",
     "$root_gen_dir/components/strings/components_strings_en-US.pak",
-    "$root_gen_dir/ui/resources/ui_resources_100_percent.pak",
-    "$root_gen_dir/ui/resources/webui_resources.pak",
-    "$root_gen_dir/ui/strings/app_locale_settings_en-US.pak",
-    "$root_gen_dir/ui/strings/ui_strings_en-US.pak",
   ]
 
   output = "$root_out_dir/components_tests_resources.pak"
@@ -342,8 +339,6 @@ repack("components_tests_pak") {
   deps = [
     "//components/resources",
     "//components/strings",
-    "//ui/resources",
-    "//ui/strings",
   ]
 }
 

components/components_tests.gyp

@@ -620,8 +620,6 @@
       'target_name': 'components_tests_pak',
       'type': 'none',
       'dependencies': [
-        '../ui/resources/ui_resources.gyp:ui_resources',
-        '../ui/strings/ui_strings.gyp:ui_strings',
         'components_resources.gyp:components_resources',
         'components_strings.gyp:components_strings',
       ],
@@ -632,10 +630,6 @@
             'pak_inputs': [
               '<(SHARED_INTERMEDIATE_DIR)/components/components_resources.pak',
               '<(SHARED_INTERMEDIATE_DIR)/components/strings/components_strings_en-US.pak',
-              '<(SHARED_INTERMEDIATE_DIR)/ui/resources/ui_resources_100_percent.pak',
-              '<(SHARED_INTERMEDIATE_DIR)/ui/resources/webui_resources.pak',
-              '<(SHARED_INTERMEDIATE_DIR)/ui/strings/app_locale_settings_en-US.pak',
-              '<(SHARED_INTERMEDIATE_DIR)/ui/strings/ui_strings_en-US.pak',
             ],
             'pak_output': '<(PRODUCT_DIR)/components_tests_resources.pak',
           },
@@ -735,6 +729,7 @@
         '../ui/gfx/gfx.gyp:gfx',
         '../ui/gfx/gfx.gyp:gfx_test_support',
         '../ui/resources/ui_resources.gyp:ui_resources',
+        '../ui/resources/ui_resources.gyp:ui_test_pak',
         '../ui/strings/ui_strings.gyp:ui_strings',
         '../url/url.gyp:url_lib',
         'components.gyp:auto_login_parser',
@@ -940,6 +935,9 @@
             # component directory structure).
             ['exclude', '^[^/]*/content/'],
           ],
+          'mac_bundle_resources': [
+            '<(PRODUCT_DIR)/ui_test.pak',
+          ],
           'dependencies': [
             '../ios/ios_tests.gyp:test_support_ios',
             '../ios/web/ios_web.gyp:test_support_ios_web',

components/components_unittests.isolate

@@ -39,6 +39,7 @@
         'files': [
           'test/data/',
           '<(PRODUCT_DIR)/components_tests_resources.pak',
+          '<(PRODUCT_DIR)/ui_test.pak',
         ],
       },
     }],

components/test/run_all_unittests.cc

@@ -61,8 +61,14 @@ class ComponentsTestSuite : public base::TestSuite {
 #else
     PathService::Get(base::DIR_MODULE, &pak_path);
 #endif
-    ui::ResourceBundle::InitSharedInstanceWithPakPath(
-        pak_path.AppendASCII("components_tests_resources.pak"));
+
+    base::FilePath ui_test_pak_path;
+    ASSERT_TRUE(PathService::Get(ui::UI_TEST_PAK, &ui_test_pak_path));
+    ui::ResourceBundle::InitSharedInstanceWithPakPath(ui_test_pak_path);
+
+    ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath(
+        pak_path.AppendASCII("components_tests_resources.pak"),
+        ui::SCALE_FACTOR_NONE);
 
     // These schemes need to be added globally to pass tests of
     // autocomplete_input_unittest.cc and content_settings_pattern*

tools/gritsettings/resource_ids

@@ -216,7 +216,7 @@
     "includes": [28050],
   },
   "ui/file_manager/file_manager_resources.grd": {
-    "includes": [28100],
+    "includes": [28110],
   },
   "components/chrome_apps/chrome_apps_resources.grd": {
     "includes": [28250],

ui/base/resource/data_pack.cc

@@ -231,6 +231,23 @@ ui::ScaleFactor DataPack::GetScaleFactor() const {
   return scale_factor_;
 }
 
+#if DCHECK_IS_ON()
+void DataPack::CheckForDuplicateResources(
+    const ScopedVector<ResourceHandle>& packs) {
+  for (size_t i = 0; i < resource_count_ + 1; ++i) {
+    const DataPackEntry* entry = reinterpret_cast<const DataPackEntry*>(
+        mmap_->data() + kHeaderLength + (i * sizeof(DataPackEntry)));
+    const uint16 resource_id = entry->resource_id;
+    const float resource_scale = GetScaleForScaleFactor(scale_factor_);
+    for (const ResourceHandle* handle : packs) {
+      DCHECK_IMPLIES(
+          GetScaleForScaleFactor(handle->GetScaleFactor()) == resource_scale,
+          !handle->HasResource(resource_id));
+    }
+  }
+}
+#endif  // DCHECK_IS_ON()
+
 // static
 bool DataPack::WritePack(const base::FilePath& path,
                          const std::map<uint16, base::StringPiece>& resources,

ui/base/resource/data_pack.h

@@ -15,6 +15,7 @@
 #include "base/files/file.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/strings/string_piece.h"
 #include "ui/base/resource/resource_handle.h"
 #include "ui/base/ui_base_export.h"
@@ -59,6 +60,12 @@ class UI_BASE_EXPORT DataPack : public ResourceHandle {
   TextEncodingType GetTextEncodingType() const override;
   ui::ScaleFactor GetScaleFactor() const override;
 
+#if DCHECK_IS_ON()
+  // Checks to see if any resource in this DataPack already exists in the list
+  // of resources.
+  void CheckForDuplicateResources(const ScopedVector<ResourceHandle>& packs);
+#endif
+
  private:
   // Does the actual loading of a pack file. Called by Load and LoadFromFile.
   bool LoadImpl();

ui/base/resource/resource_bundle.cc

@@ -662,6 +662,9 @@ void ResourceBundle::AddDataPackFromPathInternal(const base::FilePath& path,
 }
 
 void ResourceBundle::AddDataPack(DataPack* data_pack) {
+#if DCHECK_IS_ON()
+  data_pack->CheckForDuplicateResources(data_packs_);
+#endif
   data_packs_.push_back(data_pack);
 
   if (GetScaleForScaleFactor(data_pack->GetScaleFactor()) >

ui/base/ui_base_paths.cc

@@ -60,8 +60,13 @@ bool PathProvider(int key, base::FilePath* result) {
       break;
 #endif
     case UI_TEST_PAK:
+#if defined(OS_ANDROID)
+      if (!PathService::Get(ui::DIR_RESOURCE_PAKS_ANDROID, &cur))
+        return false;
+#else
       if (!PathService::Get(base::DIR_MODULE, &cur))
         return false;
+#endif
       cur = cur.AppendASCII("ui_test.pak");
       break;
     default: