Apply whitelist file to structure elements.

Resources specified with structure elements were being included in
.pak and .h files even if they were not included in a specified
whitelist file.

The .pak file issue is fixed in build.py by including structure nodes
in the collection of node types that are checked against the whitelist.
This seems to be an oversight as the code has a comment like "Apply
the same trick that data_pack.py uses" but the corresponding code in
data_pack.py was updated to include structure noded in [1]. This method
that contained this code is no longer in data_pack.py.

The .h file issue is fixed in rc_header.py by treating structure nodes
in the same manner as message nodes (i.e. only output them if they're
active). The correct fix seems to be to set output_all_resource_defines
to true in all the .grd files, but until then a structure element should
not generate an entry in a header file if it's not in the whitelist.

https://chromiumcodereview.appspot.com/10386189/diff/29002/grit/format/data_pack.py

BUG=None
R=joaodasilva@chromium.org, newt@chromium.org

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

Patch from Lane LiaBraaten <lliabraa@chromium.org>.

git-svn-id: http://grit-i18n.googlecode.com/svn/trunk@179 7262f16d-afe8-6277-6482-052fa10e57b1

5 files changed, 178 insertions, 1 deletions

diff --git a/grit/testdata/whitelist.txt b/grit/testdata/whitelist.txt
new file mode 100644
index 0000000..5b3aca4
--- /dev/null
+++ b/grit/testdata/whitelist.txt
@@ -0,0 +1,4 @@
+IDS_MESSAGE_WHITELISTED
+IDR_STRUCTURE_WHITELISTED
+IDR_STRUCTURE_IN_TRUE_IF_WHITELISTED
+IDR_INCLUDE_WHITELISTED
diff --git a/grit/testdata/whitelist_resources.grd b/grit/testdata/whitelist_resources.grd
new file mode 100644
index 0000000..a5eeb2d
--- /dev/null
+++ b/grit/testdata/whitelist_resources.grd
@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<grit latest_public_release="0"
+      current_release="1"
+      output_all_resource_defines="false">
+  <outputs>
+    <output filename="whitelist_test_resources.h" type="rc_header">
+      <emit emit_type='prepend'></emit>
+    </output>
+    <output filename="whitelist_test_resources_map.cc"
+            type="resource_file_map_source" />
+    <output filename="whitelist_test_resources_map.h"
+            type="resource_map_header" />
+    <output filename="whitelist_test_resources.pak" type="data_package" />
+  </outputs>
+  <translations>
+    <file path="substitute.xmb" lang="sv" />
+  </translations>
+    <release seq="1">
+    <structures>
+      <structure name="IDR_STRUCTURE_WHITELISTED" file="browser.html"
+                 type="chrome_html" >
+      </structure>
+      <structure name="IDR_STRUCTURE_NOT_WHITELISTED" file="deleted.html"
+                 type="chrome_html" >
+      </structure>
+      <if expr="True">
+        <structure name="IDR_STRUCTURE_IN_TRUE_IF_WHITELISTED"
+                   file="details.html"
+                   type="chrome_html" >
+        </structure>
+        <structure name="IDR_STRUCTURE_IN_TRUE_IF_NOT_WHITELISTED"
+                   file="error.html"
+                   type="chrome_html" >
+        </structure>
+      </if>
+      <if expr="False">
+        <structure name="IDR_STRUCTURE_IN_FALSE_IF_WHITELISTED"
+                   file="deleted.html"
+                   type="chrome_html" >
+        </structure>
+        <structure name="IDR_STRUCTURE_IN_FALSE_IF_NOT_WHITELISTED"
+                   file="deleted.html"
+                   type="chrome_html" >
+        </structure>
+      </if>
+    </structures>
+    <includes>
+      <include name="IDR_INCLUDE_WHITELISTED" file="klonk.ico"
+               type="BINDATA" />
+      <include name="IDR_INCLUDE_NOT_WHITELISTED" file="klonk.rc"
+               type="BINDATA" />
+    </includes>
+  </release>
+</grit>
diff --git a/grit/testdata/whitelist_strings.grd b/grit/testdata/whitelist_strings.grd
new file mode 100644
index 0000000..df80f5f
--- /dev/null
+++ b/grit/testdata/whitelist_strings.grd
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<grit latest_public_release="0"
+      current_release="1"
+      output_all_resource_defines="false">
+  <outputs >
+    <output filename="whitelist_test_resources.h" type="rc_header">
+      <emit emit_type='prepend'></emit>
+    </output>
+    <output filename="en_whitelist_test_strings.rc" type="rc_all" lang="en" />
+  </outputs>
+  <release seq="1">
+    <messages>
+      <message name="IDS_MESSAGE_WHITELISTED"
+               desc="A message in the whiltelist file.">
+        Whitelisted.
+      </message>
+      <message name="IDS_MESSAGE_NOT_WHITELISTED"
+               desc="A message that isn't in the whiltelist file.">
+        Not whitelisted.
+      </message>
+    </messages>
+  </release>
+</grit>
diff --git a/grit/tool/build.py b/grit/tool/build.py
index e72724f..186c8b1 100644
--- a/grit/tool/build.py
+++ b/grit/tool/build.py
@@ -224,11 +224,13 @@ are exported to translation interchange files (e.g. XMB files), etc.
     # be written into the target files (skip markers).
     from grit.node import include
     from grit.node import message
