Clean up deps vs data_deps handling for repack() uses.

Instead of

foo("bar") {
  deps = [ ":repack" ]
  data = [ "$root_out_dir/repack_output.pak" ]
}

use

foo("bar") {
  data_deps = [ ":repack" ]
}

This happens to remove `$root_out_dir/locales/` from data,
which is good for correct incremental builds (see bug).

I didn't touch android and iOS, since I don't know the
android_assets() / the copy_data_to_bundle approach used
in iOS well.

(For similar bundling reasons, most of the `data` lines were already in
!is_mac blocks.)

While comparing browser_tests.runtime_deps as described in comment 18
on the linked bug, I noticed that we include .pak.info files in the
isolate for repack()ed files. Since those are build-time things, remove
them from repack() outputs, so that data_deps doesn't add these to the
.runtime_deps files. (For the build, the removal is harmless since deps
are still tracked through the action stamp files.)

One side effect of this change is that most test binaries defined
in chrome/test now get all locales in their isolate, instead of just
en-US or en-US and fr previously.

The target that includes ui_tests.pak is //ui/resources:ui_test_pak_data,
switch to that in a few places -- the previous ui_test_pak target
pulled in ui/en-US.pak which apparently isn't needed anywhere.

TBR=torne,sergeyu

Bug: 912946
Change-Id: I0d9c6ff0d3b00cd5a876443b61b083a1aabf24d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989515
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729946}

chrome/BUILD.gn

