WebUI: Replace vulcanize with polymer-bundler

chrome/browser/resources/vulcanize_gn.py

 #!/usr/bin/env python
 # 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 argparse
 import itertools
 import os
 import platform
 import re
 import sys
 import tempfile
+import json

[michaelpg, 2017/07/19 18:51:59]

nit: alphabetical

[dpapad, 2017/07/19 20:49:38]

Done.

+import shutil
 
 
 _HERE_PATH = os.path.dirname(__file__)
 _SRC_PATH = os.path.normpath(os.path.join(_HERE_PATH, '..', '..', '..'))
 _CWD = os.getcwd()  # NOTE(dbeam): this is typically out/<gn_name>/.
 
 sys.path.append(os.path.join(_SRC_PATH, 'third_party', 'node'))
 import node
 import node_modules
 
@@ skipping 13 matching lines @@
 _JS_RESOURCES_PATH = os.path.join(_RESOURCES_PATH, 'js')
 
 
 _POLYMER_PATH = os.path.join(
     _SRC_PATH, 'third_party', 'polymer', 'v1_0', 'components-chromium')
 
 
 _VULCANIZE_BASE_ARGS = [
   # These files are already combined and minified.
   '--exclude', 'chrome://resources/html/polymer.html',
-  '--exclude', 'web-animations-next-lite.min.js',
+  '--exclude', 'chrome://resources/polymer/v1_0/polymer/polymer.html',
+  '--exclude', 'chrome://resources/polymer/v1_0/polymer/polymer-micro.html',
+  '--exclude', 'chrome://resources/polymer/v1_0/polymer/polymer-mini.html',
+  '--exclude', 'chrome://resources/polymer/v1_0/web-animations-js/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',
+  '--exclude', 'chrome://resources/css/roboto.css',
+  '--exclude', 'chrome://resources/css/text_defaults.css',
+  '--exclude', 'chrome://resources/css/text_defaults_md.css',
+  '--exclude', 'chrome://resources/js/load_time_data.js',
 
   '--inline-css',
   '--inline-scripts',
+  '--rewrite-urls-in-templates',
   '--strip-comments',
 ]
 
 
 _URL_MAPPINGS = [
     ('chrome://resources/cr_elements/', _CR_ELEMENTS_PATH),
     ('chrome://resources/css/', _CSS_RESOURCES_PATH),
     ('chrome://resources/html/', _HTML_RESOURCES_PATH),
     ('chrome://resources/js/', _JS_RESOURCES_PATH),
     ('chrome://resources/polymer/v1_0/', _POLYMER_PATH)
 ]
 
 
 _VULCANIZE_REDIRECT_ARGS = list(itertools.chain.from_iterable(map(
     lambda m: ['--redirect', '"%s|%s"' % (m[0], m[1])], _URL_MAPPINGS)))
 
 
 def _undo_mapping(mappings, url):
   for (redirect_url, file_path) in mappings:
     if url.startswith(redirect_url):
       return url.replace(redirect_url, file_path + os.sep)
   # TODO(dbeam): can we make this stricter?
   return url
 
-def _request_list_path(out_path, html_out_file):
-  return os.path.join(out_path, html_out_file + '_requestlist.txt')
+def _request_list_path(out_path, host):
+  return os.path.join(out_path, host + '_requestlist.txt')
 
-# Get a list of all files that were bundled with Vulcanize and update the
-# depfile accordingly such that Ninja knows when to trigger re-vulcanization.
-def _update_dep_file(in_folder, args):
+# Get a list of all files that were bundled with polymer-bundler and update the
+# depfile accordingly such that Ninja knows when to re-trigger.
+def _update_dep_file(in_folder, args, manifest):
   in_path = os.path.join(_CWD, in_folder)
-  out_path = os.path.join(_CWD, args.out_folder)
 
