Web UI: Update unpack_pak to detect files outside the directory

- Add pak_base_dir option to map generated files based on a root
directory other than the directory where the pak file is located.
This is the case for the os settings build, where the pak file is
under gen/c/b/r/settings/chromeos, but paths should be based on
gen/c/b/r/settings.
- Add exclude option to exclude certain files from being unpacked.
This is to allow UIs like the NTP, downloads, and OS settings to
keep files that should not be created by unpack_pak in their grd files.
- Add an assert, ensure unpack_pak.py never creates files/directories
outside of the unpak folder in future. Creating these directories can
cause post-CQ build failures due to race conditions, as seen in the
linked bug.

Bug: 1093405
Change-Id: If85ea9f08c4dbe008ec16ca0a457390012833c56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2251050Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780364}

chrome/browser/resources/downloads/BUILD.gn

@@ -32,6 +32,7 @@ if (optimize_webui) {
   unpak("unpak") {
     pak_file = downloads_pak_file
     out_folder = unpak_folder
+    excludes = [ "../../ui/webui/downloads/downloads.mojom-lite.js" ]
 
     deps = [ ":flattened_resources" ]
   }

chrome/browser/resources/new_tab_page/BUILD.gn

@@ -262,6 +262,12 @@ if (optimize_webui) {
     pak_file = "new_tab_page_resources.pak"
     out_folder = "unpak"
     deps = [ ":unoptimized_resources" ]
+
+    excludes = [
+      "../../ui/webui/new_tab_page/new_tab_page.mojom-lite.js",
+      "../../../common/search/omnibox.mojom-lite.js",
+      "../../../../skia/public/mojom/skcolor.mojom-lite.js",
+    ]
   }
 
   optimize_webui("optimized_js") {

chrome/browser/resources/optimize_webui.gni

@@ -103,5 +103,16 @@ template("unpak") {
       "--pak_file",
       rebase_path("$target_gen_dir/${invoker.pak_file}", root_build_dir),
     ]
+
+    if (defined(invoker.pak_base_dir)) {
+      args += [
+        "--pak_base_dir",
+        rebase_path("$target_gen_dir/${invoker.pak_base_dir}", root_build_dir),
+      ]
+    }
+
+    if (defined(invoker.excludes)) {
+      args += [ "--excludes" ] + invoker.excludes
+    }
   }
 }

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

