New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 661774 link

Starred by 5 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 715131



Sign in to add a comment

"gn analyze" step returned no targets due to incomplete source list

Project Member Reported by wychen@chromium.org, Nov 2 2016

Issue description

When landing this CL below, none of the bots actually compiled any targets because "gn analyze" step returned nothing. The root cause is that the changed file was not added to the source list in the GN files.

https://codereview.chromium.org/2469013004/

ninja can trigger the build correctly if I touch the file. Is it possible to detect incomplete source lists in GN files automatically by detecting the discrepancy between "gn analyze" and ninja?
 
Showing comments 7 - 106 of 106 Older
Labels: OS-All

Comment 8 by wychen@chromium.org, Nov 17 2016

Cc: jbudorick@chromium.org
Labels: -Pri-3 Pri-2
Owner: wychen@chromium.org
Status: Assigned (was: Available)
jbudorick@, how do I get started writing a tool to validate this on CQ bots? To run this on all platforms, I can no longer use shell scripts, right?

To avoid stale .d files and avoid clobbing too much, I think it makes sense to only run this on CQ bots with target "all", so that the .d files are always fresh after the compile step.
@wychen: yes, you'd need to rewrite this script in python. How long does the existing script take to run? 
The script should take less than 10s, either in shell script or in python. Given the fast speed, it looks like a good candidate for presubmit, but I'm afraid needing fresh .d files would block this.

Comment 11 by st...@chromium.org, Nov 17 2016

Cc: chanli@chromium.org
Is it possible to just check those files affecting the compile targets which are determined by "gn analyze"? In such case, will those .d be up-to-date?

"ninja -t graph/deps" might be helpful here.
Status: Started (was: Assigned)
Wow. "ninja -t deps" looks much better than "-d keepdepfile" and parsing those .d files. Thanks for the tip!

To avoid analyzing the target dependency, my first step is to focus on target "all". We can get more fine-grained checking if that becomes necessary.
A draft is here:

https://codereview.chromium.org/2510303002/
Re #c10: Needing to compile first and not allowing stale .d files make it impossible to be a presubmit test.
My plan is to run https://codereview.chromium.org/2510303002/ on CQ, gather all the currently missing files from stdout of the script, put them in a whitelist, and turn on this checking on CQ. This allows gradual improvements while catch further regression.

How do I run the script on CQ?

Comment 16 by st...@chromium.org, Nov 18 2016

I'm not aware of an approach to run the script/CL through CQ without change to the chromium_trybot recipe or related recipe modules.

My thought is to add a step after compile succeeds in https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium/api.py?l=160
In that case, we have to move the script to the build-side repo.

@dpranke, is this the right approach?
wychen asked about this out-of-band, too -- could we do this in //testing/scripts/ & the buildbot jsons?
Cc: thakis@chromium.org
Unfortunately, I think the only way you could actually implement this would be as a step that ran unconditionally after a full ("all") compile.

As such, you couldn't really do this with a test step, which analyze might cause to be skipped.

And I'm kinda reluctant to add this as a custom step on the builds without thinking about the potential impact a lot more.

That leads me to wonder if there might be some other way to solve this problem ...

+thakis in case he also has thoughts about this.
I gave it more though, and I'd like to hear your comments about the plan:
https://docs.google.com/document/d/1TZtquZp1uv1ENe2Okk8zyyeJP03hz-UiLRAdFly_PR8/edit#
I'd really like thakis' and brettw's thoughts on your document. 

In my idealistic view of the world, we'd make `gn check` smart enough to catch everything without a significant performance impact on GN or too much additional complexity in the code. I know historically brettw@ didn't really think that was possible (or at least worth it).
Cc: scottmg@chromium.org
 Issue 677355  has been merged into this issue.
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 10 2017

Let's continue the discussion from https://codereview.chromium.org/2510303002/#msg32 here.

On 2017/01/10 03:57:56, Dirk Pranke wrote:
> On the tryservers, builders run the `analyze` step in order to only (re)build
> the targets that are affected by the change, and to only re-run the test steps
> affected by the change. But I don't know how to say which targets to rebuild
> for this particular test, or when we'd need to re-run it.

If the dependency relationship is correctly represented in GN, i.e. there's no
missing headers in GN, "analyze" step should be able to correctly tell which
targets to rebuild. Before that happens, wrongly skipping targets is what we
want to fix, but also the cause of under-triggering of our checking script. The
workaround for this is to enable the checking on bots that have the target
"all", so that we don't have to worry about the target. "When to run the script"
would be the tough issue here.

> That's not an insignificant amount of time for a change that otherwise affects
> almost
> nothing. It's doable, but I'd prefer to make things work "properly" if we can
> figure
> out what "properly" would mean.
> I wonder if we're just overthinking this. Can we get 90%+ of this if we just
> check
> that whenever a {.cc,.h,.mm,.m} is added or removed to the repo then BUILD
files
> are modified to add or remove the corresponding entries?

This script is the costly but accurate checking. The only case it can miss the
error is when "analyze" step wrongly skips compilation. It seems we cannot
afford running this for all the CLs on CQ, so an early-abortion heuristic
targeting those suspicious CLs might be needed in order to save time. How about
running this accurate test only when the CL contains new header files? Other
types like .cc can be ignored because missing a .cc file in GN would cause
errors detectable by other means.

There are a few triggering options on the pareto curve:
- Never (fastest, no control on regression, current situation)
- When the CL contains new header files but no modification of GN files
- When the CL contains new header files (my preference right now)
- When the CL contains new header files or GN files are modified
- Always as long as compilation is triggered (slowest, most accurate, as of
PS13)
- Always, no matter what (not possible, ground truth)

Note that the ground truth is not possible, so it is always possible for a
culprit CL to go through the CQ, and get caught on another unrelated CL on CQ,
or eventually on waterfall. I guess the criteria is to make this infrequent
enough so that the whole process is a net-positive in productivity. WDYT?
Project Member

Comment 24 by bugdroid1@chromium.org, Jan 10 2017

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

commit 7b07e7b5c50594bc8fb536737ce745f1446a57b5
Author: wychen <wychen@chromium.org>
Date: Tue Jan 10 17:48:29 2017

Fix GN missing headers in //base

BUG=661774

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