-  # Prior call to vulcanize already generated the deps list, grab it from there.
-  request_list_path = _request_list_path(out_path, args.html_out_file)
-  request_list = open(request_list_path, 'r').read().splitlines()
+  # Gather the dependencies of all bundled root HTML files.
+  request_list = []
+  for html_file in manifest:
+    request_list += manifest[html_file]
 
-  if platform.system() == 'Windows':
-    # TODO(dbeam): UGH. For some reason Vulcanize is interpreting the target
-    # file path as a URL and using the drive letter (e.g. D:\) as a protocol.
-    # This is a little insane, but we're fixing here by normalizing case (which
-    # really shouldn't matter, these are all file paths and generally are all
-    # lower case) and writing from / to \ (file path) and then back again. This
-    # is compounded by NodeJS having a bug in url.resolve() that handles
-    # chrome:// protocol URLs poorly as well as us using startswith() to strip
-    # file paths (which isn't crazy awesome either). Don't remove unless you
-    # really really know what you're doing.
-    norm = lambda u: u.lower().replace('/', '\\')
-    request_list = [norm(u).replace(norm(in_path), '').replace('\\', '/')
-        for u in request_list]
+  # Add a slash in front of every dependency that is not a chrome:// URL, so
+  # that we can map it to the correct source file path below.
+  request_list = map(
+      lambda dep: '/' + dep if not dep.startswith('chrome://') else dep,
+      request_list)
 
   # Undo the URL mappings applied by vulcanize to get file paths relative to
   # current working directory.
   url_mappings = _URL_MAPPINGS + [
       ('/', os.path.relpath(in_path, _CWD)),
       ('chrome://%s/' % args.host, os.path.relpath(in_path, _CWD)),
   ]
 
   deps = [_undo_mapping(url_mappings, u) for u in request_list]
   deps = map(os.path.normpath, deps)
 
   # If the input was a folder holding an unpacked .pak file, the generated
   # depfile should not list files already in the .pak file.
   if args.input.endswith('.unpak'):
     filter_url = args.input
     deps = [d for d in deps if not d.startswith(filter_url)]
 
   with open(os.path.join(_CWD, args.depfile), 'w') as f:
-    deps_file_header = os.path.join(args.out_folder, args.html_out_file)
+    deps_file_header = os.path.join(args.out_folder, args.html_out_files[0])
     f.write(deps_file_header + ': ' + ' '.join(deps))
 
 
 def _vulcanize(in_folder, args):
   in_path = os.path.normpath(os.path.join(_CWD, in_folder))
   out_path = os.path.join(_CWD, args.out_folder)
-
-  html_out_path = os.path.join(out_path, args.html_out_file)
-  js_out_path = os.path.join(out_path, args.js_out_file)
+  manifest_out_path = _request_list_path(out_path, args.host)
 
   exclude_args = []
   for f in args.exclude or []:
     exclude_args.append('--exclude')
     exclude_args.append(f)
 
-  output = node.RunNode(
-      [node_modules.PathToVulcanize()] +
+  in_html_args = []
+  for f in args.html_in_files:
+    in_html_args.append('--in-html')
+    in_html_args.append(f)
+
+  tmp_out_dir = os.path.join(out_path, 'bundled')
+  node.RunNode(
+      [node_modules.PathToBundler()] +
       _VULCANIZE_BASE_ARGS + _VULCANIZE_REDIRECT_ARGS + exclude_args +
-      ['--out-request-list', _request_list_path(out_path, args.html_out_file),
-       '--redirect', '"/|%s"' % in_path,
+      [# This file is dynamically created by C++. Need to specify an exlusion

[calamity, 2017/07/19 06:15:33]

nit: exclusion

[dpapad, 2017/07/19 20:49:38]

Done.

+       # URL for both the relative URL and chrome:// URL syntax.
+       '--exclude', 'strings.js',
+       '--exclude', 'chrome://%s/strings.js' % args.host,
+
+       '--manifest-out', manifest_out_path,
+       '--root', in_path,
        '--redirect', '"chrome://%s/|%s"' % (args.host, in_path),
-       # TODO(dpapad): Figure out why vulcanize treats the input path
-       # differently on Windows VS Linux/Mac.
-       os.path.join(
-           in_path if platform.system() == 'Windows' else os.sep,
-           args.html_in_file)])
+       '--out-dir', os.path.relpath(tmp_out_dir, _CWD),
+       '--shell', args.html_in_files[0],
+      ] + in_html_args)
 
