Include a pak file in the binary_size output if requested.

tools/binary_size/run_binary_size_analysis.py

Index: tools/binary_size/run_binary_size_analysis.py
diff --git a/tools/binary_size/run_binary_size_analysis.py b/tools/binary_size/run_binary_size_analysis.py
index f06884f8dafe1f7ba54c8586d18882c0e0ec5227..94f6c948f70fbb8df71c8ee828b6de9825d3564f 100755
--- a/tools/binary_size/run_binary_size_analysis.py
+++ b/tools/binary_size/run_binary_size_analysis.py
@@ -18,6 +18,7 @@ import optparse
 import os
 import re
 import shutil
+import struct
 import subprocess
 import sys
 import tempfile
@@ -616,6 +617,66 @@ def GetNmSymbols(nm_infile, outfile, library, jobs, verbose,
     return list(binary_size_utils.ParseNm(infile))
 
 
+PAK_RESOURCE_ID_TO_STRING = { "inited": False }
+def GetReadablePakResourceName(pak_file, resource_id):
+  """Pak resources have a numeric identifier. It is not helpful when
+  trying to locate where footprint is generated. This does its best to
+  map the number to a usable string."""
+  if not PAK_RESOURCE_ID_TO_STRING['inited']:
+    # Try to find resource header files generated by grit when
+    # building the pak file. We'll look for files named *resources.h"
+    # and lines of the type:
+    #    #define MY_RESOURCE_JS 1234
+    PAK_RESOURCE_ID_TO_STRING['inited'] = True
+    gen_dir = os.path.join(os.path.dirname(pak_file), 'gen')
+    if os.path.isdir(gen_dir):
+      for dirname, _dirs, files in os.walk(gen_dir):
+        for filename in files:
+          if filename.endswith('resources.h'):
+            with open(os.path.join(dirname, filename)) as resource_header:
+              for line in resource_header:
+                if line.startswith("#define "):
+                  line_data = line.split()
+                  if len(line_data) == 3:
+                    try:

[andrewhayden, 2014/07/16 09:29:39]

I'm a bit leery of any code that reaches the 10th level of indentation due to
nesting of if statements and for loops. Could we simplify the flow a little bit
by "continue"ing when the condition is NOT met? E.g.:

if not os.path.isdir(gen_dir) return // whatever it should be
for dirname, _dirs, files in os.walk(gen_dir):
  if not filename.endswith('resources.h') continue
  with open(os.path.join(dirname, filename)) as resource_header:
  for line in resource_header:
    if not line.startswith("#define") continue
    line_data = line.split()
    if len(line_data) != 3 continue
    try:
      // the rest of the code here

Also, consider splitting the code from line 636 onwards into a function, e.g.
"processResourcesFile". This would remove a bunch of the nesting and separate
the code for finding the file from the code that parses its contents.

I know that using continue has it's own problems but I think that it's the
lesser of the evils here given how deeply nested this code is.

+                      resource_number = int(line_data[2])
+                      resource_name = line_data[1]
+                      PAK_RESOURCE_ID_TO_STRING[resource_number] = resource_name
+                    except ValueError:
+                      pass
+  return PAK_RESOURCE_ID_TO_STRING.get(resource_id,
+                                       'Pak Resouce %d' % resource_id)

[andrewhayden, 2014/07/16 09:29:38]

Typo: "Resouce"

+
+def AddPakData(symbols, pak_file):
+  """Adds pseudo-symbols from a pak file."""
+  pak_file = os.path.abspath(pak_file)
+  with open(pak_file, 'rb') as pak:
+    data = pak.read()
+
+  PACK_FILE_VERSION = 4

[andrewhayden, 2014/07/16 09:29:38]

For consistency could we stick with PAK instead of PACK?

+  HEADER_LENGTH = 2 * 4 + 1  # Two uint32s. (file version, number of entries)
+                             # and one uint8 (encoding of text resources)
+  INDEX_ENTRY_SIZE = 2 + 4  # Each entry is a uint16 and a uint32.
+  version, num_entries, _encoding = struct.unpack('<IIB', data[:HEADER_LENGTH])
+  assert version == PACK_FILE_VERSION, ('Unsupported pak file '
+                                        'version (%d) in %s. Only '
+                                        'support version %d' %
+                                        (version, pak_file, PACK_FILE_VERSION))
+  if num_entries > 0:
+    # Read the index and data.
+    data = data[HEADER_LENGTH:]
+    for _ in range(num_entries):
+      resource_id, offset = struct.unpack('<HI', data[:INDEX_ENTRY_SIZE])
+      data = data[INDEX_ENTRY_SIZE:]
+      _next_id, next_offset = struct.unpack('<HI', data[:INDEX_ENTRY_SIZE])
+      resource_size = next_offset - offset
+
+      symbol_name = GetReadablePakResourceName(pak_file, resource_id)
+      symbol_path = pak_file
+      symbol_type = 'd' # Data. Approximation.

[andrewhayden, 2014/07/16 09:29:39]

Could you file a crbug after we finish this to add a new symbol type that we can
use for these to split them out? The lists are totally customizable, and
although right now we've got the stuff from the "nm" manpage, we already have a
non-standard symbol to split out vtables. I think it would be reasonable to
split these out as well.

+      symbol_size = resource_size
+      symbols.append((symbol_name, symbol_type, symbol_size, symbol_path))
+
 def _find_in_system_path(binary):
   """Locate the full path to binary in the system path or return None
   if not found."""
@@ -688,6 +749,9 @@ def main():
   parser.add_option('--library', metavar='PATH',
                     help='if specified, process symbols in the library at '
                     'the specified path. Mutually exclusive with --nm-in.')
+  parser.add_option('--pak', metavar='PATH',
+                    help='if specified, includes the contents of the '
+                    'specified *.pak file in the output.')
   parser.add_option('--nm-binary',
                     help='use the specified nm binary to analyze library. '
                     'This is to be used when the nm in the path is not for '
@@ -749,6 +813,9 @@ def main():
     assert nm_binary, 'Unable to find nm in the path. Use --nm-binary '\
         'to specify location.'
 
+  if opts.pak:
+    assert os.path.isfile(opts.pak), 'Could not find ' % opts.pak
+
   print('addr2line: %s' % addr2line_binary)
   print('nm: %s' % nm_binary)
 
@@ -757,6 +824,10 @@ def main():
   symbols = GetNmSymbols(opts.nm_in, opts.nm_out, opts.library,
                          opts.jobs, opts.verbose is True,
                          addr2line_binary, nm_binary)
+
+  if opts.pak:
+    AddPakData(symbols, opts.pak)
+
   if not os.path.exists(opts.destdir):
     os.makedirs(opts.destdir, 0755)