Make js_binary.py a thousand times faster (literally) by caching dependency subtrees.

Targets like ui/file_manager/file_manager/foreground/js:closure_compile
were causing js_compile.py to spend upwards of one minute of CPU time
in Python before even invoking the closure compiler.

Caching dependency subtrees takes the python CPU time down to about 50
milliseconds. That is, a 1000x performance boost.

Note the _set_ of arguments passed to closure is identical. However, the
order can change slightly without violating dependencies specified. This
shuffling exposed a couple of errors due to underspecified dependencies.
(Fix them):

ui/file_manager/file_manager/background/js/test_util_base.js:589:
  ERROR - Property args never defined on request

chrome/browser/resources/settings/site_settings/site_data_entry.js:15:
  ERROR - Variable referenced before declaration: I18nBehavior

TBR=hcarmona@chromium.org

Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I10c27bb76b404bb40828dfd54f99be059adf0fdb
Reviewed-on: https://chromium-review.googlesource.com/1166775
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581961}

chrome/browser/resources/settings/site_settings/BUILD.gn

@@ -155,6 +155,7 @@ js_library("site_data_entry") {
     ":local_data_browser_proxy",
     "..:focus_row_behavior",
     "//ui/webui/resources/js:cr",
+    "//ui/webui/resources/js:i18n_behavior",
     "//ui/webui/resources/js:icon",
   ]
 }

third_party/closure_compiler/README.chromium

@@ -19,3 +19,5 @@ Local modifications:
   third_party/closure_compiler/externs/chrome_extensions.js.
 - Some externs here are for chrome-specific use; these primarily include the
   *_private.js files.
+- Fix bug in js_binary.py causing it to drop externs.
+- Cache dependency subtrees in js_binary.py for a huge performance boost.

third_party/closure_compiler/js_binary.py

@@ -34,26 +34,48 @@ def ParseDepList(dep):
           lines[externs_start+1:])
 
 
-def CrawlDepsTree(deps, sources, externs):
+# Cache, to avoid reading the same file twice in the dependency tree and
+# processing its dependencies again.
+depcache = {}
+
+def AppendUnique(items, new_items):
+  """Append items in |new_items| to |items|, avoiding duplicates."""
+  # Note this is O(n*n), and assumes |new_items| is already unique, but this is
+  # not a bottleneck overall.
+  items += [i for i in new_items if i not in items]
+
+def CrawlDepsTree(deps):
   """Parses the dependency tree creating a post-order listing of sources."""
-  for dep in deps:
-    cur_sources, cur_deps, cur_externs = ParseDepList(dep)
+  global depcache
 
-    child_sources, child_externs = CrawlDepsTree(
-      cur_deps, cur_sources, cur_externs)
+  if len(deps) == 0:
+    return ([], [])
 
-    # Add child dependencies of this node first.
-    new_sources = child_sources
+  new_sources = []
+  new_externs = []
+  for dep in deps:
+    if dep in depcache:
+      cur_sources, cur_externs = depcache[dep]
+    else:
+      dep_sources, dep_deps, dep_externs = ParseDepList(dep)
+      cur_sources, cur_externs = CrawlDepsTree(dep_deps)
+      # Add child dependencies of this node before the current node, then cache.
+      AppendUnique(cur_sources, dep_sources)
+      AppendUnique(cur_externs, dep_externs)
+      depcache[dep] = (cur_sources, cur_externs)
 
     # Add the current node's sources and dedupe.
-    new_sources += [s for s in cur_sources if s not in new_sources]
+    AppendUnique(new_sources, cur_sources)
+    AppendUnique(new_externs, cur_externs)
+
+  return new_sources, new_externs
 
-    # Add the original sources, none of which will be dependencies of this node,
-    # and dedupe.
-    new_sources += [s for s in sources if s not in new_sources]
-    sources = new_sources
 
-    externs += [e for e in cur_externs if e not in externs]
+def CrawlRootDepsTree(deps, target_sources, target_externs):
+  """Parses the dependency tree and adds target sources."""
+  sources, externs = CrawlDepsTree(deps)
+  AppendUnique(sources, target_sources)
+  AppendUnique(externs, target_externs)
   return sources, externs
 
 
@@ -81,10 +103,10 @@ def main():
                       help='Only performs checks and writes an empty output')
 
   args = parser.parse_args()
-  sources, externs = CrawlDepsTree(args.deps, args.sources, args.externs)
+  sources, externs = CrawlRootDepsTree(args.deps, args.sources, args.externs)
 
   compiler_args = ['--%s' % flag for flag in args.flags]
-  compiler_args += ['--externs=%s' % e for e in args.externs]
+  compiler_args += ['--externs=%s' % e for e in externs]
   compiler_args += [
       '--js_output_file',
       args.output,

ui/file_manager/file_manager/background/js/BUILD.gn

@@ -39,7 +39,6 @@ js_type_check("closure_compile") {
 js_library("closure_compile_externs") {
   sources = []
   externs_list = [
-    "$externs_path/chrome_extensions.js",
     "$externs_path/command_line_private.js",
     "$externs_path/file_manager_private.js",
     "$externs_path/file_system_provider.js",
@@ -241,6 +240,17 @@ js_library("test_util_base") {
     ":app_windows",
     "../../common/js:error_util",
   ]
+
+  # The callback test_util_base.js passes to chrome.runtime.onMessageExternal()
+  # requires chrome_extensions.js, which has a dependency on chrome.js. Ensure
+  # the externs are introduced at the same time (in this order). Note we are
+  # lucky the list below also sorts in this order, or we'd need some other fix.
+  # A compile error results if the dependency walker encounters a target
+  # that puts chrome_extensions.js into the argument list before chrome.js.
+  externs_list = [
+    "$externs_path/chrome.js",
+    "$externs_path/chrome_extensions.js",
+  ]
 }
 
 js_library("volume_info_impl") {