-  # Grit includes are not supported, use HTML imports instead.
-  output = output.replace('<include src="', '<include src-disabled="')
+  for index, html_file in enumerate(args.html_in_files):
+    with open(
+        os.path.join(os.path.relpath(tmp_out_dir, _CWD), html_file), 'r') as f:
+      output = f.read()
 
-  if args.insert_in_head:
-    assert '<head>' in output
-    # NOTE(dbeam): Vulcanize eats <base> tags after processing. This undoes
-    # that by adding a <base> tag to the (post-processed) generated output.
-    output = output.replace('<head>', '<head>' + args.insert_in_head)
+      # Grit includes are not supported, use HTML imports instead.
+      output = output.replace('<include src="', '<include src-disabled="')
 
-  crisper_input = tempfile.NamedTemporaryFile(mode='wt+', delete=False)
-  crisper_input.write(output)
-  crisper_input.close()
+      if args.insert_in_head:
+        assert '<head>' in output
+        # NOTE(dbeam): polymer-bundler eats <base> tags after processing. This
+        # undoes that by adding a <base> tag to the (post-processed) generated
+        # output.
+        output = output.replace('<head>', '<head>' + args.insert_in_head)
 
-  crisper_output = tempfile.NamedTemporaryFile(mode='wt+', delete=False)
-  crisper_output.close()
+    # Open file again with 'w' such that the previous contents are overwritten.
+    with open(
+        os.path.join(os.path.relpath(tmp_out_dir, _CWD), html_file), 'w') as f:
+      f.write(output)
+      f.close()
 
   try:
-    node.RunNode([node_modules.PathToCrisper(),
-                 '--source', crisper_input.name,
-                 '--script-in-head', 'false',
-                 '--only-split',
-                 '--html', html_out_path,
-                 '--js', crisper_output.name])
+    for index, html_in_file in enumerate(args.html_in_files):
+      html_out_file = args.html_out_files[index]
+      js_out_file = args.js_out_files[index]
 
-    # Crisper by default inserts a <script> tag with the name of the --js file,
-    # but since we are using a temporary file, need to manually insert a
-    # <script> tag with the correct final filename (in combination with
-    # --only-split flag). There is no way currently to manually specify the
-    # <script> tag's path, see https://github.com/PolymerLabs/crisper/issues/46.
-    with open(html_out_path, 'r+') as f:
-      data = f.read()
-      new_data = data.replace(
-          '</body></html>',
-          '<script src="' + args.js_out_file + '"></script></body></html>')
-      assert new_data != data, 'Expected to find </body></html> token.'
-      f.seek(0)
-      f.write(new_data)
-      f.truncate()
+      # Run crisper to separate the JS from the HTML file.
+      node.RunNode([node_modules.PathToCrisper(),
+                   '--source', os.path.join(tmp_out_dir, html_in_file),
+                   '--script-in-head', 'false',
+                   '--html', os.path.join(tmp_out_dir, html_out_file),
+                   '--js', os.path.join(tmp_out_dir, js_out_file)])

[calamity, 2017/07/19 06:15:33]

If we move the polymer_css_build step into this script, we'll end up with the
same file name for html_in_file and html_out_file, right? It might be good add a
prefix or suffix to each of the files that explains the processes that have been
done on each of them. This will help prevent name collisions and also makes the
temporary files easier to debug.

[dpapad, 2017/07/19 20:49:38]

