Vulcanize MD History to improve page-load performance

chrome/browser/resources/vulcanize.py

 #!/usr/bin/env python
-# Copyright 2015 The Chromium Authors. All rights reserved.
+# Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import os
 import subprocess
 import sys
 import tempfile
 
+# See //docs/vulcanize.md for instructions on installing prerequistes and
+# running the vulcanize build.
 
 _HERE_PATH = os.path.join(os.path.dirname(__file__))
-
-_HTML_IN_PATH = os.path.join(_HERE_PATH, 'downloads.html')
-_HTML_OUT_PATH = os.path.join(_HERE_PATH, 'vulcanized.html')
-_JS_OUT_PATH = os.path.join(_HERE_PATH, 'crisper.js')
-
-_SRC_PATH = os.path.normpath(os.path.join(_HERE_PATH, '..', '..', '..', '..'))
+_SRC_PATH = os.path.normpath(os.path.join(_HERE_PATH, '..', '..', '..'))
 
 _RESOURCES_PATH = os.path.join(_SRC_PATH, 'ui', 'webui', 'resources')
 
 _CR_ELEMENTS_PATH = os.path.join(_RESOURCES_PATH, 'cr_elements')
 _CSS_RESOURCES_PATH = os.path.join(_RESOURCES_PATH, 'css')
 _HTML_RESOURCES_PATH = os.path.join(_RESOURCES_PATH, 'html')
 _JS_RESOURCES_PATH = os.path.join(_RESOURCES_PATH, 'js')
-
 _POLYMER_PATH = os.path.join(
     _SRC_PATH, 'third_party', 'polymer', 'v1_0', 'components-chromium')
 