@@ -184,14 +184,9 @@ if (!is_android && !is_mac) {
       "$root_out_dir/resources.pak",
     ]
     if (is_linux || is_win) {
-      data += [
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/locales/en-US.pak",
-        "$root_out_dir/locales/fr.pak",
+      data_deps = [
+        "//chrome:packed_resources",
       ]
-      if (enable_hidpi) {
-        data += [ "$root_out_dir/chrome_200_percent.pak" ]
-      }
     }
 
     data_deps = []
@@ -345,6 +340,9 @@ if (!is_android && !is_mac) {
     # These files are used by the installer so we need a public dep.
     public_deps += [ ":packed_resources" ]
 
+    # The step's output are needed at runtime, so we also need a data_dep.
+    data_deps += [ ":packed_resources" ]
+
     # Only ChromeOS has precompiled Flash that needs to get copied to the output
     # directory. On other platforms, Flash is either component-updated only or
     # not supported at all.
@@ -1212,7 +1210,6 @@ if (is_win) {
       ":chrome_framework_resources",
       ":chrome_framework_services",
       ":keystone_registration_framework",
-      ":packed_resources",
       ":swiftshader_library",
       ":widevine_cdm_library",
       "//chrome/browser/resources/media/mei_preload:component_bundle",

chrome/browser/vr/BUILD.gn

@@ -599,7 +599,6 @@ source_set("vr_test_support") {
 
   public_deps = [
     ":vr_common",
-    ":vr_test_pak",
     ":vr_ui",
     "//base/test:test_support",
     "//cc:test_support",
@@ -620,8 +619,8 @@ source_set("vr_test_support") {
     # doesn't propagate to individual test executable targets.
   ]
 
-  data = [
-    "$root_out_dir/vr_test.pak",
+  data_deps = [
+    ":vr_test_pak",
   ]
 }
 

chrome/browser/vr/testapp/BUILD.gn

@@ -23,7 +23,6 @@ executable("vr_testapp") {
 
   deps = [
     ":assets_component_version_header",
-    ":vr_testapp_pak",
     ":vr_testapp_resources",
     "//chrome/browser/vr:vr_common",
     "//chrome/browser/vr:vr_test_support",
@@ -41,6 +40,9 @@ executable("vr_testapp") {
     "//ui/ozone",
     "//ui/platform_window",
   ]
+  data_deps = [
+    ":vr_testapp_pak",
+  ]
 }
 
 process_version("assets_component_version_header") {

chrome/chrome_paks.gni

@@ -310,6 +310,9 @@ template("chrome_paks") {
     if (enable_hidpi) {
       public_deps += [ ":${target_name}_200_percent" ]
     }
+    if (!defined(invoker.copy_data_to_bundle) || !invoker.copy_data_to_bundle) {
+      data_deps = public_deps
+    }
     if (defined(invoker.public_deps)) {
       public_deps += invoker.public_deps
     }

chrome/test/BUILD.gn

@@ -13,7 +13,6 @@ import("//build/config/ui.gni")
 import("//build/toolchain/toolchain.gni")
 import("//build/util/version.gni")
 import("//chrome/browser/buildflags.gni")
-import("//chrome/chrome_repack_locales.gni")
 import("//chrome/common/features.gni")
 import("//chrome/test/base/js2gtest.gni")
 import("//chromeos/assistant/assistant.gni")
@@ -682,7 +681,6 @@ if (!is_android) {
       "//base/test:test_support",
       "//base/util/memory_pressure:test_support",
       "//build:branding_buildflags",
-      "//chrome:browser_tests_pak",
       "//chrome:packed_resources",
       "//chrome:resources",
       "//chrome:strings",
@@ -770,6 +768,7 @@ if (!is_android) {
 
     # Runtime dependencies
     data_deps = [
+      "//chrome:browser_tests_pak",
       "//chrome/browser/resources/media/mei_preload:component",
 
       # TODO(thakis): Why do these need copying in browser_tests?
@@ -817,7 +816,6 @@ if (!is_android) {
       "//third_party/simplejson/",
       "//third_party/tlslite/",
       "//ui/webui/resources/",
-      "$root_out_dir/browser_tests.pak",
     ]
     data += js2gtest_js_libraries
 
@@ -1534,12 +1532,7 @@ if (!is_android) {
     }
 
     if (!is_mac) {
-      data += [
-        "$root_out_dir/locales/",
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/chrome_200_percent.pak",
-        "$root_out_dir/resources.pak",
-      ]
+      data_deps += [ "//chrome:packed_resources" ]
     }
 
     if (enable_captive_portal_detection) {
@@ -3602,12 +3595,7 @@ test("unit_tests") {
     ]
   }
   if (is_linux || is_win) {
-    data += [
-      "$root_out_dir/chrome_100_percent.pak",
-      "$root_out_dir/chrome_200_percent.pak",
-      "$root_out_dir/locales/en-US.pak",
-      "$root_out_dir/resources.pak",
-    ]
+    data_deps += [ "//chrome:packed_resources" ]
   }
   if (is_win) {
     data_deps += [ "//chrome" ]
@@ -5119,7 +5107,7 @@ test("unit_tests") {
     if (is_android) {
       deps += [ "//chrome/android:chrome_apk_paks" ]
     } else {
-      deps += [ "//chrome:packed_resources" ]
+      data_deps += [ "//chrome:packed_resources" ]
     }
   }
   if (is_win || is_mac || is_chromeos) {
@@ -5695,18 +5683,8 @@ if (!is_android) {
       "$root_out_dir/test_case.html.mock-http-headers",
       "$root_out_dir/test_page.css",
       "$root_out_dir/test_page.css.mock-http-headers",
-      "$root_out_dir/ui_test.pak",
     ]
     data += js2gtest_js_libraries
-    if (is_linux || is_win) {
-      data += [
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/chrome_200_percent.pak",
-        "$root_out_dir/locales/en-US.pak",
-        "$root_out_dir/locales/fr.pak",
-        "$root_out_dir/resources.pak",
-      ]
-    }
     if (is_linux) {
       data += [ "$root_out_dir/libppapi_tests.so" ]
     }
@@ -5753,7 +5731,6 @@ if (!is_android) {
       "//ui/base:test_support",
       "//ui/base/clipboard:clipboard_test_support",
       "//ui/events:events_interactive_ui_tests",
-      "//ui/resources:ui_test_pak",
       "//ui/web_dialogs:test_support",
     ]
 
@@ -5765,7 +5742,11 @@ if (!is_android) {
     data_deps = [
       "//ppapi:ppapi_tests",
       "//third_party/mesa_headers",
+      "//ui/resources:ui_test_pak_data",
     ]
+    if (is_linux || is_win) {
+      data_deps += [ "//chrome:packed_resources" ]
+    }
 
     if (use_aura) {
       sources += [ "../browser/ui/views/drag_and_drop_interactive_uitest.cc" ]
@@ -6251,15 +6232,6 @@ if (!is_android && !is_fuchsia) {
       "//testing/xvfb.py",
     ]
 
-    if (is_linux || is_win) {
-      data += [
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/chrome_200_percent.pak",
-        "$root_out_dir/locales/en-US.pak",
-        "$root_out_dir/resources.pak",
-      ]
-    }
-
     # TODO(phajdan.jr): Only temporary, to make transition easier.
     defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
 
@@ -6289,6 +6261,10 @@ if (!is_android && !is_fuchsia) {
       "//third_party/mesa_headers",
     ]
 
+    if (is_linux || is_win) {
+      data_deps += [ "//chrome:packed_resources" ]
+    }
+
     if (is_mac) {
       # Dictionary sync is disabled on Mac.
       sources -= [
@@ -6489,11 +6465,8 @@ if (!is_android && !is_fuchsia) {
     ]
 
     if (is_linux || is_win) {
-      data += [
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/chrome_200_percent.pak",
-        "$root_out_dir/locales/en-US.pak",
-        "$root_out_dir/resources.pak",
+      data_deps = [
+        "//chrome:packed_resources",
       ]
     }
 
@@ -6665,11 +6638,8 @@ if (is_win) {
       ]
     }
     if (!is_mac) {
-      data += [
-        "$root_out_dir/locales/",
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/chrome_200_percent.pak",
-        "$root_out_dir/resources.pak",
+      data_deps = [
+        "//chrome:packed_resources",
       ]
     }
   }
@@ -6852,7 +6822,6 @@ if (!is_android) {
 
     deps = [
       "//base",
-      "//chrome:packed_resources",
       "//chrome/test:browser_tests_runner",
       "//device/base",
       "//services/service_manager/sandbox",
@@ -6869,11 +6838,8 @@ if (!is_android) {
     ]
 
     if (!is_mac) {
-      data += [
-        "$root_out_dir/locales/",
-        "$root_out_dir/chrome_100_percent.pak",
-        "$root_out_dir/chrome_200_percent.pak",
-        "$root_out_dir/resources.pak",
+      data_deps = [
+        "//chrome:packed_resources",
       ]
     }
   }

components/BUILD.gn

@@ -50,10 +50,6 @@ test("components_unittests") {
   if (is_android || is_linux || is_mac || is_win) {
     data = [
       "test/data/",
-
-      # TODO(dpranke): Remove the next two lines after GN has rolled to 339778.
-      "$root_out_dir/components_tests_resources.pak",
-      "$root_out_dir/ui_test.pak",
     ]
   }
 
@@ -296,7 +292,7 @@ test("components_unittests") {
     data_deps = [
       ":components_tests_pak",
       "//third_party/mesa_headers",
-      "//ui/resources:ui_test_pak",
+      "//ui/resources:ui_test_pak_data",
     ]
   }  # iOS/!iOS
 
@@ -670,15 +666,13 @@ if (!is_ios && !is_fuchsia) {
 
     data_deps = [
       ":components_tests_pak",
-      "//ui/resources:ui_test_pak",
+      "//ui/resources:ui_test_pak_data",
 
       # Needed for isolate script to execute.
       "//testing:run_perf_test",
     ]
 
     data = [
-      "$root_out_dir/components_tests_resources.pak",
-      "$root_out_dir/ui_test.pak",
       "//components/subresource_filter/core/common/perftests/data/",
       "//third_party/subresource-filter-ruleset/data/",
     ]

fuchsia/engine/BUILD.gn

@@ -68,7 +68,6 @@ component("web_engine_core") {
   deps = [
     ":mojom",
     ":switches",
-    ":web_engine_pak",
     "//base",
     "//base:base_static",
     "//base/util/memory_pressure",

tools/grit/repack.gni

@@ -54,12 +54,8 @@ template("repack") {
     script = "//tools/grit/pak_util.py"
 
     inputs = invoker.sources
-    foreach(_invoker_source, invoker.sources) {
-      inputs += [ "${_invoker_source}.info" ]
-    }
     outputs = [
       invoker.output,
-      "${invoker.output}.info",
     ]
 
     args = [ "repack" ]
@@ -194,5 +190,8 @@ template("repack_locales") {
                              "testonly",
                            ])
     public_deps = _locale_targets
+    if (!defined(invoker.copy_data_to_bundle) || !invoker.copy_data_to_bundle) {
+      data_deps = public_deps
+    }
   }
 }

tools/mb/mb.py

@@ -1230,7 +1230,6 @@ class MetaBuildWrapper(object):
       # TODO(https://crbug.com/912946): Remove this if statement.
       if ((is_msan and f == 'instrumented_libraries_prebuilt/') or
           f == 'mr_extension/' or # https://crbug.com/997947
-          f == 'locales/' or
           f.startswith('nacl_test_data/') or
           f.startswith('ppapi_nacl_tests_libs/') or
           (is_cros and f in (  # https://crbug.com/1002509

ui/chromeos/BUILD.gn

@@ -76,12 +76,13 @@ test("ui_chromeos_unittests") {
     "//ui/events:test_support",
     "//ui/gl:test_support",
     "//ui/message_center",
-    "//ui/resources:ui_test_pak",
     "//ui/views",
     "//ui/views:test_support",
   ]
   data = [
     "$root_out_dir/locales/en-US.pak",
-    "$root_out_dir/ui_test.pak",
+  ]
+  data_deps = [
+    "//ui/resources:ui_test_pak_data",
   ]
 }

ui/gfx/BUILD.gn

@@ -779,7 +779,6 @@ test("gfx_unittests") {
       "transform_util_unittest.cc",
       "utf16_indexing_unittest.cc",
     ]
-    data += [ "$root_out_dir/ui_test.pak" ]
   }
 
   if (is_win) {
@@ -804,7 +803,6 @@ test("gfx_unittests") {
     "//ui/gfx/animation",
     "//ui/gfx/geometry",
     "//ui/gfx/range",
-    "//ui/resources:ui_test_pak",
   ]
 
   if (!is_ios) {
@@ -816,7 +814,7 @@ test("gfx_unittests") {
   }
 
   data_deps = [
-    "//ui/resources:ui_test_pak",
+    "//ui/resources:ui_test_pak_data",
   ]
 
   if (is_mac || is_ios) {

weblayer/shell/BUILD.gn

@@ -250,7 +250,6 @@ if (is_android) {
     defines = []
 
     deps = [
-      ":pak",
       ":weblayer_shell_lib",
       "//base",
       "//build/win:default_exe_manifest",