Acknowledged. As said in previous comment, prefer to merge polymer_css_build
into this script in a follow up CL, so will address this in that CL.

 
-    node.RunNode([node_modules.PathToUglify(), crisper_output.name,
-                  '--comments', '"/Copyright|license|LICENSE|\<\/?if/"',
-                  '--output', js_out_path])
+      # Move the HTML file to its final destination.
+      shutil.copy(os.path.join(tmp_out_dir, html_out_file), out_path)
+
+      # Pass the JS file through Uglify and write the output to its final
+      # destination.
+      node.RunNode([node_modules.PathToUglify(),
+                    os.path.join(tmp_out_dir, js_out_file),
+                    '--comments', '"/Copyright|license|LICENSE|\<\/?if/"',
+                    '--output', os.path.join(out_path, js_out_file)])
   finally:
-    if os.path.exists(crisper_input.name):
-      os.remove(crisper_input.name)
-    if os.path.exists(crisper_output.name):
-      os.remove(crisper_output.name)
+    shutil.rmtree(tmp_out_dir)

[calamity, 2017/07/19 06:15:33]

If any of the commands here fail, will the build still succeed?

[michaelpg, 2017/07/19 18:51:59]

Is it worth leaving the output files around (instead of using "finally") for
debugging purposes?

[dpapad, 2017/07/19 20:49:38]

The build still fails if any command fails.

If you run a build again after the 1st failure, Ninja will decide whether to
rebuild based on the timestamps of the declared output files, so
 
 - if the build failed before one of those files was produced (say before line
194), it will rebuild,
 - if there is an error after all output files have been produced it will do
nothing.

I believe this is the same behavior as before.
I prefer not to do this, because it increases the chances that a subsequent
build will use stale files. I've had this happened in the past where a local
build was succeeding, because stale files were present under out/../gen/..., but
the build was failing on the bots, and it was fairly confusing.

One can always comment out this line locally as needed, which is what I did
while developing this CL.

[michaelpg, 2017/07/21 00:57:11]

d'oh, yeah I saw "tmp" and thought it was a unique `mktemp -d` style temporary
directory.

+  return manifest_out_path
 
 
 def main(argv):
   parser = argparse.ArgumentParser()
   parser.add_argument('--depfile', required=True)
   parser.add_argument('--exclude', nargs='*')
   parser.add_argument('--host', required=True)
-  parser.add_argument('--html_in_file', required=True)
-  parser.add_argument('--html_out_file', required=True)
+  parser.add_argument('--html_in_files', nargs='*', required=True)
+  parser.add_argument('--html_out_files', nargs='*', required=True)
   parser.add_argument('--input', required=True)
   parser.add_argument('--insert_in_head')
-  parser.add_argument('--js_out_file', required=True)
+  parser.add_argument('--js_out_files', nargs='*', required=True)
   parser.add_argument('--out_folder', required=True)
   args = parser.parse_args(argv)
 
   # NOTE(dbeam): on Windows, GN can send dirs/like/this. When joined, you might
   # get dirs/like/this\file.txt. This looks odd to windows. Normalize to right
   # the slashes.
   args.depfile = os.path.normpath(args.depfile)
   args.input = os.path.normpath(args.input)
   args.out_folder = os.path.normpath(args.out_folder)
 
-  _vulcanize(args.input, args)
-  _update_dep_file(args.input, args)
+  manifest_out_path = _vulcanize(args.input, args)
+
+  # Prior call to _vulcanize() generated an output manifest file, containing
+  # information about all files that were bundled. Grab it from there.
+  manifest = json.loads(open(manifest_out_path, 'r').read())
+
+  # polymer-bundler reports any missing files in the output manifest, instead of
+  # directly failing. Ensure that no such files were encountered.
+  if '_missing' in manifest:
+    raise Exception(
+        'polymer-bundler could not find files for the following URLs:\n' +
+        '\n'.join(manifest['_missing']))
+
+  _update_dep_file(args.input, args, manifest)
 
 
 if __name__ == '__main__':
   main(sys.argv[1:])