Fix SyzyASAN build

Fix SyzyASAN build

Removes Kasko for hang watcher from chrome.exe build.

Fixes after changes in https://codereview.chromium.org/2250263002.

BUG=638370, 604923
Committed: https://crrev.com/4879c957ec62ddaa58340afbfdbfa9a4f72c4516
Cr-Commit-Position: refs/heads/master@{#412883}

[scottmg, 2016-08-17 16:09:50]

The CQ bit was checked by scottmg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 16:10:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-17 16:10:50]

Description was changed from

==========
Fix Kasko SyzyASAN build

After https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

to

==========
Fix Kasko SyzyASAN build

After https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

[scottmg, 2016-08-17 16:10:50]

scottmg@chromium.org changed reviewers:
+ chrisha@chromium.org, siggi@chromium.org

[scottmg, 2016-08-17 16:11:48]

First to stamp wins!

I haven't finished a full build (official!) but

ninja -C out\Release obj/chrome/chrome_initial/kasko_client.obj

builds again now in is_syzyasan=true && is_official_build=true.

[Sigurður Ásgeirsson, 2016-08-17 16:15:58]

lgtm - thanks!

[scottmg, 2016-08-17 16:21:36]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2016-08-17 16:21:37]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2016-08-17 16:21:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottmg, 2016-08-17 17:36:12]

Local build just finished and looks liek this doesn't link still, looking...

[scottmg, 2016-08-17 17:36:12]

The CQ bit was unchecked by scottmg@chromium.org

[scottmg, 2016-08-17 17:49:19]

scottmg@chromium.org changed reviewers:
+ ananta@chromium.org

[scottmg, 2016-08-17 17:52:30]

scottmg@chromium.org changed reviewers:
+ manzagop@chromium.org

[scottmg, 2016-08-17 17:53:14]

On 2016/08/17 17:36:12, scottmg wrote:
> Local build just finished and looks liek this doesn't link still, looking...

Link failure is because of the ChromeWatcherClient code here
https://cs.chromium.org/chromium/src/chrome/app/main_dll_loader_win.cc?rcl=0&...

(So, nothing to do with SyzyASAN, just to do with having is_syzyasan=true turn
on Kasko.)