[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/base/BUILD.gn
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/base/debug/stack_trace_posix.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/base/third_party/libevent/BUILD.gn
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/apps/app_url_redirector_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/component_updater/pnacl_component_installer.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/extensions/api/cast_streaming/performance_test.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/extensions/api/tab_capture/tab_capture_performancetest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/extensions/extension_loading_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/nacl_host/test/nacl_gdb_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/password_manager/password_store_factory.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/ui/views/create_application_shortcut_view.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/browser/ui/views/profiles/new_avatar_button.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/common/chrome_paths.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/test/nacl/nacl_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/chrome/test/ppapi/ppapi_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/components/nacl/browser/nacl_browser.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/components/nacl/browser/nacl_process_host.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/components/update_client/update_query_params.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/components/update_client/utils.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/components/url_formatter/elide_url_unittest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/content/common/content_paths.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/content/renderer/media/webrtc_audio_device_impl.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/7b07e7b5c50594bc8fb536737ce745f1446a57b5/ui/app_list/views/app_list_view.cc

A massive batch fix is done by thakis@
https://codereview.chromium.org/2770693003/

This CL reduced the number of missing headers from 1032 to 787!
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 26 2017

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

commit 42366da1ac6b2186e0569751941fd1a5a995801c
Author: thakis <thakis@chromium.org>
Date: Sun Mar 26 03:06:56 2017

Remove unused base/memory/singleton_objc.h

BUG=661774

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

[delete] https://crrev.com/c7749ac462ce8f82dba45a456d8aab291d281b1c/base/memory/singleton_objc.h

Project Member

Comment 32 by bugdroid1@chromium.org, Mar 26 2017

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

commit c6f53dc00d26c67ce3e8d9571996a0c7666b0288
Author: thakis <thakis@chromium.org>
Date: Sun Mar 26 03:29:29 2017

Remove unused ExceptionStatePlaceholder.cpp

BUG=661774

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

[delete] https://crrev.com/e14291f0adaf71cd42ef1c1a9d70ace4e309a1ba/third_party/WebKit/Source/bindings/core/v8/ExceptionStatePlaceholder.cpp

Project Member

Comment 33 by bugdroid1@chromium.org, Mar 26 2017

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

commit 8318c659370c46679a96d84b5735a708110c53fd
Author: thakis <thakis@chromium.org>
Date: Sun Mar 26 03:45:06 2017

Add a few missing files (including a test) to base/BUILD.gn

BUG=661774

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

[modify] https://crrev.com/8318c659370c46679a96d84b5735a708110c53fd/base/BUILD.gn

Project Member

Comment 34 by bugdroid1@chromium.org, Mar 26 2017

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

commit 71d33c3169cf175d2d367834907b86c4023c0496
Author: thakis <thakis@chromium.org>
Date: Sun Mar 26 04:59:05 2017

Add most missing blink files to .gn files

I copied the list of missing files to a txt file and ran this script:

    import os, re, subprocess
    edits = {}
    for filename in open('headers.txt'):
      filename = filename.strip()
      dirname = os.path.dirname(filename)
      while not os.path.exists(os.path.join(dirname, 'BUILD.gn')):
	dirname = os.path.dirname(dirname)
      rel = filename[len(dirname) + 1:]
      gnfile = os.path.join(dirname, 'BUILD.gn')
      #print gnfile, rel

      lines = open(gnfile).read().splitlines()
      index = next( (i for i, l in enumerate(lines) if ' sources = [' in l), -1)
      if index == -1:
	continue
      lines.insert(index + 1, '"%s",' % rel)
      open(gnfile, 'w').write('\n'.join(lines))

Then I ran `git cl format` to re-sort source lists.

I manually fixed up
third_party/WebKit/Source/platform/BUILD.gn,
third_party/WebKit/public/BUILD.gn,
third_party/WebKit/Source/platform/BUILD.gn,
where this simple heuristic hadn't worked well.

I reverted changes to third_party/WebKit/Source/platform/mojo/BUILD.gn
because I don't understand that directory.

BUG=661774
NOTRY=true
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/animation/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/clipboard/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/css/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/dom/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/events/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/frame/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/html/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/inspector/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/layout/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/loader/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/paint/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/style/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/core/svg/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/crypto/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/device_light/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/device_orientation/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/geolocation/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/mediastream/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/nfc/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/peerconnection/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/speech/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/storage/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/webdatabase/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/modules/webgl/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/platform/heap/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/platform/loader/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/Source/wtf/BUILD.gn
[modify] https://crrev.com/71d33c3169cf175d2d367834907b86c4023c0496/third_party/WebKit/public/BUILD.gn

Project Member

Comment 35 by bugdroid1@chromium.org, Mar 27 2017

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

commit 6625b2f764ae4efb0637215660500ba721b8a43d
Author: thakis <thakis@chromium.org>
Date: Mon Mar 27 03:21:27 2017

Remove sandbox/win/tools.

The files therein look unused, haven't been really touched since 2012, aren't
referenced in any build files, and have some maintenance cost.

BUG=661774
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

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

[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/finder.cc
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/finder.h
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/finder.vcproj
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/finder_fs.cc
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/finder_kernel.cc
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/finder_registry.cc
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/main.cc
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/finder/ntundoc.h
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/launcher/launcher.cc
[delete] https://crrev.com/e326a5d409dadb3df6811b3dca7f1cfecb4ef520/sandbox/win/tools/launcher/launcher.vcproj

Project Member

Comment 36 by bugdroid1@chromium.org, Mar 27 2017

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

commit 843ecf04f4af01faed28fdbb92db683944a06918
Author: hayato <hayato@chromium.org>
Date: Mon Mar 27 03:40:17 2017

Revert of Remove sandbox/win/tools. (patchset #1 id:1 of https://codereview.chromium.org/2773333002/ )

Reason for revert:
Compile failed on WinClang64.

https://uberchromegw.corp.google.com/i/chromium.win/builders/WinClang64%20%28dbg%29/builds/11236

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.win%2FWinClang64__dbg_%2F11236%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

[479/12131] CXX obj/sandbox/win/pocdll/handles.obj
FAILED: obj/sandbox/win/pocdll/handles.obj
C:\b\c\goma_client/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes /FC @obj/sandbox/win/pocdll/handles.obj.rsp /c ../../sandbox/win/sandbox_poc/pocdll/handles.cc /Foobj/sandbox/win/pocdll/handles.obj /Fd"obj/sandbox/win/pocdll_cc.pdb"
C:\b\c\b\win_clang\src\sandbox\win\sandbox_poc\pocdll\handles.cc(7,10):  fatal error: 'sandbox/win/tools/finder/ntundoc.h' file not found
#include "sandbox/win/tools/finder/ntundoc.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Original issue's description:
> Remove sandbox/win/tools.
>
> The files therein look unused, haven't been really touched since 2012, aren't
> referenced in any build files, and have some maintenance cost.
>
> BUG=661774
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
>
> Review-Url: https://codereview.chromium.org/2773333002
> Cr-Commit-Position: refs/heads/master@{#459696}
> Committed: https://chromium.googlesource.com/chromium/src/+/6625b2f764ae4efb0637215660500ba721b8a43d

TBR=wfh@chromium.org,thakis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774

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

[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/finder.cc
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/finder.h
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/finder.vcproj
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/finder_fs.cc
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/finder_kernel.cc
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/finder_registry.cc
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/main.cc
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/finder/ntundoc.h
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/launcher/launcher.cc
[add] https://crrev.com/843ecf04f4af01faed28fdbb92db683944a06918/sandbox/win/tools/launcher/launcher.vcproj

Project Member

Comment 38 by bugdroid1@chromium.org, Mar 29 2017

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

commit b663358c89e05334e8cfff0f1f855fe4c72c192a
Author: wychen <wychen@chromium.org>
Date: Wed Mar 29 01:06:58 2017

Add script fix_gn_headers.py to fix GN missing headers

The script is based on CLs authored by thakis@chromium.org:
- https://codereview.chromium.org/2770693003/
- https://codereview.chromium.org/2771373003/

BUG=661774

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

[add] https://crrev.com/b663358c89e05334e8cfff0f1f855fe4c72c192a/build/fix_gn_headers.py

Project Member

Comment 39 by bugdroid1@chromium.org, Mar 31 2017

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

commit 589d05ed666fd48defd3cd338e8e23a2bcc21937
Author: wychen <wychen@chromium.org>
Date: Fri Mar 31 00:06:36 2017

Add missing header files to GN files

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, fed to fix_gn_headers.py, and manually fixed
the results.

Read more about the manual fixes in the Review-Url.

The number of missing header files on linux and android decreased by
154, from 990 to 836.

The following spreadsheet is updated.
https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

fix_gn_headers.py is from https://codereview.chromium.org/2781603003

BUG=661774
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/base/allocator/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/chrome/browser/extensions/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/chrome/browser/media/router/discovery/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/chrome/test/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/components/nacl/renderer/plugin/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/content/browser/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/content/public/browser/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/content/public/renderer/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/content/renderer/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/extensions/browser/api/metrics_private/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/gpu/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/gpu/khronos_glcts_support/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ppapi/c/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ppapi/native_client/src/untrusted/irt_stub/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ppapi/thunk/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/services/ui/public/cpp/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/dom/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/editing/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/events/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/frame/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/html/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/layout/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/loader/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/core/page/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/modules/geolocation/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/android_crazy_linker/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/freetype/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/harfbuzz-ng/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/libaddressinput/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/libpng/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/libxml/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/third_party/snappy/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/tools/gn/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ui/gfx/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ui/gfx/ipc/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ui/gfx/ipc/color/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ui/gfx/ipc/geometry/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ui/gfx/ipc/skia/BUILD.gn
[modify] https://crrev.com/589d05ed666fd48defd3cd338e8e23a2bcc21937/ui/gl/init/BUILD.gn

Cc: tzik@chromium.org
Project Member

Comment 41 by bugdroid1@chromium.org, Apr 1 2017

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

commit eb936d60b77cbc82876bcd23e5ad7fc09a7646b1
Author: wychen <wychen@chromium.org>
Date: Sat Apr 01 05:42:56 2017

Use more accurate matching in fix_gn_headers.py

- Exclude "git grep" matches that already have header files.
- More accurate "git grep" pattern.
- Allow interactive picking which matches to handle.

BUG=661774

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

[modify] https://crrev.com/eb936d60b77cbc82876bcd23e5ad7fc09a7646b1/build/fix_gn_headers.py

Project Member

Comment 42 by bugdroid1@chromium.org, Apr 1 2017

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

commit 36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f
Author: wychen <wychen@chromium.org>
Date: Sat Apr 01 05:50:47 2017

Add missing header files to GN files (2 of N)

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file lists, fed to fix_gn_headers.py (without
AddHeadersToSources), chose which matches to apply interactively, and
manually fixed the results.

Read more about the manual fixes in the Review-Url.

The number of missing header files on linux and android decreased by 7,
from 836 to 829.

None of the fixed files are included in the following spreadsheet:
https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

fix_gn_headers.py is from https://codereview.chromium.org/2790563003

BUG=661774

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

[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/chrome/test/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/components/nacl/renderer/plugin/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/extensions/shell/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/media/cast/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/net/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/remoting/host/win/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/third_party/WebKit/Source/core/events/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/third_party/WebKit/Source/core/frame/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/third_party/leveldatabase/BUILD.gn
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/third_party/openh264/openh264_sources.gni
[modify] https://crrev.com/36575ffb10df9cdee4a9d0fe7d0e34c44323ea8f/third_party/wds/BUILD.gn

Project Member

Comment 43 by bugdroid1@chromium.org, Apr 5 2017

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

commit ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79
Author: wychen <wychen@chromium.org>
Date: Wed Apr 05 07:19:37 2017

Add missing header files to GN files (3 of N)

I ran check_gn_headers.py on Mac and iOS full builds, merged the missing
header file list, fed to fix_gn_headers.py (without AddHeadersToSources),
and manually fixed the results.

Read more about the manual fixes in the Review-Url.

The number of missing header files on Mac and iOS decreased by 54, from
814 to 760.

None of the fixed files are included in the following spreadsheet:
https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

fix_gn_headers.py is from https://codereview.chromium.org/2790563003

BUG=661774

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

[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/components/os_crypt/BUILD.gn
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/content/test/fuzzer/BUILD.gn
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/ios/chrome/browser/tabs/BUILD.gn
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/native_client_sdk/src/libraries/nacl_io/BUILD.gn
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/net/BUILD.gn
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/remoting/base/BUILD.gn
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/third_party/boringssl/BUILD.generated_tests.gni
[modify] https://crrev.com/ebb66ffb22611d7cc1a50711b0c3eb5a609a5a79/third_party/flatbuffers/BUILD.gn

Project Member

Comment 44 by bugdroid1@chromium.org, Apr 25 2017

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

commit 6b609383cf3669b0670b567249ff868ea4b26e43
Author: wychen <wychen@chromium.org>
Date: Tue Apr 25 06:30:14 2017

Optimize check_gn_headers.py for speed

Process the output of 'ninja' live through the pipe rather than waiting
for it to finish.

Before:
20.74user 16.36system 0:24.57elapsed 151%CPU

After:
16.63user 9.24system 0:13.94elapsed 185%CPU

BUG=661774

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

[modify] https://crrev.com/6b609383cf3669b0670b567249ff868ea4b26e43/build/check_gn_headers.py
[modify] https://crrev.com/6b609383cf3669b0670b567249ff868ea4b26e43/build/check_gn_headers_unittest.py

Blockedon: 715131
Project Member

Comment 46 by bugdroid1@chromium.org, Apr 26 2017

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

commit c219b8a1823f884703c3546bf2cb7c17fb737d67
Author: wychen <wychen@chromium.org>
Date: Wed Apr 26 04:09:45 2017

Revert of Optimize check_gn_headers.py for speed (patchset #1 id:1 of https://codereview.chromium.org/2842513003/ )

Reason for revert:
This optimization was buggy and broke the ninja parsing.

Original issue's description:
> Optimize check_gn_headers.py for speed
>
> Process the output of 'ninja' live through the pipe rather than waiting
> for it to finish.
>
> Before:
> 20.74user 16.36system 0:24.57elapsed 151%CPU
>
> After:
> 16.63user 9.24system 0:13.94elapsed 185%CPU
>
> BUG=661774
>
> Review-Url: https://codereview.chromium.org/2842513003
> Cr-Commit-Position: refs/heads/master@{#466900}
> Committed: https://chromium.googlesource.com/chromium/src/+/6b609383cf3669b0670b567249ff868ea4b26e43

TBR=dpranke@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774

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

[modify] https://crrev.com/c219b8a1823f884703c3546bf2cb7c17fb737d67/build/check_gn_headers.py
[modify] https://crrev.com/c219b8a1823f884703c3546bf2cb7c17fb737d67/build/check_gn_headers_unittest.py

Project Member

Comment 47 by bugdroid1@chromium.org, Apr 26 2017

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

commit 1f3923624ee0a8ae0f2ec3136e376ca3a2a5c028
Author: thakis <thakis@chromium.org>
Date: Wed Apr 26 17:45:32 2017

Remove sandbox/win/tools.

The files therein look unused, haven't been really touched since 2012, aren't
referenced in any build files, and have some maintenance cost.

BUG=661774
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
NOTRY=true

Review-Url: https://codereview.chromium.org/2773333002
Cr-Original-Commit-Position: refs/heads/master@{#459696}
Committed: https://chromium.googlesource.com/chromium/src/+/6625b2f764ae4efb0637215660500ba721b8a43d
Review-Url: https://codereview.chromium.org/2773333002
Cr-Commit-Position: refs/heads/master@{#467366}

[modify] https://crrev.com/1f3923624ee0a8ae0f2ec3136e376ca3a2a5c028/sandbox/win/BUILD.gn
[modify] https://crrev.com/1f3923624ee0a8ae0f2ec3136e376ca3a2a5c028/sandbox/win/sandbox_poc/pocdll/handles.cc
[rename] https://crrev.com/1f3923624ee0a8ae0f2ec3136e376ca3a2a5c028/sandbox/win/sandbox_poc/pocdll/ntundoc.h
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/finder.cc
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/finder.h
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/finder.vcproj
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/finder_fs.cc
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/finder_kernel.cc
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/finder_registry.cc
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/finder/main.cc
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/launcher/launcher.cc
[delete] https://crrev.com/7c5e2d42f4618591199976acb8da1e3bc1648095/sandbox/win/tools/launcher/launcher.vcproj

Project Member

Comment 48 by bugdroid1@chromium.org, Apr 26 2017

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

commit fb89a91b264ad2c6bad9acd32aeb42c45b6f1e69
Author: thakis <thakis@chromium.org>
Date: Wed Apr 26 17:49:44 2017

Add missing android_webview .h files to .gn files

BUG=661774

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

[modify] https://crrev.com/fb89a91b264ad2c6bad9acd32aeb42c45b6f1e69/android_webview/BUILD.gn

Project Member

Comment 50 by bugdroid1@chromium.org, Apr 27 2017

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

commit ef74ec9973e071da2dc5813f0b93662a0f1497bf
Author: wychen <wychen@chromium.org>
Date: Thu Apr 27 06:28:25 2017

(reland) Optimize check_gn_headers.py for speed

- Process the output of 'ninja' live through the pipe rather than
  waiting for it to finish.
- Use multiprocessing.

Benchmark results using pypy:
Before:
17.10user 14.38system 0:20.05elapsed 157%CPU (0avgtext+0avgdata 1708036maxresident)k
18.06user 15.63system 0:21.92elapsed 153%CPU (0avgtext+0avgdata 1710912maxresident)k
17.94user 13.72system 0:19.91elapsed 158%CPU (0avgtext+0avgdata 1172128maxresident)k

After:
15.28user 7.11system 0:06.25elapsed 357%CPU (0avgtext+0avgdata 319236maxresident)k
16.03user 7.74system 0:06.84elapsed 347%CPU (0avgtext+0avgdata 319396maxresident)k
15.24user 8.16system 0:06.36elapsed 367%CPU (0avgtext+0avgdata 318272maxresident)k

BUG=661774

Review-Url: https://codereview.chromium.org/2842513003
Cr-Original-Commit-Position: refs/heads/master@{#466900}
Review-Url: https://codereview.chromium.org/2846473002
Cr-Commit-Position: refs/heads/master@{#467604}

[modify] https://crrev.com/ef74ec9973e071da2dc5813f0b93662a0f1497bf/build/check_gn_headers.py
[modify] https://crrev.com/ef74ec9973e071da2dc5813f0b93662a0f1497bf/build/check_gn_headers_unittest.py

Project Member

Comment 51 by bugdroid1@chromium.org, Apr 27 2017

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

commit a8786d03dcf223dd01b3459b5566bfed41beb2ba
Author: thakis <thakis@chromium.org>
Date: Thu Apr 27 17:54:39 2017

Remove unused app_mode_app_tests.cc

It was added as an empty executable https://codereview.chromium.org/9274003/
and never grew to more than that, except that the cc file isn't even
referenced from any .gn file now that the gyp build is gone.

BUG=661774

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

[delete] https://crrev.com/f9f2d84e6e7e578e3b7f248184d6564d1f92c43f/chrome/test/base/app_mode_app_tests.cc

Project Member

Comment 52 by bugdroid1@chromium.org, Apr 27 2017

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

commit 7a2e1cc92b74ecd7dff00e5cc91e7f21f97dc7cd
Author: thakis <thakis@chromium.org>
Date: Thu Apr 27 21:24:54 2017

Re-add OfflinePageModelQueryTests to build files.

It got lost in https://codereview.chromium.org/2489443002/
(see changes to components/offline_pages/BUILD.gn and
components/offline_pages/core/BUILD.gn in that change).

BUG=654635,661774

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

[modify] https://crrev.com/7a2e1cc92b74ecd7dff00e5cc91e7f21f97dc7cd/components/offline_pages/core/BUILD.gn
[modify] https://crrev.com/7a2e1cc92b74ecd7dff00e5cc91e7f21f97dc7cd/components/offline_pages/core/offline_page_model_query_unittest.cc

Project Member

Comment 53 by bugdroid1@chromium.org, Apr 28 2017

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

commit 5523578be7767b76e2eff354c98a82dcd8f653b5
Author: wychen <wychen@chromium.org>
Date: Fri Apr 28 01:59:15 2017

Handle |public| in check_gn_headers.py

If |public| of a target contains files, count them as recognized
headers as well.

BUG=661774

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

[modify] https://crrev.com/5523578be7767b76e2eff354c98a82dcd8f653b5/build/check_gn_headers.py
[modify] https://crrev.com/5523578be7767b76e2eff354c98a82dcd8f653b5/build/check_gn_headers_unittest.py

Project Member

Comment 54 by bugdroid1@chromium.org, Apr 28 2017

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

commit 5dad32e367d077bf8b84f1700a6cd00e74a20cc1
Author: thakis <thakis@chromium.org>
Date: Fri Apr 28 02:14:22 2017

Add missing .h files to chrome/common/BUILD.gn

BUG=661774

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

[modify] https://crrev.com/5dad32e367d077bf8b84f1700a6cd00e74a20cc1/chrome/common/BUILD.gn

Project Member

Comment 55 by bugdroid1@chromium.org, Apr 28 2017

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

commit 992dc1adae8123ed7cacdeaa7f321bbbbf7837f5
Author: mnissler <mnissler@chromium.org>
Date: Fri Apr 28 14:11:52 2017

Fix and re-enable DeviceSettingsCacheTest.

The device_settings_cache_unittest.cc file was accidentally dropped
from the build a long time ago.

BUG= chromium:139126 ,chromium:661774
TEST=DeviceSettingsCacheTest* passes.

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

[modify] https://crrev.com/992dc1adae8123ed7cacdeaa7f321bbbbf7837f5/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/992dc1adae8123ed7cacdeaa7f321bbbbf7837f5/chrome/browser/chromeos/settings/device_settings_cache_unittest.cc

Project Member

Comment 56 by bugdroid1@chromium.org, Apr 29 2017

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

commit fd60d8ca4a34a9cacfa3f45d65598165543fa74e
Author: wychen <wychen@chromium.org>
Date: Sat Apr 29 00:34:00 2017

Add missing header files to GN files (4 of N)

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file lists, fed to fix_gn_headers.py (without
AddHeadersToSources), chose which matches to apply interactively, and
manually fixed the results.

Read more about the manual fixes in the Review-Url.

The number of missing header files on linux and android decreased by 9,
from 840 to 831.

None of the fixed files are included in the following spreadsheet:
https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

fix_gn_headers.py is from https://codereview.chromium.org/2790563003

BUG=661774

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

[modify] https://crrev.com/fd60d8ca4a34a9cacfa3f45d65598165543fa74e/content/test/BUILD.gn
[modify] https://crrev.com/fd60d8ca4a34a9cacfa3f45d65598165543fa74e/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/fd60d8ca4a34a9cacfa3f45d65598165543fa74e/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/fd60d8ca4a34a9cacfa3f45d65598165543fa74e/ui/gfx/BUILD.gn

Project Member

Comment 57 by bugdroid1@chromium.org, May 1 2017

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

commit a10837ebfdd79144b6644f33dae60fc66059b760
Author: wychen <wychen@chromium.org>
Date: Mon May 01 16:10:16 2017

Fix missing headers in brotli

Decreased the number of missing header files by 4.

BUG=661774

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

[modify] https://crrev.com/a10837ebfdd79144b6644f33dae60fc66059b760/.gn
[modify] https://crrev.com/a10837ebfdd79144b6644f33dae60fc66059b760/third_party/brotli/BUILD.gn

Project Member

Comment 58 by bugdroid1@chromium.org, May 2 2017

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

commit 9d04cf08c6f0850dc580ae9416365909768c011a
Author: wychen <wychen@chromium.org>
Date: Tue May 02 00:10:05 2017

Fix missing headers in chrome/browser/task_manager/providers/web_contents

Decreased the number of missing header files by 18.

BUG=661774

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

[modify] https://crrev.com/9d04cf08c6f0850dc580ae9416365909768c011a/chrome/browser/task_manager/web_contents_tags.cc

Project Member

Comment 59 by bugdroid1@chromium.org, May 2 2017

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

commit e4e376d3183be96ccb95b1e0fd8f4d648a54775b
Author: wychen <wychen@chromium.org>
Date: Tue May 02 00:46:07 2017

Fix missing headers about vpn service

Decreased the number of missing header files by 1.

BUG=661774

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

[modify] https://crrev.com/e4e376d3183be96ccb95b1e0fd8f4d648a54775b/extensions/browser/browser_context_keyed_service_factories.cc

Project Member

Comment 61 by bugdroid1@chromium.org, May 3 2017

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

commit 2f8d9b469f243f117f37ec929920f33bf61eb40a
Author: wychen <wychen@chromium.org>
Date: Wed May 03 01:06:07 2017

Only include base/win/windows_version.h on Windows build

I ran check_gn_headers.py on linux and android full builds and
manually fixed it.

BUG=661774

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

[modify] https://crrev.com/2f8d9b469f243f117f37ec929920f33bf61eb40a/base/allocator/partition_allocator/address_space_randomization.cc

Project Member

Comment 63 by bugdroid1@chromium.org, May 8 2017

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

commit 6c067d4597d22586b8d51ef59ed8916378101239
Author: tfarina <tfarina@chromium.org>
Date: Mon May 08 20:30:27 2017

content: add missing entries to common's BUILD.gn file

These were header files that should have been listed in the sources list, but weren't.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774
R=avi@chromium.org

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

[modify] https://crrev.com/6c067d4597d22586b8d51ef59ed8916378101239/content/common/BUILD.gn

Project Member

Comment 64 by bugdroid1@chromium.org, May 10 2017

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

commit e120c2fd1e0c07b721caddef640aabf835d75840
Author: wychen <wychen@chromium.org>
Date: Wed May 10 07:37:07 2017

Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774, 720159

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

[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/ash/shell.h
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/certificate_manager_model.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/cryptauth/chrome_cryptauth_service.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/diagnostics/diagnostics_controller_unittest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/diagnostics/sqlite_diagnostics.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/page_capture/page_capture_apitest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/settings_private/settings_private_event_router.h
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/extensions/api/web_request/web_request_permissions_unittest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/net/predictor_tab_helper.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/profiles/profile_manager_unittest.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/ssl/security_state_tab_helper.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/chrome/browser/ui/input_method/input_method_engine_base.h
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/e120c2fd1e0c07b721caddef640aabf835d75840/extensions/browser/api/web_request/web_request_permissions.cc

Project Member

Comment 65 by bugdroid1@chromium.org, May 10 2017

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

commit 2be088539a91154a02f4fecb27f03778cca772e1
Author: tfarina <tfarina@chromium.org>
Date: Wed May 10 21:05:53 2017

base: remove FreeBSD port of platform_thread.h

It's unused and not referenced from any .gn files, they can keep
this file as downstream diff or or upstream their diff to us.

BUG=661774
R=thakis@chromium.org

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

[delete] https://crrev.com/5fa8d94b746048527041ad5d0e8a4c11f90cb597/base/threading/platform_thread_freebsd.cc

Project Member

Comment 66 by bugdroid1@chromium.org, May 10 2017

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

commit c75e1fb9d386f28b9ec500c75181833fd0398b1f
Author: tfarina <tfarina@chromium.org>
Date: Wed May 10 21:21:13 2017

printing: add native_drawing_context.h to printing sources list

Headers files should also be listed in the sources list, but I missed this when I moved it from skia/.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774
R=thestig@chromium.org

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

[modify] https://crrev.com/c75e1fb9d386f28b9ec500c75181833fd0398b1f/printing/BUILD.gn

Project Member

Comment 67 by bugdroid1@chromium.org, May 12 2017

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

commit 83d0a339b738a5d4a42673d8b763f4745b05a8f6
Author: tfarina <tfarina@chromium.org>
Date: Fri May 12 01:51:30 2017

base: remove icu_util_nacl_win64.cc

No .gn files reference this file anymore, and base_nacl_win64 target
which used to reference it does not exist anymore as well.

BUG=661774
R=thakis@chromium.org

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

[delete] https://crrev.com/e8ce0ec0131153014f66ef73920527c25c0589d8/base/i18n/icu_util_nacl_win64.cc

Project Member

Comment 68 by bugdroid1@chromium.org, May 12 2017

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

commit 17e05472ffb707c0b92cd3fb69e92434f1993578
Author: Thiago Farina <tfarina@chromium.org>
Date: Fri May 12 16:41:29 2017

ui/gfx: add some missing header files to BUILD.gn file

Header files should be listed in the sources list, so gn analyze
can work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774
R=thakis@chromium.org

Change-Id: I4760a62994f5e90f881c0eacda1e8dd26329de58
Reviewed-on: https://chromium-review.googlesource.com/504647
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#471332}
[modify] https://crrev.com/17e05472ffb707c0b92cd3fb69e92434f1993578/ui/gfx/BUILD.gn

Project Member

Comment 69 by bugdroid1@chromium.org, May 16 2017

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

commit 1b5779004b4e7dead4c062cd66c7908d378e86d8
Author: Thiago Farina <tfarina@chromium.org>
Date: Tue May 16 13:12:17 2017

ui/app_list: add some missing header files to BUILD.gn file

Header files should be listed in the sources list, so gn analyze
can work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774

Change-Id: Iadda7cb54bb8358d33710a22a422ea0102b4dc04
Reviewed-on: https://chromium-review.googlesource.com/506151
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472073}
[modify] https://crrev.com/1b5779004b4e7dead4c062cd66c7908d378e86d8/ui/app_list/BUILD.gn

Project Member

Comment 70 by bugdroid1@chromium.org, May 17 2017

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

commit 90e87f4bb986def04af8f8310cd495bfbead49bb
Author: wychen <wychen@chromium.org>
Date: Wed May 17 06:50:31 2017

Fix a typo in the header filename in media/base/BUILD.gn

The typo was introduced in https://codereview.chromium.org/2856253004.

BUG=661774,  716359 

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

[modify] https://crrev.com/90e87f4bb986def04af8f8310cd495bfbead49bb/media/base/BUILD.gn

Project Member

Comment 71 by bugdroid1@chromium.org, May 17 2017

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

commit 5439f5e5f020c0e7e9ccf59693882f1fc2d92917
Author: Thiago Farina <tfarina@chromium.org>
Date: Wed May 17 17:17:42 2017

content: remove run_gl_benchmark.cc

It was added by commit 0d371f537e79: ("Remove unittest related content
dependences from content/common/gpu"), and was added to the
content_gl_benchmark target.

Then it was removed from content/test/BUILD.gn by commit
540b8c11b041: ("Introduce components/display_compositor"),
without adding it to another build file.

Source and header files should be listed in the sources list,
in order to "gn analyze" work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774

Change-Id: Ib1e1f97679b28e0ea428412cb13a85f21be9aed2
Reviewed-on: https://chromium-review.googlesource.com/507728
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472483}
[delete] https://crrev.com/f7452d2eaafcf78e874f5f2eda336228e45f8d1e/content/test/run_gl_benchmark.cc

Project Member

Comment 72 by bugdroid1@chromium.org, May 18 2017

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

commit 10ec8a62efd2843f18facd2dfe29bc31b874c2cc
Author: Thiago Farina <tfarina@chromium.org>
Date: Wed May 17 23:59:48 2017

sandbox: add sandbox_export target

This target holds the sandbox_export.h header file, as header files should
be listed in the sources list in some way or another, so gn analyze can work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774

Change-Id: If50b21091c460e26cc6e827a3e8c12fdc8d78b3d
Reviewed-on: https://chromium-review.googlesource.com/506131
Reviewed-by: Justin Schuh <jschuh@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#472608}
[modify] https://crrev.com/10ec8a62efd2843f18facd2dfe29bc31b874c2cc/sandbox/BUILD.gn
[modify] https://crrev.com/10ec8a62efd2843f18facd2dfe29bc31b874c2cc/sandbox/linux/BUILD.gn
[modify] https://crrev.com/10ec8a62efd2843f18facd2dfe29bc31b874c2cc/sandbox/mac/BUILD.gn

Project Member

Comment 73 by bugdroid1@chromium.org, May 23 2017

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

commit f010d72f5d621e78c0a0a4ae6ba3ad22f97a920b
Author: Thiago Farina <tfarina@chromium.org>
Date: Tue May 23 02:52:39 2017

ui/views: add some missing header files to BUILD.gn file

Header files should be listed in the sources list, so gn analyze
can work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774

Change-Id: I46e72232a525833bd805a306a4a1e2075f78adef
Reviewed-on: https://chromium-review.googlesource.com/508669
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#473791}
[modify] https://crrev.com/f010d72f5d621e78c0a0a4ae6ba3ad22f97a920b/ui/views/BUILD.gn
[modify] https://crrev.com/f010d72f5d621e78c0a0a4ae6ba3ad22f97a920b/ui/views/mus/BUILD.gn

Project Member

Comment 74 by bugdroid1@chromium.org, May 24 2017

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

commit ded5e27012a9b0d50de4b80a16e024fe5106b160
Author: wychen <wychen@chromium.org>
Date: Wed May 24 08:57:12 2017

Check missing headers in the build on CQ

This script is run on the following bots on CQ:
- linux_chromium_dbg_ng
- linux_chromium_rel_ng

Note that this script can only be added to bots that have "all"
in "additional_compile_targets".

BUG=661774

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

[modify] https://crrev.com/ded5e27012a9b0d50de4b80a16e024fe5106b160/build/check_gn_headers.py
[add] https://crrev.com/ded5e27012a9b0d50de4b80a16e024fe5106b160/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/ded5e27012a9b0d50de4b80a16e024fe5106b160/testing/buildbot/chromium.linux.json
[add] https://crrev.com/ded5e27012a9b0d50de4b80a16e024fe5106b160/testing/scripts/check_gn_headers.py

Project Member

Comment 75 by bugdroid1@chromium.org, May 24 2017

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

commit 5eba554ee38b7a6f78f54f1826fbd279f948c982
Author: vitaliii <vitaliii@chromium.org>
Date: Wed May 24 12:04:00 2017

Revert of Check missing headers in the build on CQ (patchset #8 id:160001 of https://codereview.chromium.org/2628543003/ )

Reason for revert:
This CL likely breaks |check_gn_headers| step. See  crbug.com/725877  for details.

Original issue's description:
> Check missing headers in the build on CQ
>
> This script is run on the following bots on CQ:
> - linux_chromium_dbg_ng
> - linux_chromium_rel_ng
>
> Note that this script can only be added to bots that have "all"
> in "additional_compile_targets".
>
> BUG=661774
>
> Review-Url: https://codereview.chromium.org/2628543003
> Cr-Commit-Position: refs/heads/master@{#474218}
> Committed: https://chromium.googlesource.com/chromium/src/+/ded5e27012a9b0d50de4b80a16e024fe5106b160

TBR=dpranke@chromium.org,thakis@chromium.org,wychen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774, 725877 

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

[modify] https://crrev.com/5eba554ee38b7a6f78f54f1826fbd279f948c982/build/check_gn_headers.py
[delete] https://crrev.com/de1925cd7cf533069b328ce5b8e4ec3863650364/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/5eba554ee38b7a6f78f54f1826fbd279f948c982/testing/buildbot/chromium.linux.json
[delete] https://crrev.com/de1925cd7cf533069b328ce5b8e4ec3863650364/testing/scripts/check_gn_headers.py

Project Member

Comment 76 by bugdroid1@chromium.org, May 25 2017

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

commit 2f5a1ee159209aace78614d6012c361de5c3d2c3
Author: wychen <wychen@chromium.org>
Date: Thu May 25 05:45:30 2017

(reland) Check missing headers in the build on CQ

This script is run on the following bots on CQ:
- linux_chromium_dbg_ng
- linux_chromium_rel_ng

Note that this script can only be added to bots that have "all"
in "additional_compile_targets".

BUG=661774,  725877 

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

[modify] https://crrev.com/2f5a1ee159209aace78614d6012c361de5c3d2c3/build/check_gn_headers.py
[add] https://crrev.com/2f5a1ee159209aace78614d6012c361de5c3d2c3/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/2f5a1ee159209aace78614d6012c361de5c3d2c3/testing/buildbot/chromium.linux.json
[add] https://crrev.com/2f5a1ee159209aace78614d6012c361de5c3d2c3/testing/scripts/check_gn_headers.py

Project Member

Comment 77 by bugdroid1@chromium.org, May 25 2017

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

commit 0ceaba5ef538ce1780578d5e948641bc27f5829b
Author: wychen <wychen@chromium.org>
Date: Thu May 25 06:46:11 2017

Add helpful error messages to check_gn_headers.py

Before running check_gn_headers.py, it is necessary to build all
in order to get the dependency info from the compiler. Directly
checking whether rebuilding is needed by running a build dry run
takes too long, so some heuristics are used instead.

BUG=661774

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

[modify] https://crrev.com/0ceaba5ef538ce1780578d5e948641bc27f5829b/build/check_gn_headers.py
[modify] https://crrev.com/0ceaba5ef538ce1780578d5e948641bc27f5829b/build/check_gn_headers_unittest.py

Project Member

Comment 78 by bugdroid1@chromium.org, May 25 2017

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

commit d3c2681426db53550547f2bc115c3f72ba40f276
Author: mahmadi <mahmadi@chromium.org>
Date: Thu May 25 14:52:02 2017

Revert of Add helpful error messages to check_gn_headers.py (patchset #3 id:40001 of https://codereview.chromium.org/2891363004/ )

Reason for revert:
This CL is likely causing massive failures:

https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder/builds/84473
to
https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder/builds/84498

and

https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder%20%28dbg%29/builds/111528
to
https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder%20%28dbg%29/builds/111542

Original issue's description:
> Add helpful error messages to check_gn_headers.py
>
> Before running check_gn_headers.py, it is necessary to build all
> in order to get the dependency info from the compiler. Directly
> checking whether rebuilding is needed by running a build dry run
> takes too long, so some heuristics are used instead.
>
> BUG=661774
>
> Review-Url: https://codereview.chromium.org/2891363004
> Cr-Commit-Position: refs/heads/master@{#474588}
> Committed: https://chromium.googlesource.com/chromium/src/+/0ceaba5ef538ce1780578d5e948641bc27f5829b

TBR=dpranke@chromium.org,tfarina@chromium.org,wychen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774

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

[modify] https://crrev.com/d3c2681426db53550547f2bc115c3f72ba40f276/build/check_gn_headers.py
[modify] https://crrev.com/d3c2681426db53550547f2bc115c3f72ba40f276/build/check_gn_headers_unittest.py

Project Member

Comment 79 by bugdroid1@chromium.org, May 25 2017

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

commit f9662f54a78b4eaaa0126c9726d2b67328488bef
Author: wychen <wychen@chromium.org>
Date: Thu May 25 17:43:10 2017

Revert of (reland) Check missing headers in the build on CQ (patchset #3 id:40001 of https://codereview.chromium.org/2903733004/ )

Reason for revert:
Hit assertion in bots.  crbug.com/725877 

Original issue's description:
> (reland) Check missing headers in the build on CQ
>
> This script is run on the following bots on CQ:
> - linux_chromium_dbg_ng
> - linux_chromium_rel_ng
>
> Note that this script can only be added to bots that have "all"
> in "additional_compile_targets".
>
> BUG=661774,  725877 
>
> Review-Url: https://codereview.chromium.org/2903733004
> Cr-Commit-Position: refs/heads/master@{#474570}
> Committed: https://chromium.googlesource.com/chromium/src/+/2f5a1ee159209aace78614d6012c361de5c3d2c3

TBR=dpranke@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774,  725877 

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

[modify] https://crrev.com/f9662f54a78b4eaaa0126c9726d2b67328488bef/build/check_gn_headers.py
[delete] https://crrev.com/a98ce9b5af5aaeb2e4998663610afa34f157b9e3/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/f9662f54a78b4eaaa0126c9726d2b67328488bef/testing/buildbot/chromium.linux.json
[delete] https://crrev.com/a98ce9b5af5aaeb2e4998663610afa34f157b9e3/testing/scripts/check_gn_headers.py

Project Member

Comment 80 by bugdroid1@chromium.org, May 25 2017

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

commit 0362911661559a7edca7d4ca9c36f1091a527b04
Author: wychen <wychen@chromium.org>
Date: Thu May 25 20:37:44 2017

Reland of Add helpful error messages to check_gn_headers.py (patchset #1 id:1 of https://codereview.chromium.org/2905863002/ )

Reason for revert:
The real culprit is https://crrev.com/2903733004/, which was reverted.
This CL (https://crrev.com/2891363004) made it fail more gracefully, and should be kept.

Original issue's description:
> Revert of Add helpful error messages to check_gn_headers.py (patchset #3 id:40001 of https://codereview.chromium.org/2891363004/ )
>
> Reason for revert:
> This CL is likely causing massive failures:
>
> https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder/builds/84473
> to
> https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder/builds/84498
>
> and
>
> https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder%20%28dbg%29/builds/111528
> to
> https://build.chromium.org/p/chromium.linux/builders/Linux%20Builder%20%28dbg%29/builds/111542
>
> Original issue's description:
> > Add helpful error messages to check_gn_headers.py
> >
> > Before running check_gn_headers.py, it is necessary to build all
> > in order to get the dependency info from the compiler. Directly
> > checking whether rebuilding is needed by running a build dry run
> > takes too long, so some heuristics are used instead.
> >
> > BUG=661774
> >
> > Review-Url: https://codereview.chromium.org/2891363004
> > Cr-Commit-Position: refs/heads/master@{#474588}
> > Committed: https://chromium.googlesource.com/chromium/src/+/0ceaba5ef538ce1780578d5e948641bc27f5829b
>
> TBR=dpranke@chromium.org,tfarina@chromium.org,wychen@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=661774
>
> Review-Url: https://codereview.chromium.org/2905863002
> Cr-Commit-Position: refs/heads/master@{#474649}
> Committed: https://chromium.googlesource.com/chromium/src/+/d3c2681426db53550547f2bc115c3f72ba40f276

TBR=dpranke@chromium.org,tfarina@chromium.org,mahmadi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=661774

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

[modify] https://crrev.com/0362911661559a7edca7d4ca9c36f1091a527b04/build/check_gn_headers.py
[modify] https://crrev.com/0362911661559a7edca7d4ca9c36f1091a527b04/build/check_gn_headers_unittest.py

Project Member

Comment 81 by bugdroid1@chromium.org, May 26 2017

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

commit 09692cdc03a2723afadf7109b2b8404a9eb497ed
Author: wychen <wychen@chromium.org>
Date: Fri May 26 01:57:16 2017

Handle exceptions in check_gn_headers.py

If exceptions are thrown, the main process would wait forever if they
are not handled. This could potentially cause timeout on the bots.

BUG=661774

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

[modify] https://crrev.com/09692cdc03a2723afadf7109b2b8404a9eb497ed/build/check_gn_headers.py

Project Member

Comment 82 by bugdroid1@chromium.org, Jun 5 2017

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

commit f7d9e91e64165b26cbfd95d29c33d4d6f0b46126
Author: wychen <wychen@chromium.org>
Date: Mon Jun 05 19:35:17 2017

Add scripts for running GN missing header checker on CQ

This relands part of crrev.com/2903733004, but not enabled.

The enabling config would be landed in a separate CL.

BUG=661774,  725877 

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

[add] https://crrev.com/f7d9e91e64165b26cbfd95d29c33d4d6f0b46126/build/check_gn_headers_whitelist.txt
[add] https://crrev.com/f7d9e91e64165b26cbfd95d29c33d4d6f0b46126/testing/scripts/check_gn_headers.py

Project Member

Comment 83 by bugdroid1@chromium.org, Jun 5 2017

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

commit 00f2b7ed28e3d1d864943497d6aa865ef1cde5e2
Author: wychen <wychen@chromium.org>
Date: Mon Jun 05 20:00:39 2017

Enable checking GN missing headers on CQ and waterfall

This relands the rest of crrev.com/2903733004.

Step "check_gn_headers" is enabled on the following bots on CQ:
- linux_chromium_dbg_ng
- linux_chromium_rel_ng

And also on the following builders on waterfall:
- Linux Builder
- Linux Builder (dbg)

Note that this script can only be added to bots that have "all"
in "additional_compile_targets".

BUG=661774,  725877 

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

[modify] https://crrev.com/00f2b7ed28e3d1d864943497d6aa865ef1cde5e2/testing/buildbot/chromium.linux.json

Project Member

Comment 84 by bugdroid1@chromium.org, Jun 6 2017

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

commit 1ef74e6c994ef0e20184f79fdf0372ff756e13fd
Author: wychen <wychen@chromium.org>
Date: Tue Jun 06 18:02:15 2017

Anyone can edit check_gn_headers_whitelist.txt

BUG=661774

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

[modify] https://crrev.com/1ef74e6c994ef0e20184f79fdf0372ff756e13fd/build/OWNERS
[modify] https://crrev.com/1ef74e6c994ef0e20184f79fdf0372ff756e13fd/build/check_gn_headers_whitelist.txt

Project Member

Comment 85 by bugdroid1@chromium.org, Jun 6 2017

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

commit 1ef74e6c994ef0e20184f79fdf0372ff756e13fd
Author: wychen <wychen@chromium.org>
Date: Tue Jun 06 18:02:15 2017

Anyone can edit check_gn_headers_whitelist.txt

BUG=661774

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

[modify] https://crrev.com/1ef74e6c994ef0e20184f79fdf0372ff756e13fd/build/OWNERS
[modify] https://crrev.com/1ef74e6c994ef0e20184f79fdf0372ff756e13fd/build/check_gn_headers_whitelist.txt

Project Member

Comment 86 by bugdroid1@chromium.org, Jun 6 2017

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

commit bc29797ee3f133d33e0505901ceaee0202b2c1ad
Author: wychen <wychen@chromium.org>
Date: Tue Jun 06 19:16:07 2017

Force building when check_gn_headers is changed

BUG=661774

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

[modify] https://crrev.com/bc29797ee3f133d33e0505901ceaee0202b2c1ad/testing/buildbot/trybot_analyze_config.json

Project Member

Comment 87 by bugdroid1@chromium.org, Jun 12 2017

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

commit 6cdbe5061c8a26d0b9647d13c1d89e9d152491db
Author: wychen <wychen@chromium.org>
Date: Mon Jun 12 22:52:46 2017

Add CQ_INCLUDE_TRYBOTS for GN header checkers

BUG=661774

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

[add] https://crrev.com/6cdbe5061c8a26d0b9647d13c1d89e9d152491db/build/PRESUBMIT.py

Output of:
https://codereview.chromium.org/2932153002/

Here's the list of headers affecting more than 100 object files:

5023 cc/cc_export.h
4723 third_party/WebKit/public/platform/WebFeaturePolicyFeature.h
4565 net/cert/cert_type.h
4112 gpu/gpu_export.h
4100 third_party/WebKit/Source/platform/text/TextDirection.h
3895 third_party/WebKit/Source/platform/graphics/paint/PaintImage.h
3885 third_party/WebKit/Source/platform/fonts/Glyph.h
3884 third_party/WebKit/Source/platform/text/TabSize.h
3856 third_party/WebKit/public/platform/WebFeature.h
3827 third_party/WebKit/Source/platform/fonts/FontSelector.h
3766 third_party/WebKit/Source/core/css/CSSPropertyMetadata.h
3752 third_party/WebKit/Source/platform/transforms/TransformOperation.h
3737 third_party/WebKit/Source/core/style/ShapeValue.h
3737 third_party/WebKit/Source/core/style/TransformOrigin.h
3408 cc/input/scrollbar.h
3406 cc/input/event_listener_properties.h
2700 mojo/edk/system/system_impl_export.h
2687 ui/accessibility/ax_export.h
2392 third_party/WebKit/Source/core/dom/ArrayBufferViewHelpers.h
2351 ui/events/keycodes/keyboard_codes_posix.h
1217 components/sessions/core/sessions_export.h
1211 ui/base/dragdrop/download_file_interface.h
1098 ui/shell_dialogs/shell_dialogs_export.h
961 third_party/khronos/KHR/khrplatform.h
944 ppapi/shared_impl/ppapi_shared_export.h
862 ui/gl/gpu_preference.h
807 gpu/command_buffer/common/gles2_cmd_utils_autogen.h
804 ui/aura/aura_export.h
799 ppapi/cpp/pass_ref.h
785 extensions/browser/extension_event_histogram_value.h
780 net/quic/core/stream_notifier_interface.h
752 third_party/khronos/GLES2/gl2.h
752 gpu/GLES2/gl2chromium.h
752 third_party/khronos/GLES2/gl2platform.h
752 gpu/GLES2/gl2chromium_autogen.h
715 ppapi/shared_impl/api_id.h
714 extensions/browser/extension_function_histogram_value.h
662 ui/native_theme/native_theme_export.h
648 third_party/WebKit/public/platform/WebSourceLocation.h
558 third_party/khronos/GLES2/gl2ext.h
516 ui/gfx/swap_result.h
508 third_party/WebKit/public/platform/WebTouchInfo.h
502 ui/gfx/overlay_transform.h
486 ppapi/shared_impl/singleton_resource_id.h
471 third_party/WebKit/Source/core/css/resolver/StyleBuilder.h
469 ppapi/proxy/ppapi_proxy_export.h
434 net/http/http_status_code_list.h
364 ppapi/proxy/plugin_resource_callback.h
364 ppapi/proxy/dispatch_reply_message.h
338 ppapi/shared_impl/dir_contents.h
294 third_party/khronos/GLES3/gl3.h
294 third_party/khronos/GLES3/gl3platform.h
282 components/sync/engine/connection_status.h
281 gpu/command_buffer/common/gpu_memory_allocation.h
259 gpu/command_buffer/client/gles2_interface_autogen.h
253 ui/gl/gpu_switching_observer.h
232 media/audio/audio_logging.h
229 cc/resources/release_callback_impl.h
228 cc/resources/return_callback.h
197 third_party/libvpx/source/config/linux/x64/vpx_config.h
186 third_party/khronos/EGL/egl.h
186 third_party/khronos/EGL/eglplatform.h
177 cc/input/browser_controls_state.h
170 third_party/khronos/EGL/eglext.h
168 mojo/common/mojo_common_export.h
164 cc/layers/performance_properties.h
153 third_party/libvpx/source/config/nacl/vpx_config.h
144 cc/base/ring_buffer.h
137 components/sync/engine/net/network_time_update_callback.h
127 third_party/WebKit/Source/platform/scheduler/base/task_queue.h
126 gpu/command_buffer/client/context_support.h
124 third_party/libaddressinput/chromium/override/basictypes_override.h
122 tools/gn/ordered_set.h
111 gpu/command_buffer/common/command_buffer_shared.h
106 gpu/GLES2/gl2extchromium.h

Project Member

Comment 91 by bugdroid1@chromium.org, Jun 13 2017

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

commit a11f199d1739e18b41f5170e043644158ac27a95
Author: James Zern <jzern@google.com>
Date: Tue Jun 13 01:07:43 2017

libwebp,BUILD.gn: add missing mux headers

BUG=661774

Change-Id: I5aab11ec5a5e8070d8e6f269f0a02e65a3b1f1ac
Reviewed-on: https://chromium-review.googlesource.com/531741
Reviewed-by: Urvang Joshi <urvang@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: James Zern <jzern@google.com>
Cr-Commit-Position: refs/heads/master@{#478850}
[modify] https://crrev.com/a11f199d1739e18b41f5170e043644158ac27a95/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/a11f199d1739e18b41f5170e043644158ac27a95/third_party/libwebp/BUILD.gn

Project Member

Comment 92 by bugdroid1@chromium.org, Jun 13 2017

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

commit 8cc3123c31e0538f0f10be6329683d9c698fbfed
Author: wychen <wychen@chromium.org>
Date: Tue Jun 13 10:21:23 2017

Print detailed info in check_gn_headers.py

When --verbose is given, print more detailed info to help debugging.
The dependency of offending header files and the obj files they affect
should help determine which files to fix. The number of affected
object files can be a proxy of seriousness.

If the whitelist is not given, the extra detail would be very long.

BUG=661774
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_dbg_ng

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

[modify] https://crrev.com/8cc3123c31e0538f0f10be6329683d9c698fbfed/build/check_gn_headers.py
[modify] https://crrev.com/8cc3123c31e0538f0f10be6329683d9c698fbfed/build/check_gn_headers_unittest.py
[modify] https://crrev.com/8cc3123c31e0538f0f10be6329683d9c698fbfed/testing/scripts/check_gn_headers.py

Project Member

Comment 93 by bugdroid1@chromium.org, Jun 15 2017

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

commit b8abad0d6876d494af4f39c76bcfdb86eef15f97
Author: Thiago Farina <tfarina@chromium.org>
Date: Thu Jun 15 07:11:58 2017

ui/accessibility: list ax_export.h in the sources list of accessibility target

Header files should be listed in the sources list, so gn analyze
can work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_dbg_ng
Change-Id: I16b91525512825da0663330d52b3c0b55ff35bdc
Reviewed-on: https://chromium-review.googlesource.com/534513
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479631}
[modify] https://crrev.com/b8abad0d6876d494af4f39c76bcfdb86eef15f97/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/b8abad0d6876d494af4f39c76bcfdb86eef15f97/ui/accessibility/BUILD.gn

Project Member

Comment 94 by bugdroid1@chromium.org, Jun 15 2017

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

commit 6f572e6775095f973220e5ab1c53a340747fbb2d
Author: Thiago Farina <tfarina@chromium.org>
Date: Thu Jun 15 22:50:32 2017

ui/native_theme: add native_theme_export.h to the sources list of native_theme target

Header files should be listed in the sources list in order to gn analyze work.

https://docs.google.com/spreadsheets/d/15az3FMl-jAS0mx4E9XVSBVHVpmEzo-9EAGY0ywe7bZs/edit#gid=0

https://groups.google.com/a/chromium.org/d/topic/chromium-dev/1kK45mVemBc/discussion

BUG=661774

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_dbg_ng
Change-Id: I1485b03add2f62ae1858d63eb11a7f22d9022ffa
Reviewed-on: https://chromium-review.googlesource.com/536993
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479879}
[modify] https://crrev.com/6f572e6775095f973220e5ab1c53a340747fbb2d/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/6f572e6775095f973220e5ab1c53a340747fbb2d/ui/native_theme/BUILD.gn

Project Member

Comment 95 by bugdroid1@chromium.org, Jun 16 2017

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

commit d8b1ec3835eee2daddcae4c292f9a87683c20001
Author: wychen <wychen@chromium.org>
Date: Fri Jun 16 23:33:16 2017

Revert of Enable checking GN missing headers on CQ and waterfall (patchset #3 id:40001 of https://codereview.chromium.org/2910603002/ )

Reason for revert:
The false negative rate of step check_gn_headers on CQ is higher than expected. Disable it to ease waterfall breakage.

BUG=733500

Original issue's description:
> Enable checking GN missing headers on CQ and waterfall
>
> This relands the rest of crrev.com/2903733004.
>
> Step "check_gn_headers" is enabled on the following bots on CQ:
> - linux_chromium_dbg_ng
> - linux_chromium_rel_ng
>
> And also on the following builders on waterfall:
> - Linux Builder
> - Linux Builder (dbg)
>
> Note that this script can only be added to bots that have "all"
> in "additional_compile_targets".
>
> BUG=661774,  725877 
>
> Review-Url: https://codereview.chromium.org/2910603002
> Cr-Commit-Position: refs/heads/master@{#477058}
> Committed: https://chromium.googlesource.com/chromium/src/+/00f2b7ed28e3d1d864943497d6aa865ef1cde5e2

TBR=dpranke@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=661774,  725877 

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

[modify] https://crrev.com/d8b1ec3835eee2daddcae4c292f9a87683c20001/testing/buildbot/chromium.linux.json

Project Member

Comment 96 by bugdroid1@chromium.org, Jun 27 2017

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

commit 6f4153b1af72a7fa468028e202995e1f823a7a24
Author: Thiago Farina <tfarina@chromium.org>
Date: Tue Jun 27 04:22:49 2017

ui/views: remove test_ink_drop_delegate.*

Nothing uses it anymore and it is not even being built.

BUG=661774

Change-Id: Ic474b3f1066a9d656f12f30b04ea029f8244d770
Reviewed-on: https://chromium-review.googlesource.com/546376
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thiago Farina <tfarina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482536}
[delete] https://crrev.com/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9/ui/views/animation/test/test_ink_drop_delegate.cc
[delete] https://crrev.com/9f0da5d0c526cddcb7b4d6fdf11fa4ec54ba84c9/ui/views/animation/test/test_ink_drop_delegate.h

Project Member

Comment 97 by bugdroid1@chromium.org, Jul 15 2017

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

commit 2ca6ae0f8886b307b24f8ff5a269a5921d0c093f
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Sat Jul 15 09:46:56 2017

Add missing header from ui/views/linux_ui to BUILD.gn

BUG=661774
TBR=erg@chromium.org

Change-Id: I16aa7ff8acd6c389173f970485fcb9659900a45d
Reviewed-on: https://chromium-review.googlesource.com/572871
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486989}
[modify] https://crrev.com/2ca6ae0f8886b307b24f8ff5a269a5921d0c093f/ui/views/BUILD.gn

Project Member

Comment 98 by bugdroid1@chromium.org, Jul 17 2017

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

commit 85285744e21ecbd7f10bc06763a0e5f0555bfabd
Author: Duc Bui <ducbui@google.com>
Date: Mon Jul 17 14:20:40 2017

Add a missing header file session_restore_observer.h into BUILD.gn.

The header file was added when adding signals that mark the start/end
 of session restore (https://codereview.chromium.org/2935183002).

Bug: 661774
Change-Id: I4cfa262281ceb880e76cf99a84b319119e7261e1
Reviewed-on: https://chromium-review.googlesource.com/573488
Commit-Queue: Duc Bui <ducbui@google.com>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487073}
[modify] https://crrev.com/85285744e21ecbd7f10bc06763a0e5f0555bfabd/chrome/browser/BUILD.gn

Project Member

Comment 99 by bugdroid1@chromium.org, Jul 17 2017

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

commit 700ac398fcdc27d3c2eb6237b9856412c9b464a7
Author: Fernando Serboncini <fserb@chromium.org>
Date: Mon Jul 17 17:41:56 2017

include header as GN dependency

Bug: 661774
Change-Id: I7af30f80306fc506ab3b0b0b3eeed5b79fbbd845
Reviewed-on: https://chromium-review.googlesource.com/574632
Commit-Queue: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487144}
[modify] https://crrev.com/700ac398fcdc27d3c2eb6237b9856412c9b464a7/third_party/WebKit/Source/platform/BUILD.gn

Project Member

Comment 100 by bugdroid1@chromium.org, Jul 17 2017

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

commit aba78123539a26adf3ce90ef8bbba4ac3c0ffe4e
Author: Zijie He <zijiehe@chromium.org>
Date: Mon Jul 17 18:38:55 2017

Only include "base/win/windows_version.h on Windows build

In change https://codereview.chromium.org/2941623003, base/win/windows_version.h
was included in remoting/host/host_attributes.cc for all platforms. This issue
breaks gn analyze. Refer to the bug for more details.

Bug: 661774
Change-Id: I940bf9a87870fad76d14f6f7bea2c439cde081dc
Reviewed-on: https://chromium-review.googlesource.com/573560
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487170}
[modify] https://crrev.com/aba78123539a26adf3ce90ef8bbba4ac3c0ffe4e/remoting/host/host_attributes.cc

Some analysis during the period of time where we don't have check_gn_headers enabled.

During 6/27 ~ 7/14, there were 7 CLs introducing regressions.

1. r482654 https://codereview.chromium.org/2935183002 chrome/browser/sessions/session_restore_observer.h
2. r482890 https://codereview.chromium.org/2941623003 base/win/windows_version.h
3. r483625 https://codereview.chromium.org/2963033002 ui/views/linux_ui/device_scale_factor_observer.h
4. r484122 https://chromium-review.googlesource.com/c/544783/ third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFontCache.h
5. r484594 https://chromium-review.googlesource.com/c/559629/ chrome/browser/chromeos/arc/arc_service_launcher.h
6. r484737 https://chromium-review.googlesource.com/c/558888/ third_party/WebKit/public/platform/modules/payments/WebCanMakePaymentEventData.h
7. r486794 https://chromium-review.googlesource.com/c/570426/ third_party/WebKit/Source/platform/fonts/FontTestUtilities.h

The root causes were:
1, 6, 7: added header(s) and also touched GN files (but did it wrong)
3, 4: added header without touching GN files
2, 5: include OS-specific header without using #if defined(OS_...)

Can be caught by PRESUBMIT checking:
1, 3, 4, 6, 7

Can be caught by check_gn_headers in CQ:
1, 6, 7

Including OS-specific header is a tricky one, and can only be caught by check_gn_headers in waterfall.

Therefore, adding PRESUBMIT seems to be a good idea, and more effective in preventing regressions than how check_gn_headers can be run on CQ.
I'm in the process of eliminating the USE_ASH define and replacing it with OS_CHROMEOS (since src/ash is only used on Chrome OS these days). See  issue 673826 

I just noticed that there are a large number of src/ash headers in build/check_gn_headers_whitelist.txt -- many of these includes used to be wrapped in #if defined(USE_ASH) blocks and are moving into OS_CHROMEOS blocks.

Do you think that will remove the need to whitelist these files? Is there any point in trying to clean up the whitelist? If so, how would I know it's safe to remove an entry?

We stopped running check_gn_headers in CQ and waterfall a while ago, so the checking is more manual, and the whitelist is not enforced. See issue 733500.

With PRESUBMIT, which is not there yet, we can re-enable check_gn_headers in CQ and waterfall if it's stable enough. Before that, we don't have to clean up the whitelist.

If you are doing this kind of cleaning up, it is highly recommended to run the checking tool manually, just to make sure no new errors are introduced. We can chat offline if you couldn't get the checking tool running properly.
OK. Given that this isn't in PRESUBMIT yet and the whitelist is not enforced, I'm not going to do any cleanup right now.

Feel free to ping me in the future if you need help with //ash or other chromeos headers.

Project Member

Comment 105 by bugdroid1@chromium.org, Jul 25

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

commit 3123f49bb3aee24655765cbe1b616fcaae935040
Author: Avi Drissman <avi@chromium.org>
Date: Wed Jul 25 20:44:07 2018

Remove unreferenced files.

These were added more than 4 years ago in
https://codereview.chromium.org/314393003 but not added to any
build files, and they haven't been touched since.

This resurrects https://codereview.chromium.org/2845063003/
which intended to remove these files but was never submitted.

BUG=661774,  381466 

Change-Id: I9a06a3f4e936c59a16cb271db2d0b83af249df48
Reviewed-on: https://chromium-review.googlesource.com/1150664
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578045}
[delete] https://crrev.com/363542350774552b96898910ed4937e8e64e25df/content/browser/web_contents/web_contents_view_overscroll_animator_mac.h
[delete] https://crrev.com/363542350774552b96898910ed4937e8e64e25df/content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.h
[delete] https://crrev.com/363542350774552b96898910ed4937e8e64e25df/content/browser/web_contents/web_contents_view_overscroll_animator_slider_mac.mm

Project Member

Comment 106 by bugdroid1@chromium.org, Jul 30

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

commit c0624d00588a1690f3a7d614deb6a778e409e75f
Author: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Date: Mon Jul 30 18:22:19 2018

Check for new headers without GN changes in PRESUBMIT

A CL adding new headers without corresponding GN changes is likely
wrong.

Bug: 661774
Change-Id: I11da5615b36ebc7b9f60490beb3a39874bd74ac2
Reviewed-on: https://chromium-review.googlesource.com/1152126
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579092}
[modify] https://crrev.com/c0624d00588a1690f3a7d614deb6a778e409e75f/PRESUBMIT.py
[modify] https://crrev.com/c0624d00588a1690f3a7d614deb6a778e409e75f/PRESUBMIT_test.py

Showing comments 7 - 106 of 106 Older

Sign in to add a comment