@@ -56,7 +56,20 @@ if (optimize_webui) {
   unpak("unpak") {
     pak_file = settings_pak_file
     out_folder = unpak_folder
-
+    pak_base_dir = "../"
+    excludes = [
+      "../../ui/webui/settings/chromeos/constants/routes.mojom-lite.js",
+      "../../ui/webui/settings/chromeos/constants/setting.mojom-lite.js",
+      "../../../../skia/public/mojom/bitmap.mojom-lite.js",
+      "../../../../mojo/public/mojom/base/file_path.mojom-lite.js",
+      "../../../../skia/public/mojom/image_info.mojom-lite.js",
+      "../../../../ui/gfx/image/mojom/image.mojom-lite.js",
+      "../../ui/webui/app_management/app_management.mojom-lite.js",
+      "../../../../components/services/app_service/public/mojom/types.mojom-lite.js",
+      "../../ui/webui/settings/chromeos/search/search.mojom-lite.js",
+      "../../ui/webui/settings/chromeos/search/search_result_icon.mojom-lite.js",
+      "../../ui/webui/settings/chromeos/search/user_action_recorder.mojom-lite.js",
+    ]
     deps = [ ":flattened_resources" ]
   }
 

chrome/browser/resources/unpack_pak.py

@@ -24,7 +24,7 @@ from grit.format import data_pack
 def ParseLine(line):
   return re.match('  {"([^"]+)", ([^},]+)', line)
 
-def GetFileAndDirName(out_path, pak_dir, resource_path):
+def GetFileAndDirName(pak_dir, resource_path):
   dirname = os.path.dirname(resource_path)
   # When files are generated, |dirname| becomes
   # @out_folder@/<gen_path>/path_to_resource. To make the structure look as if
@@ -32,10 +32,10 @@ def GetFileAndDirName(out_path, pak_dir, resource_path):
   if ('@out_folder@' in dirname):
     dirname = os.path.relpath(dirname, os.path.join('@out_folder@', pak_dir))
   filename = os.path.basename(resource_path)
-  dirname = os.path.join(out_path, dirname)
+
   return (filename, dirname)
 
-def Unpack(pak_path, out_path):
+def Unpack(pak_path, out_path, pak_base_dir, excludes):
   pak_dir = os.path.dirname(pak_path)
   pak_id = os.path.splitext(os.path.basename(pak_path))[0]
 
@@ -62,13 +62,26 @@ def Unpack(pak_path, out_path):
         resource_filenames[res.group(2)] = res.group(1)
   assert resource_filenames
 
+  root_dir = pak_base_dir if pak_base_dir else pak_dir
+  excludes = excludes or []
   # Extract packed files, while preserving directory structure.
   for (resource_id, text) in data.resources.iteritems():
     (filename, dirname) = GetFileAndDirName(
-        out_path, pak_dir, resource_filenames[resource_ids[resource_id]])
-    if not os.path.exists(dirname):
-      os.makedirs(dirname)
-    with open(os.path.join(dirname, filename), 'w') as file:
+        root_dir, resource_filenames[resource_ids[resource_id]])
+    resource_path = os.path.join(dirname, filename).replace('\\', '/')
+    if (resource_path in excludes):
+      continue
+
+    normalized_dir = os.path.normpath(
+        os.path.join(out_path, dirname)).replace('\\', '/')
+    assert normalized_dir.startswith(out_path), \
+           'Cannot unpack files to locations not in %s. %s should be removed ' \
+           'from the pak file or excluded from unpack.' \
+           % (out_path, resource_path)
+
+    if not os.path.exists(normalized_dir):
+      os.makedirs(normalized_dir)
+    with open(os.path.join(normalized_dir, filename), 'w') as file:
       file.write(text)
 
 
@@ -76,9 +89,11 @@ def main():
   parser = argparse.ArgumentParser()
   parser.add_argument('--pak_file')
   parser.add_argument('--out_folder')
+  parser.add_argument('--pak_base_dir')
+  parser.add_argument('--excludes', nargs='*')
   args = parser.parse_args()
 
-  Unpack(args.pak_file, args.out_folder)
+  Unpack(args.pak_file, args.out_folder, args.pak_base_dir, args.excludes)
 
   timestamp_file_path = os.path.join(args.out_folder, _TIMESTAMP_FILENAME)
   with open(timestamp_file_path, 'a'):

chrome/browser/resources/unpack_pak_test.py

@@ -16,17 +16,16 @@ class UnpackPakTest(unittest.TestCase):
     self.assertTrue(unpack_pak.ParseLine('  {"path.js", IDR_PATH, true}'))
 
   def testGetFileAndDirName(self):
-    (f, d) = unpack_pak.GetFileAndDirName(
-        'out/build/gen/foo/foo.unpak', 'out/build/gen/foo', 'a/b.js')
+    (f, d) = unpack_pak.GetFileAndDirName('out/build/gen/foo', 'a/b.js')
     self.assertEquals('b.js', f)
-    self.assertEquals('out/build/gen/foo/foo.unpak/a', d)
+    self.assertEquals('a', d)
 
   def testGetFileAndDirNameForGeneratedResource(self):
     (f, d) = unpack_pak.GetFileAndDirName(
-        'out/build/gen/foo/foo.unpak', 'out/build/gen/foo',
+        'out/build/gen/foo',
         '@out_folder@/out/build/gen/foo/a/b.js')
     self.assertEquals('b.js', f)
-    self.assertEquals('out/build/gen/foo/foo.unpak/a', d)
+    self.assertEquals('a', d)
 
 if __name__ == '__main__':
   unittest.main()