Grit: Compress all HTML, JS, CSS, SVG files by default.

Change Grit's default behavior to gzip compress all such
files, unless compress="false" is explicitly specified.

In other words, compress="gzip" is now the default behavior, and
same as not specifying a |compress| attribute.

out/<out_folder>/resources.pak sizes before/after
(with optimize_webui=true)

Linux: 7.1MB ->  5.9MB,  -4.9%
CrOS: 21.0MB -> 18.0MB, -14.3%

Bug: 1068407
Change-Id: I3fa63a4db6087b2f3502cbb8b76a6ca3b576e7fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1612333
Commit-Queue: dpapad <dpapad@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761031}

tools/grit/grit/node/base.py

@@ -31,6 +31,9 @@ class Node(object):
   _CONTENT_TYPE_CDATA = 1  # Only CDATA, no children.
   _CONTENT_TYPE_MIXED = 2  # CDATA and children, possibly intermingled
 
+  # Types of files to be compressed by default.
+  _COMPRESS_BY_DEFAULT_EXTENSIONS = ('.js', '.html', '.css', '.svg')
+
   # Default nodes to not whitelist skipped
   _whitelist_marked_as_skip = False
 
@@ -614,7 +617,16 @@ class Node(object):
       The data in gzipped or brotli compressed format. If the format is
       unspecified then this returns the data uncompressed.
     '''
-    if self.attrs.get('compress') == 'gzip':
+
+    compress = self.attrs.get('compress')
+
+    # Compress JS, HTML, CSS and SVG files by default (gzip), unless |compress|
+    # is explicitly specified.
+    compress_by_default = (compress == 'default'
+                           and self.attrs.get('file').endswith(
+                               self._COMPRESS_BY_DEFAULT_EXTENSIONS))
+
+    if compress == 'gzip' or compress_by_default:
       # We only use rsyncable compression on Linux.
       # We exclude ChromeOS since ChromeOS bots are Linux based but do not have
       # the --rsyncable option built in for gzip. See crbug.com/617950.
@@ -622,7 +634,7 @@ class Node(object):
         return grit.format.gzip_string.GzipStringRsyncable(data)
       return grit.format.gzip_string.GzipString(data)
 
-    elif self.attrs.get('compress') == 'brotli':
+    if compress == 'brotli':
       # The length of the uncompressed data as 8 bytes little-endian.
       size_bytes = struct.pack("<q", len(data))
       data = brotli_util.BrotliCompress(data)
@@ -636,11 +648,10 @@ class Node(object):
              b''.join(struct.unpack(formatter, size_bytes)) +
              data)
 
-    elif self.attrs.get('compress') == 'false':
+    if compress == 'false' or compress == 'default':
       return data
 
-    else:
-      raise Exception('Invalid value for compression')
+    raise Exception('Invalid value for compression')
 
 
 class ContentNode(Node):

tools/grit/grit/node/include.py

@@ -56,19 +56,20 @@ class IncludeNode(base.Node):
        skip_minify:           If true, skips minifying the node's contents.
        skip_in_resource_map:  If true, do not add to the resource map.
     """
-    return {'translateable' : 'true',
-            'generateid': 'true',
-            'filenameonly': 'false',
-            'mkoutput': 'false',
-            'preprocess': 'false',
-            'flattenhtml': 'false',
-            'compress': 'false',
-            'allowexternalscript': 'false',
-            'relativepath': 'false',
-            'use_base_dir': 'true',
-            'skip_minify': 'false',
-            'skip_in_resource_map': 'false',
-           }
+    return {
+        'translateable': 'true',
+        'generateid': 'true',
+        'filenameonly': 'false',
+        'mkoutput': 'false',
+        'preprocess': 'false',
+        'flattenhtml': 'false',
+        'compress': 'default',
+        'allowexternalscript': 'false',
+        'relativepath': 'false',
+        'use_base_dir': 'true',
+        'skip_minify': 'false',
+        'skip_in_resource_map': 'false',
+    }
 
   def GetInputPath(self):
     # Do not mess with absolute paths, that would make them invalid.

tools/grit/grit/node/include_unittest.py

@@ -21,6 +21,22 @@ from grit.node import empty
 from grit import util
 
 
+def checkIsGzipped(filename, compress_attr):
+  test_data_root = util.PathFromRoot('grit/testdata')
+  root = util.ParseGrdForUnittest(
+      '''
+      <includes>
+        <include name="TEST_TXT" file="%s" %s type="BINDATA"/>
+      </includes>''' % (filename, compress_attr),
+      base_dir=test_data_root)
+  node, = root.GetChildrenOfType(include.IncludeNode)
+  compressed = node.GetDataPackValue(lang='en', encoding=util.BINARY)
+
+  decompressed_data = zlib.decompress(compressed, 16 + zlib.MAX_WBITS)
+  expected = util.ReadFile(os.path.join(test_data_root, filename), util.BINARY)
+  return expected == decompressed_data
+
+
 class IncludeNodeUnittest(unittest.TestCase):
   def testGetPath(self):
     root = misc.GritNode()
@@ -72,15 +88,18 @@ class IncludeNodeUnittest(unittest.TestCase):
                      expected_path)
 
   def testCompressGzip(self):
-    root = util.ParseGrdForUnittest('''
-        <includes>
-          <include name="TEST_TXT" file="test_text.txt"
-                   compress="gzip" type="BINDATA"/>
-        </includes>''', base_dir = util.PathFromRoot('grit/testdata'))
-    inc, = root.GetChildrenOfType(include.IncludeNode)
-    compressed = inc.GetDataPackValue(lang='en', encoding=util.BINARY)
+    self.assertTrue(checkIsGzipped('test_text.txt', 'compress="gzip"'))
+
+  def testCompressGzipByDefault(self):
+    self.assertTrue(checkIsGzipped('test_html.html', ''))
+    self.assertTrue(checkIsGzipped('test_js.js', ''))
+    self.assertTrue(checkIsGzipped('test_css.css', ''))
+    self.assertTrue(checkIsGzipped('test_svg.svg', ''))
 
-    decompressed_data = zlib.decompress(compressed, 16 + zlib.MAX_WBITS)
+    self.assertTrue(checkIsGzipped('test_html.html', 'compress="default"'))
+    self.assertTrue(checkIsGzipped('test_js.js', 'compress="default"'))
+    self.assertTrue(checkIsGzipped('test_css.css', 'compress="default"'))
+    self.assertTrue(checkIsGzipped('test_svg.svg', 'compress="default"'))
 
   def testSkipInResourceMap(self):
     root = util.ParseGrdForUnittest('''
