Include a pak file in the binary_size output if requested.

Include a pak file in the binary_size output if requested.

By adding --pak out/Release/content_shell.pak at the command line
the contents of it will be added to the visual output.

BUG=370383
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284440

[Daniel Bratell, 2014-07-16 07:49:27]

Could you take a look please?

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

LGTM overall.

Philosophically, the direction I'd like to take the tool is to allow it to
process an entire installation package - whatever that might be. APK, EXE,
tarball, whatever. This is a good step in that direction.

In the end I'd like to see this code moved out to its own python module, as I
see this as the tip of a larger iceberg where we process additional resources
that are part of the install package: nested archives and ARSC files come to
mind on Android, and I'm sure there's more stuff in the DMG on mac and the EXE
on Windows. If you want to do that now, great. If not, please either file a
crbug to get it done later.

Thanks for your continued contributions!

https://codereview.chromium.org/380693002/diff/1/tools/binary_size/run_binary...
File tools/binary_size/run_binary_size_analysis.py (right):

https://codereview.chromium.org/380693002/diff/1/tools/binary_size/run_binary...
tools/binary_size/run_binary_size_analysis.py:641: try:
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.

https://codereview.chromium.org/380693002/diff/1/tools/binary_size/run_binary...
tools/binary_size/run_binary_size_analysis.py:648: 'Pak Resouce %d' %
resource_id)
Typo: "Resouce"

https://codereview.chromium.org/380693002/diff/1/tools/binary_size/run_binary...
tools/binary_size/run_binary_size_analysis.py:656: PACK_FILE_VERSION = 4
For consistency could we stick with PAK instead of PACK?

https://codereview.chromium.org/380693002/diff/1/tools/binary_size/run_binary...
tools/binary_size/run_binary_size_analysis.py:676: symbol_type = 'd' # Data.
Approximation.
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.

[Daniel Bratell, 2014-07-21 12:29:26]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2014-07-21 12:29:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bratell@opera.com/380693002/20001

[Daniel Bratell, 2014-07-21 12:31:00]

On 2014/07/16 09:29:39, andrewhayden wrote:
> LGTM overall.
> 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.

Filed bug 395583 and addressed the other issues.

I think that it would be worthwhile letting run_binary_size.py figure out the
types of files given it as well.

Right now you can feed it an nm file, an ELF binary or a PAK file and I think it
could detect which is which pretty easily.

[commit-bot: I haz the power, 2014-07-21 12:38:38]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel_swarming on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/30987)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/35390)

[commit-bot: I haz the power, 2014-07-21 12:39:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-21 12:39:50]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/30990)

[Daniel Bratell, 2014-07-21 14:35:43]

The CQ bit was checked by bratell@opera.com

[commit-bot: I haz the power, 2014-07-21 14:37:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bratell@opera.com/380693002/40001

[commit-bot: I haz the power, 2014-07-21 16:18:05]

Change committed as 284440