Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7945)

Unified Diff: chrome/installer/util/prebuild/create_string_rc.py

Issue 2791593002: Allow installer::GetLocalizedString to return mode-specific strings. (Closed)
Patch Set: Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/installer/util/prebuild/create_string_rc.py
diff --git a/chrome/installer/util/prebuild/create_string_rc.py b/chrome/installer/util/prebuild/create_string_rc.py
index 9f73dacbfc2857f64f95d9245bf4b3ca43fe9fbd..05bca8f4b995c092de47a947feab3b35609a855c 100755
--- a/chrome/installer/util/prebuild/create_string_rc.py
+++ b/chrome/installer/util/prebuild/create_string_rc.py
@@ -45,32 +45,98 @@ from grit.extern import tclib
# The IDs of strings we want to import from the .grd files and include in
# setup.exe's resources.
manzagop (departed) 2017/03/31 16:19:01 nit: is it common practice to mention the alphabet
grt (UTC plus 2) 2017/04/03 11:59:42 There's no technical requirement, so I don't think
STRING_IDS = [
- 'IDS_PRODUCT_NAME',
- 'IDS_SXS_SHORTCUT_NAME',
- 'IDS_PRODUCT_DESCRIPTION',
'IDS_ABOUT_VERSION_COMPANY_NAME',
- 'IDS_INSTALL_HIGHER_VERSION',
+ 'IDS_APP_SHORTCUTS_SUBDIR_NAME',
+ 'IDS_APP_SHORTCUTS_SUBDIR_NAME_CANARY',
+ 'IDS_INBOUND_MDNS_RULE_DESCRIPTION',
+ 'IDS_INBOUND_MDNS_RULE_DESCRIPTION_CANARY',
+ 'IDS_INBOUND_MDNS_RULE_NAME',
+ 'IDS_INBOUND_MDNS_RULE_NAME_CANARY',
+ 'IDS_INSTALL_EXISTING_VERSION_LAUNCHED',
'IDS_INSTALL_FAILED',
- 'IDS_SAME_VERSION_REPAIR_FAILED',
- 'IDS_SETUP_PATCH_FAILED',
- 'IDS_INSTALL_OS_NOT_SUPPORTED',
+ 'IDS_INSTALL_HIGHER_VERSION',
+ 'IDS_INSTALL_INSUFFICIENT_RIGHTS',
+ 'IDS_INSTALL_INVALID_ARCHIVE',
'IDS_INSTALL_OS_ERROR',
+ 'IDS_INSTALL_OS_NOT_SUPPORTED',
'IDS_INSTALL_SINGLETON_ACQUISITION_FAILED',
'IDS_INSTALL_TEMP_DIR_FAILED',
'IDS_INSTALL_UNCOMPRESSION_FAILED',
- 'IDS_INSTALL_INVALID_ARCHIVE',
- 'IDS_INSTALL_INSUFFICIENT_RIGHTS',
- 'IDS_SHORTCUT_TOOLTIP',
+ 'IDS_PRODUCT_DESCRIPTION',
+ 'IDS_PRODUCT_NAME',
+ 'IDS_SAME_VERSION_REPAIR_FAILED',
+ 'IDS_SETUP_PATCH_FAILED',
'IDS_SHORTCUT_NEW_WINDOW',
- 'IDS_APP_SHORTCUTS_SUBDIR_NAME',
- 'IDS_APP_SHORTCUTS_SUBDIR_NAME_CANARY',
- 'IDS_INBOUND_MDNS_RULE_NAME',
- 'IDS_INBOUND_MDNS_RULE_NAME_CANARY',
- 'IDS_INBOUND_MDNS_RULE_DESCRIPTION',
- 'IDS_INBOUND_MDNS_RULE_DESCRIPTION_CANARY',
- 'IDS_INSTALL_EXISTING_VERSION_LAUNCHED',
+ 'IDS_SHORTCUT_TOOLTIP',
+ 'IDS_SXS_SHORTCUT_NAME',
]
+# Certain strings are conditional on a brand's install mode (see
+# chrome/install_static/install_modes.h for details). This allows
+# installer::GetLocalizedString to return a resource specific to the current
+# install mode at runtime (e.g., "Google Chrome SxS" as IDS_SHORTCUT_NAME for
+# the localized shortcut name for Google Chrome's canary channel). This mapping
+# provides brand- and mode-specific string ids for a given input id as described
+# here:
+# {
+# resource_id_1: { # A resource ID for use with GetLocalizedString.
+# brand: [ # 'google_chrome', for example.
+# string_id_1, # Strings listed in order of the brand's modes, as
manzagop (departed) 2017/03/31 16:19:00 Add a comment in the mode file that this needs to
grt (UTC plus 2) 2017/04/03 11:59:42 Done.
+# string_id_2, # specified in install_static::InstallConstantIndex.
+# ...
+# string_id_N,
+# ],
+# '': [ # Default brand fallback (Chromium).
+# string_id_1,
+# ],
+# },
+# resource_id_2: ...
+# }
+# 'resource_id_1' may name an existing string ID or may be distinct. In the
+# former case, all calls to installer::GetLocalizedString with the existing
+# string ID will map to the mode-specific string. It becomes impossible to get
+# the general ("primary") variant of the string when a secondary install mode
+# is in use. By using a distinct ID, both the mode-specific and the general can
+# be retrieved as needed.
manzagop (departed) 2017/03/31 16:19:01 (rambling) IIUC we have string ids, and a layer o
grt (UTC plus 2) 2017/04/03 11:59:42 I introduced the pseudo-id to deal with IDS_PRODUC
grt (UTC plus 2) 2017/04/03 11:59:42 The hybrid was just something I dreamed up while t
+MODE_SPECIFIC_STRINGS = {
+ 'IDS_APP_SHORTCUTS_SUBDIR_NAME': {
+ 'google_chrome': [
manzagop (departed) 2017/03/31 16:19:01 nit: use a constant? For the default brand too?
grt (UTC plus 2) 2017/04/03 11:59:42 This matches the -b BRAND value provided on the co
manzagop (departed) 2017/04/03 13:06:33 Oh, I meant introduce constants GOOGLE_BRAND and C
grt (UTC plus 2) 2017/04/03 13:31:30 Okay. The new validation I added should catch any
+ 'IDS_APP_SHORTCUTS_SUBDIR_NAME',
+ 'IDS_APP_SHORTCUTS_SUBDIR_NAME_CANARY',
+ ],
+ '': [
+ 'IDS_APP_SHORTCUTS_SUBDIR_NAME',
+ ],
+ },
+ 'IDS_INBOUND_MDNS_RULE_DESCRIPTION': {
+ 'google_chrome': [
+ 'IDS_INBOUND_MDNS_RULE_DESCRIPTION',
+ 'IDS_INBOUND_MDNS_RULE_DESCRIPTION_CANARY',
+ ],
+ '': [
+ 'IDS_INBOUND_MDNS_RULE_DESCRIPTION',
+ ],
+ },
+ 'IDS_INBOUND_MDNS_RULE_NAME': {
+ 'google_chrome': [
+ 'IDS_INBOUND_MDNS_RULE_NAME',
+ 'IDS_INBOUND_MDNS_RULE_NAME_CANARY',
+ ],
+ '': [
+ 'IDS_INBOUND_MDNS_RULE_NAME',
+ ],
+ },
+ 'IDS_SHORTCUT_NAME': {
+ 'google_chrome': [
+ 'IDS_PRODUCT_NAME',
manzagop (departed) 2017/03/31 16:19:01 Can you speak to the tradeoffs of introducing a ne
grt (UTC plus 2) 2017/04/03 11:59:42 Does my response above address this question?
manzagop (departed) 2017/04/03 13:06:33 Yep! Thanks.
+ 'IDS_SXS_SHORTCUT_NAME',
+ ],
+ '': [
+ 'IDS_PRODUCT_NAME',
+ ],
+ },
+}
+
# The ID of the first resource string.
FIRST_RESOURCE_ID = 1600
@@ -215,8 +281,8 @@ class XtbHandler(sax.handler.ContentHandler):
class StringRcMaker(object):
"""Makes .h and .rc files containing strings and translations."""
- def __init__(self, name, inputs, outdir):
- """Constructs a maker.
+ def __init__(self, name, inputs, outdir, brand):
+ """constructs a maker.
manzagop (departed) 2017/03/31 16:19:01 nitty question: no capital letter?
grt (UTC plus 2) 2017/04/03 11:59:42 I wasn't aware I'd changed that. Oops.
Args:
name: The base name of the generated files (e.g.,
@@ -227,6 +293,7 @@ class StringRcMaker(object):
self.name = name
self.inputs = inputs
self.outdir = outdir
+ self.brand = brand
def MakeFiles(self):
translated_strings = self.__ReadSourceAndTranslatedStrings()
@@ -347,6 +414,7 @@ class StringRcMaker(object):
lines = []
do_languages_lines = ['\n#define DO_LANGUAGES']
installer_string_mapping_lines = ['\n#define DO_INSTALLER_STRING_MAPPING']
+ do_mode_strings_lines = ['\n#define DO_MODE_STRINGS']
# Write the values for how the languages ids are offset.
seen_languages = set()
@@ -370,6 +438,18 @@ class StringRcMaker(object):
resource_id))
resource_id += 1
+ # Handle mode-specific strings.
+ for string_id, brands in MODE_SPECIFIC_STRINGS.iteritems():
+ # Write the synthetic resource ids for per-mode strings.
+ if string_id not in STRING_IDS:
+ lines.append('#define %s_BASE %s' % (string_id, resource_id))
+ resource_id += 1
+ # Populate the DO_MODE_STRINGS macro.
+ brand_strings = brands.get(self.brand, brands[''])
+ do_mode_strings_lines.append(
+ ' HANDLE_MODE_STRING(%s_BASE, %s)'
+ % (string_id, ', '.join([ ('%s_BASE' % s) for s in brand_strings])))
manzagop (departed) 2017/03/31 16:19:00 Should we validate that no brand string is a pseud
grt (UTC plus 2) 2017/04/03 11:59:42 noop -- i've removed psuedo strings
+
# Write out base ID values.
for string_id in STRING_IDS:
lines.append('#define %s_BASE %s_%s' % (string_id,
@@ -383,6 +463,7 @@ class StringRcMaker(object):
outfile.write('\n#ifndef RC_INVOKED')
outfile.write(' \\\n'.join(do_languages_lines))
outfile.write(' \\\n'.join(installer_string_mapping_lines))
+ outfile.write(' \\\n'.join(do_mode_strings_lines))
# .rc files must end in a new line
outfile.write('\n#endif // ndef RC_INVOKED\n')
@@ -398,6 +479,10 @@ def ParseCommandLine():
parser = argparse.ArgumentParser(
description='Generate .h and .rc files for installer strings.')
+ parser.add_argument('-b',
+ required=True,
+ help='identifier of the browser brand (e.g., chromium).',
+ dest='brand')
parser.add_argument('-i', action='append',
type=GrdPathAndXtbDirPair,
required=True,
@@ -417,7 +502,7 @@ def ParseCommandLine():
def main():
args = ParseCommandLine()
- StringRcMaker(args.name, args.inputs, args.outdir).MakeFiles()
+ StringRcMaker(args.name, args.inputs, args.outdir, args.brand).MakeFiles()
return 0

Powered by Google App Engine
This is Rietveld 408576698