Are we still using that code path? It's been always-off since Jul 19th, when the
Crashpad database and client moved to chrome_elf (the database wasn't
initialized in chrome.exe, so GetUploadsEnabled() was always false until
https://codereview.chromium.org/2250263002 yesterday. 

It's non-trivial to resurrect it because ChromeCrashReporterClient isn't in
chrome.exe any more.

[manzagop (departed), 2016-08-17 18:24:10]

On 2016/08/17 17:53:14, scottmg wrote:
> On 2016/08/17 17:36:12, scottmg wrote:
> > Local build just finished and looks liek this doesn't link still, looking...
> 
> Link failure is because of the ChromeWatcherClient code here
>
https://cs.chromium.org/chromium/src/chrome/app/main_dll_loader_win.cc?rcl=0&...
> 
> (So, nothing to do with SyzyASAN, just to do with having is_syzyasan=true turn
> on Kasko.)
> 
> Are we still using that code path? It's been always-off since Jul 19th, when
the
> Crashpad database and client moved to chrome_elf (the database wasn't
> initialized in chrome.exe, so GetUploadsEnabled() was always false until
> https://codereview.chromium.org/2250263002 yesterday. 
> 
> It's non-trivial to resurrect it because ChromeCrashReporterClient isn't in
> chrome.exe any more.

The browser watcher was using that kasko to send the browser hang reports. BUT,
that was removed in
https://codereview.chromium.org/2086403002/diff/60001/chrome/chrome_watcher/c...

So I don't see this being used.

[Sigurður Ásgeirsson, 2016-08-17 20:52:31]

On 2016/08/17 17:53:14, scottmg wrote:
> On 2016/08/17 17:36:12, scottmg wrote:
> > Local build just finished and looks liek this doesn't link still, looking...
> 
> Link failure is because of the ChromeWatcherClient code here
>
https://cs.chromium.org/chromium/src/chrome/app/main_dll_loader_win.cc?rcl=0&...
> 
> (So, nothing to do with SyzyASAN, just to do with having is_syzyasan=true turn
> on Kasko.)
> 
> Are we still using that code path? It's been always-off since Jul 19th, when
the
> Crashpad database and client moved to chrome_elf (the database wasn't
> initialized in chrome.exe, so GetUploadsEnabled() was always false until
> https://codereview.chromium.org/2250263002 yesterday. 
> 
> It's non-trivial to resurrect it because ChromeCrashReporterClient isn't in
> chrome.exe any more.

The hang watcher is gonsky for now, it's fine to scuttle this - care to do the
honors?

[scottmg, 2016-08-17 20:58:01]

On 2016/08/17 20:52:31, Sigurður Ásgeirsson wrote:
> On 2016/08/17 17:53:14, scottmg wrote:
> > On 2016/08/17 17:36:12, scottmg wrote:
> > > Local build just finished and looks liek this doesn't link still,
looking...
> > 
> > Link failure is because of the ChromeWatcherClient code here
> >
>
https://cs.chromium.org/chromium/src/chrome/app/main_dll_loader_win.cc?rcl=0&...
> > 
> > (So, nothing to do with SyzyASAN, just to do with having is_syzyasan=true
turn
> > on Kasko.)
> > 
> > Are we still using that code path? It's been always-off since Jul 19th, when
> the
> > Crashpad database and client moved to chrome_elf (the database wasn't
> > initialized in chrome.exe, so GetUploadsEnabled() was always false until
> > https://codereview.chromium.org/2250263002 yesterday. 
> > 
> > It's non-trivial to resurrect it because ChromeCrashReporterClient isn't in
> > chrome.exe any more.
> 
> The hang watcher is gonsky for now, it's fine to scuttle this - care to do the
> honors?

Aha, OK, thanks. Updated this CL to remove the watcher code which incidentally
will fix the SyzyASAN build too since that's the bit of code that was breaking
it.

[scottmg, 2016-08-17 20:58:47]

Description was changed from

==========
Fix Kasko SyzyASAN build

After https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

to

==========
Fix SyzyASAN build

Removes Kasko for hang watcher from chrome.exe build.

Fixes after changes in https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

[scottmg, 2016-08-17 21:00:05]

scottmg@chromium.org changed reviewers:
+ robertshield@chromium.org

[scottmg, 2016-08-17 21:00:05]

+robertshield for re-removing the bit in chrome_elf that got added yesterday.

[robertshield, 2016-08-17 21:46:26]

On 2016/08/17 21:00:05, scottmg wrote:
> +robertshield for re-removing the bit in chrome_elf that got added yesterday.

Removing chrome_elf bit lgtm

[scottmg, 2016-08-17 21:47:13]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-08-17 21:47:14]

The patchset sent to the CQ was uploaded after l-g-t-m from siggi@chromium.org
Link to the patchset: https://codereview.chromium.org/2256723002/#ps20001
(title: ".")

[commit-bot: I haz the power, 2016-08-17 21:47:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 21:55:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-17 21:55:13]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[scottmg, 2016-08-17 21:57:40]

scottmg@chromium.org changed reviewers:
+ grt@chromium.org

[scottmg, 2016-08-17 21:57:41]

oops, +grt for chrome/app.

[grt (UTC plus 2), 2016-08-18 08:12:43]

lgtm w/ gypi fix

https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (left):

https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn#oldcode144
chrome/BUILD.gn:144: "app/kasko_client.cc",
please remove these same two lines from chrome/chrome_exe.gypi

[grt (UTC plus 2), 2016-08-18 08:12:44]

lgtm w/ gypi fix

[scottmg, 2016-08-18 16:26:58]

https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (left):

https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn#oldcode144
chrome/BUILD.gn:144: "app/kasko_client.cc",
On 2016/08/18 08:12:43, grt (UTC plus 2) wrote:
> please remove these same two lines from chrome/chrome_exe.gypi

I was under the impression that we weren't supposed to maintain the gyp build
any more. But since this is trivial, I will here.

[scottmg, 2016-08-18 16:27:03]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-08-18 16:27:04]

The patchset sent to the CQ was uploaded after l-g-t-m from siggi@chromium.org,
robertshield@chromium.org, grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2256723002/#ps40001
(title: ".")

[commit-bot: I haz the power, 2016-08-18 16:27:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 17:45:27]

Description was changed from

==========
Fix SyzyASAN build

Removes Kasko for hang watcher from chrome.exe build.

Fixes after changes in https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

to

==========
Fix SyzyASAN build

Removes Kasko for hang watcher from chrome.exe build.

Fixes after changes in https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

[commit-bot: I haz the power, 2016-08-18 17:45:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-18 17:48:46]

Description was changed from

==========
Fix SyzyASAN build

Removes Kasko for hang watcher from chrome.exe build.

Fixes after changes in https://codereview.chromium.org/2250263002.

BUG=638370, 604923
==========

to

==========
Fix SyzyASAN build

Removes Kasko for hang watcher from chrome.exe build.

Fixes after changes in https://codereview.chromium.org/2250263002.

BUG=638370, 604923

Committed: https://crrev.com/4879c957ec62ddaa58340afbfdbfa9a4f72c4516
Cr-Commit-Position: refs/heads/master@{#412883}
==========

[commit-bot: I haz the power, 2016-08-18 17:48:47]

Patchset 3 (id:??) landed as
https://crrev.com/4879c957ec62ddaa58340afbfdbfa9a4f72c4516
Cr-Commit-Position: refs/heads/master@{#412883}

[grt (UTC plus 2), 2016-08-18 20:38:51]

https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn
File chrome/BUILD.gn (left):

https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn#oldcode144
chrome/BUILD.gn:144: "app/kasko_client.cc",
On 2016/08/18 16:26:58, scottmg wrote:
> On 2016/08/18 08:12:43, grt (UTC plus 2) wrote:
> > please remove these same two lines from chrome/chrome_exe.gypi
> 
> I was under the impression that we weren't supposed to maintain the gyp build
> any more. But since this is trivial, I will here.

Thanks. My understanding was that GYP was no longer run by default, but that it
was still supported in some sense (see "PSA: GYP is no longer run by default in
gclient runhooks" on chromium-dev). I don't expect it to really work, but I find
it confusing to have ghosts present in .gyp* files.

[scottmg, 2016-08-18 20:44:55]

On 2016/08/18 20:38:51, grt (UTC plus 2) wrote:
> https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn
> File chrome/BUILD.gn (left):
> 
>
https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn#oldcode144
> chrome/BUILD.gn:144: "app/kasko_client.cc",
> On 2016/08/18 16:26:58, scottmg wrote:
> > On 2016/08/18 08:12:43, grt (UTC plus 2) wrote:
> > > please remove these same two lines from chrome/chrome_exe.gypi
> > 
> > I was under the impression that we weren't supposed to maintain the gyp
build
> > any more. But since this is trivial, I will here.
> 
> Thanks. My understanding was that GYP was no longer run by default, but that
it
> was still supported in some sense (see "PSA: GYP is no longer run by default
in
> gclient runhooks" on chromium-dev). I don't expect it to really work, but I
find
> it confusing to have ghosts present in .gyp* files.

Yup, it's confusing to have grep show things up, especially since GN still pulls
file lists from gypi files in many cases.

I was going based on
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/NZkPr-CXvQ0/dn1rb...
from a couple weeks after "PSA: GYP is no longer run by default in
gclient runhooks". So I think we're off now officially the gyp hook.

I inquired as to when we might be able to do the great purge, but for some
not-too-clear reason, we're keeping the gyp files until M54. Not long though...

[grt (UTC plus 2), 2016-08-19 07:53:09]

On 2016/08/18 20:44:55, scottmg wrote:
> On 2016/08/18 20:38:51, grt (UTC plus 2) wrote:
> > https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn
> > File chrome/BUILD.gn (left):
> > 
> >
>
https://codereview.chromium.org/2256723002/diff/20001/chrome/BUILD.gn#oldcode144
> > chrome/BUILD.gn:144: "app/kasko_client.cc",
> > On 2016/08/18 16:26:58, scottmg wrote:
> > > On 2016/08/18 08:12:43, grt (UTC plus 2) wrote:
> > > > please remove these same two lines from chrome/chrome_exe.gypi
> > > 
> > > I was under the impression that we weren't supposed to maintain the gyp
> build
> > > any more. But since this is trivial, I will here.
> > 
> > Thanks. My understanding was that GYP was no longer run by default, but that
> it
> > was still supported in some sense (see "PSA: GYP is no longer run by default
> in
> > gclient runhooks" on chromium-dev). I don't expect it to really work, but I
> find
> > it confusing to have ghosts present in .gyp* files.
> 
> Yup, it's confusing to have grep show things up, especially since GN still
pulls
> file lists from gypi files in many cases.
> 
> I was going based on
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/NZkPr-CXvQ0/dn1rb...
> from a couple weeks after "PSA: GYP is no longer run by default in
> gclient runhooks". So I think we're off now officially the gyp hook.

Awesome. Thanks for the link.