-_VULCANIZE_ARGS = [
+_VULCANIZE_BASE_ARGS = [
   '--exclude', 'crisper.js',
 
   # These files are already combined and minified.
   '--exclude', 'chrome://resources/html/polymer.html',
   '--exclude', 'web-animations-next-lite.min.js',
 
   # These files are dynamically created by C++.
   '--exclude', 'load_time_data.js',
   '--exclude', 'strings.js',
   '--exclude', 'text_defaults.css',
   '--exclude', 'text_defaults_md.css',
 
   '--inline-css',
   '--inline-scripts',
 
-  '--redirect', 'chrome://downloads/|%s' % _HERE_PATH,
   '--redirect', 'chrome://resources/cr_elements/|%s' % _CR_ELEMENTS_PATH,
   '--redirect', 'chrome://resources/css/|%s' % _CSS_RESOURCES_PATH,
   '--redirect', 'chrome://resources/html/|%s' % _HTML_RESOURCES_PATH,
   '--redirect', 'chrome://resources/js/|%s' % _JS_RESOURCES_PATH,
   '--redirect', 'chrome://resources/polymer/v1_0/|%s' % _POLYMER_PATH,
 
   '--strip-comments',
 ]
 
-def main():
+
+def vulcanize(directory, host, html_in_file, html_out_file, js_out_file,

[Dan Beam, 2016/08/10 18:17:41]

nit: this is not public-ish to those that import this .py file.  do you want
that?  I don't.

[Dan Beam, 2016/08/10 18:17:41]

can you name some or all of these params?

[tsergeant, 2016/08/11 01:28:26]

I'm not sure what you mean? They are named?

[Dan Beam, 2016/08/11 02:29:49]

sorry, keyword=arguments
https://docs.python.org/3/tutorial/controlflow.html#keyword-arguments

[tsergeant, 2016/08/11 03:28:00]

Right, got it. I've changed the output files to keyword arguments.

+              extra_args = []):

[Dan Beam, 2016/08/10 18:17:41]

python style guide asks you to use something like None type for this, i.e.

  extra_args=None

(as well as drop the spaces next to =)

and if you want to homogenize the type later, use

  extra_args = extra_args or []

[tsergeant, 2016/08/11 01:28:26]

Done.

   def _run_cmd(cmd_parts, stdout=None):
     cmd = "'" + "' '".join(cmd_parts) + "'"
     process = subprocess.Popen(
         cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
     stdout, stderr = process.communicate()
 
     if stderr:
       print >> sys.stderr, '%s failed: %s' % (cmd, stderr)
       raise
 
     return stdout
 
-  output = _run_cmd(['vulcanize'] + _VULCANIZE_ARGS + [_HTML_IN_PATH])
+  print 'Vulcanizing %s' % directory
+
+  target_path = os.path.join(_HERE_PATH, directory)
+  html_in_path = os.path.join(target_path, html_in_file)
+  html_out_path = os.path.join(target_path, html_out_file)
+  js_out_path = os.path.join(target_path, js_out_file)
+
+  output = _run_cmd(['vulcanize'] + _VULCANIZE_BASE_ARGS + extra_args +
+                    ['--redirect', 'chrome://%s/|%s' % (host, target_path),
+                     html_in_path])
 
   with tempfile.NamedTemporaryFile(mode='wt+', delete=False) as tmp:
     tmp.write(output.replace(
         '<include src="', '<include src="../../../../ui/webui/resources/js/'))

[Dan Beam, 2016/08/10 18:17:41]

sooooooo are we continuing this hack?

../../../../ is specific to chrome/browser/resources/md_downloads (it just
happens to match the depth of chrome/browser/resources/md_history as well)

it's also probably silly to just assume all <include> want to go to
ui/webui/resources/js/

[tsergeant, 2016/08/11 01:28:26]

Hmmmmmmmmm.

At the moment, the only place this is used is util.js including assert.js. We
can replace that with an import in util.html, which results in the same
vulcanize output for downloads.

However, we still need to deal with the <include> tag in util.js. I've changed
this line to rewrite the tag so grit doesn't pick it up. I'm not sure if there's
any particularly nice way to do it.

[tsergeant, 2016/08/11 03:28:00]

After discussing this: I'm leaving it this way for simplicity's sake, and the
small amount of debugging information it gives.

[michaelpg, 2016/08/11 07:42:14]

I think the right way to do this is to run grit *before* vulcanizing. This is
basically required for Settings because of all the <if> grit tags, but it should
take care of <include> as well before it ever gets to vulcanize.

Here's the hacky script I use locally:
https://codereview.chromium.org/1870853003/diff/20001/chrome/browser/resource...

[tsergeant, 2016/08/15 05:54:18]

Grit output is platform specific (due to <if> tags), which is not well suited to
what we're doing with History and Downloads (we don't want to be generating a
HTML/JS file per platform and storing them in the repo).

 
   try:
     _run_cmd(['crisper', '--source', tmp.name,
                          '--script-in-head', 'false',
-                         '--html', _HTML_OUT_PATH,
-                         '--js', _JS_OUT_PATH])
+                         '--html', html_out_path,
+                         '--js', js_out_path])
+
+    # TODO(tsergeant): Remove when JS resources are minified by default:
+    # crbug.com/619091.
+    _run_cmd(['uglifyjs', js_out_path,

[tsergeant, 2016/08/10 06:40:34]

Uglify-with-beautify is...a little strange. Stripping comments saves us at least
200K, so we definitely want to do it, and uglify is the best option I've found
for stripping all comments without destroying the structure of the code. It does
still end up in effectively rewriting big chunks of md_downloads/crisper.js,
though.

We could use //third_party/closure_compiler/js_minify.py, but that strips out
almost all whitespace unconditionally, leaving us with a big blob which is
terrible for git.

[Dan Beam, 2016/08/10 18:17:41]

I don't really care what we use, but either way we should sync up with aberent.

+                         '--beautify', 'indent-level=2,quote_style=3',
+                         '--comments', '/Copyright|license|LICENSE|\<\/?if/',
+                         '--output', js_out_path])
   finally:
     os.remove(tmp.name)
 
 
+def main():
+  vulcanize('md_downloads', 'downloads', 'downloads.html', 'vulcanized.html',
+            'crisper.js')
+
+  # Already loaded by history.html:
+  history_extra_args = ['--exclude', 'chrome://resources/html/util.html',
+                        '--exclude', 'chrome://history/constants.html']
+  vulcanize('md_history', 'history', 'app.html', 'app.vulcanized.html',
+            'app.crisper.js', history_extra_args)
+

[Dan Beam, 2016/08/10 18:17:41]

\n\n between top-levels

[tsergeant, 2016/08/11 01:28:26]

Done.

 if __name__ == '__main__':
   main()