Componentize script to generate UI string overrides mapping.
Change the API of the script to generate both the header and source file
and to take a list of header files as input (to allow overridding strings
from components/components_strings.grd).
The script is now generating a method CreateUIStringOverrider() and hides
the arrays in the implementation file and allow customization of the
namespace in which the function and code is generated.
Introduce a new target "chrome_ui_string_overrider_factory" in the gyp
build to mimic the same target in the gn build (compiles the source file
generated by the script).
Rename the script from to generate_ui_string_overrider.py to reflect the
new role of the script and move it to components/variations/service.
Introduce .gni file to help using the script by different embedders (don't
introduce .gypi files as it is much harder to share code for gyp).
Componentize ui_string_overrider_unittest.cc.
BUG=534257
Review URL: https://codereview.chromium.org/1374773002
Cr-Commit-Position: refs/heads/master@{#352308}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 9fa3cf05..dab46f1 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -93,9 +93,9 @@
"//chrome:extra_resources",
"//chrome:resources",
"//chrome:strings",
- "//chrome/app:generated_resources_map",
"//chrome/app/resources:platform_locale_settings",
"//chrome/app/theme:theme_resources",
+ "//chrome/browser/metrics/variations:chrome_ui_string_overrider_factory",
"//chrome/browser/net:probe_message_proto",
"//chrome/browser/ui",
"//chrome/common",
@@ -264,9 +264,9 @@
deps += [
"//apps",
"//cc",
- "//chrome/app:generated_resources_map",
"//chrome/app/theme:theme_resources",
"//chrome/browser/devtools",
+ "//chrome/browser/metrics/variations:chrome_ui_string_overrider_factory",
"//chrome/browser/resources:component_extension_resources",
"//chrome/common/net",
"//chrome/installer/util",
diff --git a/chrome/browser/metrics/metrics_services_manager.cc b/chrome/browser/metrics/metrics_services_manager.cc
index a981620..6e71aa1 100644
--- a/chrome/browser/metrics/metrics_services_manager.cc
+++ b/chrome/browser/metrics/metrics_services_manager.cc
@@ -14,7 +14,7 @@
#include "chrome/browser/metrics/chrome_metrics_service_client.h"
#include "chrome/browser/metrics/metrics_reporting_state.h"
#include "chrome/browser/metrics/variations/chrome_variations_service_client.h"
-#include "chrome/browser/metrics/variations/generated_resources_map.h"
+#include "chrome/browser/metrics/variations/ui_string_overrider_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/browser_otr_state.h"
@@ -69,9 +69,7 @@
variations_service_ = variations::VariationsService::Create(
make_scoped_ptr(new ChromeVariationsServiceClient()), local_state_,
GetMetricsStateManager(), switches::kDisableBackgroundNetworking,
- variations::UIStringOverrider(chrome_variations::kResourceHashes,
- chrome_variations::kResourceIndices,
- chrome_variations::kNumResources));
+ chrome_variations::CreateUIStringOverrider());
}
return variations_service_.get();
}
diff --git a/chrome/browser/metrics/variations/BUILD.gn b/chrome/browser/metrics/variations/BUILD.gn
new file mode 100644
index 0000000..ed39b8c
--- /dev/null
+++ b/chrome/browser/metrics/variations/BUILD.gn
@@ -0,0 +1,18 @@
+# Copyright 2015 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.
+
+import("//components/variations/service/generate_ui_string_overrider.gni")
+
+# GYP version: chrome/chrome_resources.gyp:chrome_ui_string_overrider_factory
+generate_ui_string_overrider("chrome_ui_string_overrider_factory") {
+ inputs = [
+ "$root_gen_dir/chrome/grit/generated_resources.h",
+ ]
+ deps = [
+ "//chrome/app:generated_resources",
+ ]
+ namespace = "chrome_variations"
+ header_filename = "ui_string_overrider_factory.h"
+ source_filename = "ui_string_overrider_factory.cc"
+}
diff --git a/chrome/browser/metrics/variations/generate_resources_map.py b/chrome/browser/metrics/variations/generate_resources_map.py
deleted file mode 100755
index 6cf535b..0000000
--- a/chrome/browser/metrics/variations/generate_resources_map.py
+++ /dev/null
@@ -1,183 +0,0 @@
-#!/usr/bin/python
-# Copyright 2014 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.
-
-import collections
-import hashlib
-import operator
-import os
-import re
-import sys
-
-
-RESOURCE_EXTRACT_REGEX = re.compile('^#define (\S*) (\d*)$', re.MULTILINE)
-
-class Error(Exception):
- """Base error class for all exceptions in generated_resources_map."""
-
-
-class HashCollisionError(Error):
- """Multiple resource names hash to the same value."""
-
-
-Resource = collections.namedtuple("Resource", ['hash', 'name', 'index'])
-
-
-def _HashName(name):
- """Returns the hash id for a name.
-
- Args:
- name: The name to hash.
-
- Returns:
- An int that is at most 32 bits.
- """
- md5hash = hashlib.md5()
- md5hash.update(name)
- return int(md5hash.hexdigest()[:8], 16)
-
-
-def _GetNameIndexPairsIter(string_to_scan):
- """Gets an iterator of the resource name and index pairs of the given string.
-
- Scans the input string for lines of the form "#define NAME INDEX" and returns
- an iterator over all matching (NAME, INDEX) pairs.
-
- Args:
- string_to_scan: The input string to scan.
-
- Yields:
- A tuple of name and index.
- """
- for match in RESOURCE_EXTRACT_REGEX.finditer(string_to_scan):
- yield match.group(1, 2)
-
-
-def _GetResourceListFromString(resources_content):
- """Produces a list of |Resource| objects from a string.
-
- The input string conaints lines of the form "#define NAME INDEX". The returned
- list is sorted primarily by hash, then name, and then index.
-
- Args:
- resources_content: The input string to process, contains lines of the form
- "#define NAME INDEX".
-
- Returns:
- A sorted list of |Resource| objects.
- """
- resources = [Resource(_HashName(name), name, index) for name, index in
- _GetNameIndexPairsIter(resources_content)]
-
- # The default |Resource| order makes |resources| sorted by the hash, then
- # name, then index.
- resources.sort()
-
- return resources
-
-
-def _CheckForHashCollisions(sorted_resource_list):
- """Checks a sorted list of |Resource| objects for hash collisions.
-
- Args:
- sorted_resource_list: A sorted list of |Resource| objects.
-
- Returns:
- A set of all |Resource| objects with collisions.
- """
- collisions = set()
- for i in xrange(len(sorted_resource_list) - 1):
- resource = sorted_resource_list[i]
- next_resource = sorted_resource_list[i+1]
- if resource.hash == next_resource.hash:
- collisions.add(resource)
- collisions.add(next_resource)
-
- return collisions
-
-
-def _GenDataArray(
- resources, entry_pattern, array_name, array_type, data_getter):
- """Generates a C++ statement defining a literal array containing the hashes.
-
- Args:
- resources: A sorted list of |Resource| objects.
- entry_pattern: A pattern to be used to generate each entry in the array. The
- pattern is expected to have a place for data and one for a comment, in
- that order.
- array_name: The name of the array being generated.
- array_type: The type of the array being generated.
- data_getter: A function that gets the array data from a |Resource| object.
-
- Returns:
- A string containing a C++ statement defining the an array.
- """
- lines = [entry_pattern % (data_getter(r), r.name) for r in resources]
- pattern = """const %(type)s %(name)s[] = {
-%(content)s
-};
-"""
- return pattern % {'type': array_type,
- 'name': array_name,
- 'content': '\n'.join(lines)}
-
-
-def _GenerateFileContent(resources_content):
- """Generates the .cc content from the given generated_resources.h content.
-
- Args:
- resources_content: The input string to process, contains lines of the form
- "#define NAME INDEX".
-
- Returns:
- .cc file content defining the kResourceHashes and kResourceIndices arrays.
- """
- hashed_tuples = _GetResourceListFromString(resources_content)
-
- collisions = _CheckForHashCollisions(hashed_tuples)
- if collisions:
- error_message = "\n".join(
- ["hash: %i, name: %s" % (i[0], i[1]) for i in sorted(collisions)])
- error_message = ("\nThe following names had hash collisions "
- "(sorted by the hash value):\n%s\n" %(error_message))
- raise HashCollisionError(error_message)
-
- hashes_array = _GenDataArray(
- hashed_tuples, " %iU, // %s", 'kResourceHashes', 'uint32_t',
- operator.attrgetter('hash'))
- indices_array = _GenDataArray(
- hashed_tuples, " %s, // %s", 'kResourceIndices', 'int',
- operator.attrgetter('index'))
-
- return (
- "// This file was generated by generate_resources_map.py. Do not edit.\n"
- "\n\n"
- "#include "
- "\"chrome/browser/metrics/variations/generated_resources_map.h\"\n\n"
- "namespace chrome_variations {\n\n"
- "const size_t kNumResources = %i;\n\n"
- "%s"
- "\n"
- "%s"
- "\n"
- "} // namespace chrome_variations\n") % (
- len(hashed_tuples), hashes_array, indices_array)
-
-
-def main(resources_file, map_file):
- generated_resources_h = ""
- with open(resources_file, "r") as resources:
- generated_resources_h = resources.read()
-
- if len(generated_resources_h) == 0:
- raise Error("No content loaded for %s." % (resources_file))
-
- file_content = _GenerateFileContent(generated_resources_h)
-
- with open(map_file, "w") as generated_file:
- generated_file.write(file_content)
-
-
-if __name__ == '__main__':
- sys.exit(main(sys.argv[1], sys.argv[2]))
diff --git a/chrome/browser/metrics/variations/generate_resources_map_unittest.py b/chrome/browser/metrics/variations/generate_resources_map_unittest.py
deleted file mode 100755
index a7b1a2b..0000000
--- a/chrome/browser/metrics/variations/generate_resources_map_unittest.py
+++ /dev/null
@@ -1,93 +0,0 @@
-#!/usr/bin/python
-# Copyright 2014 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.
-
-"""Unittests for generate_resources_map.py"""
-
-import unittest
-
-import generate_resources_map
-
-
-class GenerateResourcesMapUnittest(unittest.TestCase):
- TEST_INPUT = """
-// This file is automatically generated by GRIT. Do not edit.
-
-#pragma once
-
-#define IDS_BOOKMARKS_NO_ITEMS 12500
-#define IDS_BOOKMARK_BAR_IMPORT_LINK 12501
-#define IDS_BOOKMARK_GROUP_FROM_IE 12502
-#define IDS_BOOKMARK_GROUP_FROM_FIREFOX 12503
-"""
-
- def testGetResourceListFromString(self):
- expected_tuples = [(301430091, "IDS_BOOKMARKS_NO_ITEMS", "12500"),
- (2654138887, "IDS_BOOKMARK_BAR_IMPORT_LINK", "12501"),
- (2894469061, "IDS_BOOKMARK_GROUP_FROM_IE", "12502"),
- (3847176170, "IDS_BOOKMARK_GROUP_FROM_FIREFOX", "12503")]
- expected = [generate_resources_map.Resource(*t) for t in expected_tuples]
-
- actual_tuples = generate_resources_map._GetResourceListFromString(
- self.TEST_INPUT)
-
- self.assertEqual(expected_tuples, actual_tuples)
-
-
- def testCheckForHashCollisions(self):
- collisions_tuples = [(123, "IDS_FOO", "12500"),
- (456, "IDS_BAR", "12501"),
- (456, "IDS_BAZ", "12502"),
- (890, "IDS_QUX", "12503"),
- (899, "IDS_NO", "12504"),
- (899, "IDS_YES", "12505")]
- list_with_collisions = [generate_resources_map.Resource(*t)
- for t in collisions_tuples]
-
- expected_collision_tuples = [(456, "IDS_BAR", "12501"),
- (456, "IDS_BAZ", "12502"),
- (899, "IDS_NO", "12504"),
- (899, "IDS_YES", "12505")]
- expected_collisions = [generate_resources_map.Resource(*t)
- for t in expected_collision_tuples]
-
- actual_collisions = sorted(
- generate_resources_map._CheckForHashCollisions(list_with_collisions))
- actual_collisions
-
- self.assertEqual(expected_collisions, actual_collisions)
-
- def testGenerateFileContent(self):
- expected = (
- """// This file was generated by generate_resources_map.py. Do not edit.
-
-
-#include "chrome/browser/metrics/variations/generated_resources_map.h"
-
-namespace chrome_variations {
-
-const size_t kNumResources = 4;
-
-const uint32_t kResourceHashes[] = {
- 301430091U, // IDS_BOOKMARKS_NO_ITEMS
- 2654138887U, // IDS_BOOKMARK_BAR_IMPORT_LINK
- 2894469061U, // IDS_BOOKMARK_GROUP_FROM_IE
- 3847176170U, // IDS_BOOKMARK_GROUP_FROM_FIREFOX
-};
-
-const int kResourceIndices[] = {
- 12500, // IDS_BOOKMARKS_NO_ITEMS
- 12501, // IDS_BOOKMARK_BAR_IMPORT_LINK
- 12502, // IDS_BOOKMARK_GROUP_FROM_IE
- 12503, // IDS_BOOKMARK_GROUP_FROM_FIREFOX
-};
-
-} // namespace chrome_variations
-""")
- actual = generate_resources_map._GenerateFileContent(self.TEST_INPUT)
-
- self.assertEqual(expected, actual)
-
-if __name__ == '__main__':
- unittest.main()
diff --git a/chrome/browser/metrics/variations/generated_resources_map.h b/chrome/browser/metrics/variations/generated_resources_map.h
deleted file mode 100644
index c4c6cd86..0000000
--- a/chrome/browser/metrics/variations/generated_resources_map.h
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef CHROME_BROWSER_METRICS_VARIATIONS_GENERATED_RESOURCES_MAP_H_
-#define CHROME_BROWSER_METRICS_VARIATIONS_GENERATED_RESOURCES_MAP_H_
-
-#include "base/basictypes.h"
-
-namespace chrome_variations {
-
-// This file provides a mapping from hashes of generated resource names to their
-// IDs. This mapping is achieved by having two arrays: |kResourceHashes|, a
-// sorted array of resource name hashes; and |kResourceIndices|, an array of
-// resource indices in the same order as |kResourceHashes|. So, if
-// generated_resources.h contains |#define IDS_FOO 12345|, then for some index i
-// kResourceHashes[i] = HASH("IDS_FOO") and kResourceIndices[i] = 12345. Both
-// arrays are of length |kNumResources|.
-
-// The definitions of the arrays are generated by generate_resources_map.py from
-// the content of generated_resources.h.
-
-// Length of |kResourceHashes| and |kResourceIndices|.
-extern const size_t kNumResources;
-
-// A sorted array of hashed generated resource names.
-extern const uint32_t kResourceHashes[];
-
-// An array of generated resource indices. The order of this array corresponds
-// to the order of |kResourceHashes|.
-extern const int kResourceIndices[];
-
-} // namespace chrome_variations
-
-#endif // CHROME_BROWSER_METRICS_VARIATIONS_GENERATED_RESOURCES_MAP_H_
diff --git a/chrome/browser/metrics/variations/ui_string_overrider_unittest.cc b/chrome/browser/metrics/variations/ui_string_overrider_unittest.cc
deleted file mode 100644
index 5aace0e..0000000
--- a/chrome/browser/metrics/variations/ui_string_overrider_unittest.cc
+++ /dev/null
@@ -1,45 +0,0 @@
-// Copyright 2014 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 "components/variations/service/ui_string_overrider.h"
-
-#include "chrome/browser/metrics/variations/generated_resources_map.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-// TODO(sdefresne): componentize this tests as part of the resolution
-// of http://crbug.com/534257 ("Decouple iOS port from using //chrome's
-// VariationsService client").
-
-namespace chrome_variations {
-
-class UIStringOverriderTest : public ::testing::Test {
- public:
- UIStringOverriderTest()
- : provider_(kResourceHashes, kResourceIndices, kNumResources) {}
-
- int GetResourceIndex(uint32_t hash) {
- return provider_.GetResourceIndex(hash);
- }
-
- private:
- variations::UIStringOverrider provider_;
-
- DISALLOW_COPY_AND_ASSIGN(UIStringOverriderTest);
-};
-
-TEST_F(UIStringOverriderTest, LookupNotFound) {
- EXPECT_EQ(-1, GetResourceIndex(0));
- EXPECT_EQ(-1, GetResourceIndex(kResourceHashes[kNumResources - 1] + 1));
-
- // Lookup a hash that shouldn't exist.
- // 13257681U is 1 + the hash for IDS_ZOOM_NORMAL
- EXPECT_EQ(-1, GetResourceIndex(13257681U));
-}
-
-TEST_F(UIStringOverriderTest, LookupFound) {
- for (size_t i = 0; i < kNumResources; ++i)
- EXPECT_EQ(kResourceIndices[i], GetResourceIndex(kResourceHashes[i]));
-}
-
-} // namespace chrome_variations