+    from grit.node import structure
     for node in start_node:
       # Same trick data_pack.py uses to see what nodes actually result in
       # real items.
       if (isinstance(node, include.IncludeNode) or
-          isinstance(node, message.MessageNode)):
+          isinstance(node, message.MessageNode) or
+          isinstance(node, structure.StructureNode)):
         text_ids = node.GetTextualIds()
         # Mark the item to be skipped if it wasn't in the whitelist.
         if text_ids and text_ids[0] not in whitelist_names:
diff --git a/grit/tool/build_unittest.py b/grit/tool/build_unittest.py
index debe4d4..d94b30e 100644
--- a/grit/tool/build_unittest.py
+++ b/grit/tool/build_unittest.py
@@ -6,6 +6,7 @@
 '''Unit tests for the 'grit build' tool.
 '''
 
+import codecs
 import os
 import sys
 import tempfile
@@ -85,5 +86,98 @@ class BuildUnittest(unittest.TestCase):
             '-a', os.path.abspath(
                 os.path.join(output_dir, 'resource.h'))]))
 
+  def _verifyWhitelistedOutput(self,
+                               filename,
+                               whitelisted_ids,
+                               non_whitelisted_ids,
+                               encoding='utf8'):
+    self.failUnless(os.path.exists(filename))
+    whitelisted_ids_found = []
+    non_whitelisted_ids_found = []
+    with codecs.open(filename, encoding=encoding) as f:
+      for line in f.readlines():
+        for whitelisted_id in whitelisted_ids:
+          if whitelisted_id in line:
+            whitelisted_ids_found.append(whitelisted_id)
+        for non_whitelisted_id in non_whitelisted_ids:
+          if non_whitelisted_id in line:
+            non_whitelisted_ids_found.append(non_whitelisted_id)
+    self.longMessage = True
+    self.assertEqual(whitelisted_ids,
+                     whitelisted_ids_found,
+                     '\nin file {}'.format(os.path.basename(filename)))
+    non_whitelisted_msg = ('Non-Whitelisted IDs {} found in {}'
+        .format(non_whitelisted_ids_found, os.path.basename(filename)))
+    self.assertFalse(non_whitelisted_ids_found, non_whitelisted_msg)
+
+  def testWhitelistStrings(self):
+    output_dir = tempfile.mkdtemp()
+    builder = build.RcBuilder()
+    class DummyOpts(object):
+      def __init__(self):
+        self.input = util.PathFromRoot('grit/testdata/whitelist_strings.grd')
+        self.verbose = False
+        self.extra_verbose = False
+    whitelist_file = util.PathFromRoot('grit/testdata/whitelist.txt')
+    builder.Run(DummyOpts(), ['-o', output_dir,
+                              '-w', whitelist_file])
+    header = os.path.join(output_dir, 'whitelist_test_resources.h')
+    rc = os.path.join(output_dir, 'en_whitelist_test_strings.rc')
+
+    whitelisted_ids = ['IDS_MESSAGE_WHITELISTED']
+    non_whitelisted_ids = ['IDS_MESSAGE_NOT_WHITELISTED']
+    self._verifyWhitelistedOutput(
+      header,
+      whitelisted_ids,
+      non_whitelisted_ids,
+    )
+    self._verifyWhitelistedOutput(
+      rc,
+      whitelisted_ids,
+      non_whitelisted_ids,
+      encoding='utf16'
+    )
+
+  def testWhitelistResources(self):
+    output_dir = tempfile.mkdtemp()
+    builder = build.RcBuilder()
+    class DummyOpts(object):
+      def __init__(self):
+        self.input = util.PathFromRoot('grit/testdata/whitelist_resources.grd')
+        self.verbose = False
+        self.extra_verbose = False
+    whitelist_file = util.PathFromRoot('grit/testdata/whitelist.txt')
+    builder.Run(DummyOpts(), ['-o', output_dir,
+                              '-w', whitelist_file])
+    header = os.path.join(output_dir, 'whitelist_test_resources.h')
+    map_cc = os.path.join(output_dir, 'whitelist_test_resources_map.cc')
+    map_h = os.path.join(output_dir, 'whitelist_test_resources_map.h')
+    pak = os.path.join(output_dir, 'whitelist_test_resources.pak')
+
+    # Ensure the resource map header and .pak files exist, but don't verify
+    # their content.
+    self.failUnless(os.path.exists(map_h))
+    self.failUnless(os.path.exists(pak))
+
+    whitelisted_ids = [
+        'IDR_STRUCTURE_WHITELISTED',
+        'IDR_STRUCTURE_IN_TRUE_IF_WHITELISTED',
+        'IDR_INCLUDE_WHITELISTED',
+    ]
+    non_whitelisted_ids = [
+        'IDR_STRUCTURE_NOT_WHITELISTED',
+        'IDR_STRUCTURE_IN_TRUE_IF_NOT_WHITELISTED',
+        'IDR_STRUCTURE_IN_FALSE_IF_WHITELISTED',
+        'IDR_STRUCTURE_IN_FALSE_IF_NOT_WHITELISTED',
+        'IDR_INCLUDE_NOT_WHITELISTED',
+    ]
+    for output_file in (header, map_cc):
+      self._verifyWhitelistedOutput(
+        output_file,
+        whitelisted_ids,
+        non_whitelisted_ids,
+      )
+
+
 if __name__ == '__main__':
   unittest.main()