@@ -97,11 +116,13 @@ class IncludeNodeUnittest(unittest.TestCase):
     self.assertTrue(inc[2].IsResourceMapSource())
 
   def testAcceptsPreprocess(self):
-    root = util.ParseGrdForUnittest('''
+    root = util.ParseGrdForUnittest(
+        '''
         <includes>
           <include name="PREPROCESS_TEST" file="preprocess_test.html"
-                   preprocess="true" type="chrome_html"/>
-        </includes>''', base_dir = util.PathFromRoot('grit/testdata'))
+                   preprocess="true" compress="false" type="chrome_html"/>
+        </includes>''',
+        base_dir=util.PathFromRoot('grit/testdata'))
     inc, = root.GetChildrenOfType(include.IncludeNode)
     result = inc.GetDataPackValue(lang='en', encoding=util.BINARY)
     self.assertIn(b'should be kept', result)

tools/grit/grit/node/structure.py

@@ -117,32 +117,33 @@ class StructureNode(base.Node):
     return ['type', 'name', 'file']
 
   def DefaultAttributes(self):
-    return { 'encoding' : 'cp1252',
-             'exclude_from_rc' : 'false',
-             'line_end' : 'unix',
-             'output_encoding' : 'utf-8',
-             'generateid': 'true',
-             'expand_variables' : 'false',
-             'output_filename' : '',
-             'fold_whitespace': 'false',
-             # Run an arbitrary command after translation is complete
-             # so that it doesn't interfere with what's in translation
-             # console.
-             'run_command' : '',
-             # Leave empty to run on all platforms, comma-separated
-             # for one or more specific platforms. Values must match
-             # output of platform.system().
-             'run_command_on_platforms' : '',
-             'allowexternalscript': 'false',
-             # preprocess takes the same code path as flattenhtml, but it
-             # disables any processing/inlining outside of <if> and <include>.
-             'preprocess': 'false',
-             'flattenhtml': 'false',
-             'fallback_to_low_resolution': 'default',
-             'variables': '',
-             'compress': 'false',
-             'use_base_dir': 'true',
-             }
+    return {
+        'encoding': 'cp1252',
+        'exclude_from_rc': 'false',
+        'line_end': 'unix',
+        'output_encoding': 'utf-8',
+        'generateid': 'true',
+        'expand_variables': 'false',
+        'output_filename': '',
+        'fold_whitespace': 'false',
+        # Run an arbitrary command after translation is complete
+        # so that it doesn't interfere with what's in translation
+        # console.
+        'run_command': '',
+        # Leave empty to run on all platforms, comma-separated
+        # for one or more specific platforms. Values must match
+        # output of platform.system().
+        'run_command_on_platforms': '',
+        'allowexternalscript': 'false',
+        # preprocess takes the same code path as flattenhtml, but it
+        # disables any processing/inlining outside of <if> and <include>.
+        'preprocess': 'false',
+        'flattenhtml': 'false',
+        'fallback_to_low_resolution': 'default',
+        'variables': '',
+        'compress': 'default',
+        'use_base_dir': 'true',
+    }
 
   def IsExcludedFromRc(self):
     return self.attrs['exclude_from_rc'] == 'true'

