Compressing .pak resources with new option: "type=GZIPPABLE_BINDATA"

tools/grit/grit/node/include.py

Index: tools/grit/grit/node/include.py
diff --git a/tools/grit/grit/node/include.py b/tools/grit/grit/node/include.py
index ebcd53108f503005dadae9ae67b192245fb0a66d..99763bdb5cfd6d3019c28cddf81a31f07e43f918 100755
--- a/tools/grit/grit/node/include.py
+++ b/tools/grit/grit/node/include.py
@@ -6,14 +6,17 @@
 """Handling of the <include> element.
 """
 
+import gzip
 import os
+import StringIO
+import subprocess
+import sys
 
+from grit import util
 import grit.format.html_inline
-import grit.format.rc_header
 import grit.format.rc
-
+import grit.format.rc_header
 from grit.node import base
-from grit import util
 
 class IncludeNode(base.Node):
   """An <include> element."""
@@ -38,6 +41,34 @@ class IncludeNode(base.Node):
               allow_external_script=allow_external_script))
     return self._flattened_data
 
+  def _GzipString(self, data):

[flackr, 2016/05/13 23:02:17]

This helper function (or functions if you follow my suggestion) should probably
be in a separate file in tools/grit/format/ alongside similar data
transformation functions like html_inline (maybe call it gzip_string.py).

[smaier, 2016/05/19 14:17:56]

Done.

+    if sys.platform == 'linux2':

[flackr, 2016/05/13 23:02:16]

nit: I'd prefer to see either two gzip functions (GzipString and
GzipStringRsyncable) or a parameter to the function, though I have a preference
for two functions since the behavior is drastically different.

Then at the call site decide which one to call with a short comment that we only
require rsyncable on linux. e.g.

# We only use rsyncable strings on linux platforms.
if sys.platform == 'linux2':
  data = GzipStringRsyncable(data)
else
  data = GzipString(data)

[smaier, 2016/05/19 14:17:56]

Done.

+      # Make call to linux's gzip to get access to --rsyncable option. This
+      # option makes updates much smaller - if one line is changed in the
+      # resource, it won't have to push the entire compressed resource with
+      # the update. Instead, --rsyncable breaks the file into small chunks,
+      # so that one doesn't affect the other in compression, and then only
+      # that chunk will have to be updated.
+      gzip_proc = subprocess.Popen(['gzip', '--stdout', '--rsyncable',
+                                    '--best', '--no-name'],
+                                   stdin=subprocess.PIPE,
+                                   stdout=subprocess.PIPE,
+                                   stderr=subprocess.PIPE)
+      data, stderr = gzip_proc.communicate(data)
+      if gzip_proc.returncode != 0:
+        raise subprocess.CalledProcessError(gzip_proc.returncode, 'gzip',
+                                            stderr)
+    else:
+      # Only expecting linux gzip calls to work: windows doesn't ship with

[flackr, 2016/05/13 23:02:17]

This comment needs updating, or to be removed.

[smaier, 2016/05/19 14:17:56]

Done.

+      # gzip, and OSX's gzip does not have an --rsyncable option built in.
+      # Therefore, fall back into the built in python function.
+      gzip_output = StringIO.StringIO()
+      with gzip.GzipFile('', 'wb', 9, gzip_output, 0) as gzip_file:
+        gzip_file.write(data)
+      data = gzip_output.getvalue()
+      gzip_output.close()
+    return data
+
   def MandatoryAttributes(self):
     return ['name', 'type', 'file']
 
@@ -90,6 +121,9 @@ class IncludeNode(base.Node):
       filename = self.ToRealPath(self.GetInputPath())
       data = util.ReadFile(filename, util.BINARY)
 
+    if self.attrs['type'] == 'GZIPPABLE_BINDATA':

[flackr, 2016/05/13 23:02:17]

I don't think this should be a new type. The type of the included file is still
BINDATA. Can we add an attribute like compress="gzip" or a boolean like
compress="true" or gzip="true" instead? Slight preference to compress since it
provides an easy path if we ever want to support a different compression.

[smaier, 2016/05/19 14:17:56]

Done.

+      data = self._GzipString(data)

[flackr, 2016/05/13 23:02:17]

This new behavior should have a test. There's some good examples of creating a
temporary directory / file structure for tests in
tools/grit/grit/gather/chrome_html_unittest.py and reading the output file in
tools/grit/grit/format/rc_unittest.py.

[smaier, 2016/05/19 14:17:56]

Let me know if the unit tests written are sufficient. 

+
     # Include does not care about the encoding, because it only returns binary
     # data.
     return id, data