Interestingly, win_clang and linux_chromium_rel_ng both did the compile because they have the 'all' additional target. I wonder if having 'all' there slows down CQ runs? This is probably another issue though.

Sounds like a bug in the trybot logic, tagging as chrome-specific. phajdan/dpranke could you triage further?

I don't see any reference to these files in either the GYP or GN builds. How are they actually pulled into the browser, and how would we know that?

(In other words, this looks like missing/underspecified dependencies to me, not a bug in analyze).

Separately, "all" definitely slows down builds, and the only reason things got built on the builders that do specify "all" is because of  bug 555273 , which I'm almost ready to land a fix for.

(feel free to bounce back to me when you tell me how these things are actually used in the build).

The files are referenced from:

https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=plugins_win.json&sq=package:chromium&l=327&dr=C

it sounds like whatever-step-is-consuming-browser_resources.grd be listing all these extra input files, there seem to be a lot of them.

Okay, so it looks like we don't have this issue for other files in //chrome/browser/resources because there's a whitelist exception for them:

https://cs.chromium.org/chromium/src/testing/buildbot/trybot_analyze_config.json?rcl=0&l=34

Ideally all of these whitelist exceptions should go away and be replaced by correct dependencies being listed as inputs in the GN files. I suspect that we don't currently do this because it would be painful to do this manually and doing it automatically (e.g., via calling out to a grit to return the list) would be too slow (IIRC we used to do this but brettw@ changed that).

So, the workaround for the moment would be to add the json files to the whitelist, and then punt this to brettw@ for a recommendation as to the right way to get the dependencies computed.

Oh yeah, none of the grit files are correct as far as analyze goes :(

We do handle incremental rebuilds for these properly by listing them out in a .d file when the grit file is processed. But this doesn't help analyze.

What I'm thinking is we have a build flag that enables a code path in the grit template ("force_grit_inputs_for_analyze_correctness"?) to shell out to grit and ask for the inputs for every .grd. Normal developers don't need this since the .d file makes compiles correct and it will slow down GN runs a lot.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd8c219c2282f2d6f238517c16c76b15fdf82f39

commit bd8c219c2282f2d6f238517c16c76b15fdf82f39
Author: brettw <brettw@chromium.org>
Date: Sat Oct 01 01:56:13 2016

Optionally compute grit inputs precisely.

Adds a new flag that will cause grit inputs to be computed precisely by
calling out to grit at GN-time.

This is off by default so it won't impact the performance of the normal runs
for developers (where it is not necessary). This can be turned explicitly for
bots that use analyze.

Updates the closure compiler template so there's not a pass-through group
in the dependency chain. That group is not helping and because the deps
were not public, the input was not allowed by the generated grit file.

Remove the default value for the resource_ids file that grit_info used. The
Android build depends on not setting a resource ID for a few files where it's
not needed, and referring to the shared produced an error because there was
no entry for the files in the ID file.

BUG= 639328 

Review-Url: https://codereview.chromium.org/2375283004
Cr-Commit-Position: refs/heads/master@{#422268}

[modify] https://crrev.com/bd8c219c2282f2d6f238517c16c76b15fdf82f39/.gn
[modify] https://crrev.com/bd8c219c2282f2d6f238517c16c76b15fdf82f39/content/browser/devtools/BUILD.gn
[modify] https://crrev.com/bd8c219c2282f2d6f238517c16c76b15fdf82f39/content/browser/tracing/BUILD.gn
[modify] https://crrev.com/bd8c219c2282f2d6f238517c16c76b15fdf82f39/third_party/google_input_tools/closure.gni
[modify] https://crrev.com/bd8c219c2282f2d6f238517c16c76b15fdf82f39/tools/grit/grit_info.py
[modify] https://crrev.com/bd8c219c2282f2d6f238517c16c76b15fdf82f39/tools/grit/grit_rule.gni

Now that the above change is in, we should be able to set compute_grit_inputs_for_analyze on the bots to fix this bug. Assigning to Dirk.

 Issue 662261  has been merged into this issue.

 Issue 698387  has been merged into this issue.

is there any additional work required here to fix this issue?

I need to flip a GN flag. I'll try to post a CL for this today.

"today", he said three and a half months ago ...

friendly ping, Dirk can you do this or reassign?

In comment #18, I actually did work on this and post a patch, and then I got distracted again. I'll try to get this landed today:

https://chromium-review.googlesource.com/c/545150/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d

commit a3727f9e4441fb0dc1fa7d8df8317ea0af40519d
Author: Dirk Pranke <dpranke@chromium.org>
Date: Mon Jul 17 17:30:33 2017

set compute_grit_inputs_for_analyze=True in the analyze step.

Normally when grit actions are defined during `gn gen`, we don't
include all of the files that the .grd file depends on as inputs,
because we don't need to; grit generates a .d file for dependencies
so that rebuilds will happen correctly. And, computing the 
dependencies is slow (it's equivalent to calling grit).

However, if we don't include the dependencies in the gn build graph,
then `analyze` doesn't know about them, and hence a change to a file
that grit dependencies on will be ignored.

This CL changes things such that when `gn gen` is being run as part
of the `analyze` step, we do set the extra flag to compute the
dependencies. This'll slow GN down, but that's better than 
misreporting dependencies.

In some cases, the .grd files try to include generated files
(which may not exist at GN time). To work around this, you can
mark the grit targets as `source_is_generated = true`; the
downside of that is that that target will be skipped just as if
compute_grit_inputs_for_analyze=False, however, this is still an
improvement over not checking *any* of the grit targets. A better
fix would be to have grit_info ignore any paths in the generated
directory if they don't exist, and I'll try to do that in a 
follow-up patch.

R=brettw@chromium.org
BUG= 639328 

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9f6702b06f8a41a159e4599bf192db0abf9e77eb
Reviewed-on: https://chromium-review.googlesource.com/545150
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487135}
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/android_webview/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/chrome/browser/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/chrome/browser/resources/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/chrome/browser/resources/settings/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/content/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/ios/web/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/ios/web/shell/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/ios/web/test/BUILD.gn
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/tools/grit/grit_info.py
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/tools/grit/grit_rule.gni
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/tools/mb/mb.py
[modify] https://crrev.com/a3727f9e4441fb0dc1fa7d8df8317ea0af40519d/ui/resources/BUILD.gn

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ae16c71ded107455163b329894caa5aa0a08eec

commit 7ae16c71ded107455163b329894caa5aa0a08eec
Author: Will Harris <wfh@chromium.org>
Date: Wed Oct 10 17:13:17 2018

Remove superfluous plugin metadata.

These are old plugins that never load. See http://shortn/_mYd6V5DGXg.

Also, make sure resources get rebuilt correctly when metadata is
updated.

BUG=692135, 639328 

Change-Id: I66d7a2290dc4a489db21442503baa9d4c9a832d5
Reviewed-on: https://chromium-review.googlesource.com/c/1263349
Commit-Queue: Will Harris <wfh@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598373}
[modify] https://crrev.com/7ae16c71ded107455163b329894caa5aa0a08eec/chrome/browser/BUILD.gn
[modify] https://crrev.com/7ae16c71ded107455163b329894caa5aa0a08eec/chrome/browser/resources/plugin_metadata/plugins_linux.json
[modify] https://crrev.com/7ae16c71ded107455163b329894caa5aa0a08eec/chrome/browser/resources/plugin_metadata/plugins_mac.json
[modify] https://crrev.com/7ae16c71ded107455163b329894caa5aa0a08eec/chrome/browser/resources/plugin_metadata/plugins_win.json