tools/grit/grit/node/structure_unittest.py

@@ -27,6 +27,23 @@ from grit.node import structure
 from grit.format import rc
 
 
+def checkIsGzipped(filename, compress_attr):
+  test_data_root = util.PathFromRoot('grit/testdata')
+  root = util.ParseGrdForUnittest(
+      '''
+      <structures>
+        <structure name="TEST_TXT" file="%s" %s type="chrome_html"/>
+      </structures>''' % (filename, compress_attr),
+      base_dir=test_data_root)
+  node, = root.GetChildrenOfType(structure.StructureNode)
+  node.RunPreSubstitutionGatherer()
+  compressed = node.GetDataPackValue(lang='en', encoding=util.BINARY)
+
+  decompressed_data = zlib.decompress(compressed, 16 + zlib.MAX_WBITS)
+  expected = util.ReadFile(os.path.join(test_data_root, filename), util.BINARY)
+  return expected == decompressed_data
+
+
 class StructureUnittest(unittest.TestCase):
   def testSkeleton(self):
     grd = util.ParseGrdForUnittest('''
@@ -100,20 +117,18 @@ class StructureUnittest(unittest.TestCase):
                          base_dir, r'structure_variables.html')))
 
   def testCompressGzip(self):
-    test_data_root = util.PathFromRoot('grit/testdata')
-    root = util.ParseGrdForUnittest('''
-        <structures>
-          <structure name="TEST_TXT" file="test_text.txt"
-                   compress="gzip" type="chrome_html" />
-        </structures>''', base_dir=test_data_root)
-    node, = root.GetChildrenOfType(structure.StructureNode)
-    node.RunPreSubstitutionGatherer()
-    compressed = node.GetDataPackValue(lang='en', encoding=util.BINARY)
-
-    decompressed_data = zlib.decompress(compressed, 16 + zlib.MAX_WBITS)
-    self.assertEqual(util.ReadFile(
-        os.path.join(test_data_root, 'test_text.txt'), util.BINARY),
-                     decompressed_data)
+    self.assertTrue(checkIsGzipped('test_text.txt', 'compress="gzip"'))
+
+  def testCompressGzipByDefault(self):
+    self.assertTrue(checkIsGzipped('test_html.html', ''))
+    self.assertTrue(checkIsGzipped('test_js.js', ''))
+    self.assertTrue(checkIsGzipped('test_css.css', ''))
+    self.assertTrue(checkIsGzipped('test_svg.svg', ''))
+
+    self.assertTrue(checkIsGzipped('test_html.html', 'compress="default"'))
+    self.assertTrue(checkIsGzipped('test_js.js', 'compress="default"'))
+    self.assertTrue(checkIsGzipped('test_css.css', 'compress="default"'))
+    self.assertTrue(checkIsGzipped('test_svg.svg', 'compress="default"'))
 
   def testCompressBrotli(self):
     test_data_root = util.PathFromRoot('grit/testdata')

tools/grit/grit/testdata/test_css.css

+This is a test!

tools/grit/grit/testdata/test_html.html

+This is a test!

tools/grit/grit/testdata/test_js.js

+This is a test!

tools/grit/grit/testdata/test_svg.svg

+This is a test!