Fix regex to parse Variations UI override strings

components/variations/service/generate_ui_string_overrider.py

Index: components/variations/service/generate_ui_string_overrider.py
diff --git a/components/variations/service/generate_ui_string_overrider.py b/components/variations/service/generate_ui_string_overrider.py
index 6e43559d60f99cf65f57439ef6ab8ce20c8e5431..d15228cceda8f7683f3ce8f98191eff41343f8f2 100755
--- a/components/variations/service/generate_ui_string_overrider.py
+++ b/components/variations/service/generate_ui_string_overrider.py
@@ -12,7 +12,19 @@ import re
 import sys
 
 SCRIPT_NAME = "generate_ui_string_overrider.py"
+

[Raj, 2016/05/12 01:40:27]

The pragma format is defined in
https://code.google.com/p/chromium/codesearch#chromium/src/tools/grit/grit_ru...

+
+# Regular expressions for parsing the #define macro format. Separate regular
+# expressions are used for parsing lines with pragma (for builds with
+# enable_resource_whitelist_generation flag) in windows and non-windows, and for
+# lines without pragma,

[rkaplow, 2016/05/12 01:52:53]

could you give samples for the cases here

[Raj, 2016/05/12 17:03:40]

Done.

 RESOURCE_EXTRACT_REGEX = re.compile('^#define (\S*) (\d*)$', re.MULTILINE)
+RESOURCE_EXTRACT_REGEX_PRAGMA = re.compile(
+      '^#define (\S*) _Pragma\("whitelisted_resource_\d*"\) (\d*)$',

[Alexei Svitkine (slow), 2016/05/12 14:40:41]

Looks like these are defined here:

https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...

Can you add a comment both to this file referencing those locations and to the
GYPI and GN files referencing this script and mentioning that they have to be
kept in sync.

Also, I wonder if it makes sense to make this Python script fail (causing a
build error) if it found no entries matching the regexes.

[Raj, 2016/05/12 17:03:40]

Sure. I will add the comment in a separate CL. I do not want to wait for another
owner approval, which may delay this CL merge to M51. I just want to keep this
CL small for easy merge. WDYT.

Yes. It is better to fail this script when no entries are found. In fact we
should parse line-by-line and if one line does not match any of the regex, the
script could fail. I will add this in next CL, which may need more testing with
many platforms. WDYT.

+      re.MULTILINE)
+RESOURCE_EXTRACT_REGEX_PRAGMA_WINDOWS = re.compile(

[Raj, 2016/05/12 01:40:27]

Not tested with Windows yet. Will test asap.

[Raj, 2016/05/12 01:47:29]

I tested in linux for android build, that builds with or without
enable_resource_whitelist_generation generate the same
gen/chrome/browser/metrics/variations/ui_string_overrider_factory.cc

+      '^#define (\S*) __pragma\(message\("whitelisted_resource_\d*"\)\) (\d*)$',

[rkaplow, 2016/05/12 01:52:53]

just to verify this isn't a typo - this is lowercase pragma compared to the
other regex?

[Raj, 2016/05/12 17:03:40]

Yes. The pragma format is defined in:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/grit/grit_ru...

+      re.MULTILINE)
 
 class Error(Exception):
   """Base error class for all exceptions in generated_resources_map."""
@@ -53,6 +65,10 @@ def _GetNameIndexPairsIter(string_to_scan):
   """
   for match in RESOURCE_EXTRACT_REGEX.finditer(string_to_scan):
     yield match.group(1, 2)
+  for match in RESOURCE_EXTRACT_REGEX_PRAGMA.finditer(string_to_scan):
+    yield match.group(1, 2)
+  for match in RESOURCE_EXTRACT_REGEX_PRAGMA_WINDOWS.finditer(string_to_scan):
+    yield match.group(1, 2)
 
 
 def _GetResourceListFromString(resources_content):