Fix remove_webcore_debug_symbols to avoid constant building

Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols (in
crrev.com/1716523003) didn't generate PDBs for some of the webkit DLLs
in component builds, but that caused problems because ninja still
expects them to be outputs. This caused the build to never be clean (on
Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG=647525
Review-Url: https://codereview.chromium.org/2948363004
Cr-Commit-Position: refs/heads/master@{#482917}
Committed: https://chromium.googlesource.com/chromium/src/+/b919d95a7f8be9ca291d6bbff942cfb42d08d5a1

[brucedawson, 2017-06-24 00:16:46]

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

[commit-bot: I haz the power, 2017-06-24 00:17:03]

Dry run: CQ is trying da patch.

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

[brucedawson, 2017-06-24 00:19:06]

Description was changed from

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original implementation of remove_webcore_debug_symbols didn't
generate PDBs for some of the webkit DLLs in component builds, but that
causes problems because ninja still expects them to be outputs. This
causes the build to never be clean (on Windows) when this flag is used.

This gets these DLLs to use minimal_symbols (on Windows builds where
symbol_level != 0) so that a PDB is always generated when expected.

This is a tradeoff - linking of these DLLs will be slightly slower - but
the cost is minimal and the gains (especially in consistent behavior)
are quite nice.

BUG=647525
==========

to

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original implementation of remove_webcore_debug_symbols didn't
generate PDBs for some of the webkit DLLs in component builds, but that
causes problems because ninja still expects them to be outputs. This
causes the build to never be clean (on Windows) when this flag is used.

This gets these DLLs to use minimal_symbols (on Windows builds where
symbol_level != 0) so that a PDB is always generated when expected.

This is a tradeoff - linking of these DLLs will be slightly slower - but
the cost is minimal and the gains (especially in consistent behavior)
are quite nice.

BUG=647525
==========

[brucedawson, 2017-06-24 00:19:06]

brucedawson@chromium.org changed reviewers:
+ dpranke@chromium.org

[commit-bot: I haz the power, 2017-06-24 02:31:53]

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

[commit-bot: I haz the power, 2017-06-24 02:31:53]

Dry run: This issue passed the CQ dry run.

[brucedawson, 2017-06-26 18:34:16]

Description was changed from

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original implementation of remove_webcore_debug_symbols didn't
generate PDBs for some of the webkit DLLs in component builds, but that
causes problems because ninja still expects them to be outputs. This
causes the build to never be clean (on Windows) when this flag is used.

This gets these DLLs to use minimal_symbols (on Windows builds where
symbol_level != 0) so that a PDB is always generated when expected.

This is a tradeoff - linking of these DLLs will be slightly slower - but
the cost is minimal and the gains (especially in consistent behavior)
are quite nice.

BUG=647525
==========

to

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols didn't
generate PDBs for some of the webkit DLLs in component builds, but that
caused problems because ninja still expects them to be outputs. This
caused the build to never be clean (on Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG=647525
==========

[brucedawson, 2017-06-26 18:35:27]

Description was changed from

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols didn't
generate PDBs for some of the webkit DLLs in component builds, but that
caused problems because ninja still expects them to be outputs. This
caused the build to never be clean (on Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG=647525
==========

to

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols (in
crrev.com/1716523003) didn't generate PDBs for some of the webkit DLLs
in component builds, but that caused problems because ninja still
expects them to be outputs. This caused the build to never be clean (on
Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG=647525
==========

[brucedawson, 2017-06-26 18:37:51]

On 2017/06/26 18:35:35, brucedawson wrote:

BTW, the other fix for these not-clean builds is crrev.com/2951413004, also out
for review.

[brucedawson, 2017-06-27 20:06:04]

I updated this change to avoid duplicating the logic in a half-dozen files. It's
much cleaner and more maintainable now.

PTAL.

[Dirk Pranke, 2017-06-28 02:00:42]

Makes sense, nice fix.

lgtm.

[hashimoto, 2017-06-28 02:30:42]

hashimoto@chromium.org changed reviewers:
+ hashimoto@chromium.org

[hashimoto, 2017-06-28 02:30:42]

lgtm

[brucedawson, 2017-06-28 06:18:23]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2017-06-28 06:18:41]

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, 2017-06-28 08:20:00]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1498630703416090,
"parent_rev": "eb576e6c660bc1ce8e7aaf93bfab70042479a237", "commit_rev":
"b919d95a7f8be9ca291d6bbff942cfb42d08d5a1"}

[commit-bot: I haz the power, 2017-06-28 08:20:19]

Description was changed from

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols (in
crrev.com/1716523003) didn't generate PDBs for some of the webkit DLLs
in component builds, but that caused problems because ninja still
expects them to be outputs. This caused the build to never be clean (on
Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG=647525
==========

to

==========
Fix remove_webcore_debug_symbols to avoid constant building

The original gn implementation of remove_webcore_debug_symbols (in
crrev.com/1716523003) didn't generate PDBs for some of the webkit DLLs
in component builds, but that caused problems because ninja still
expects them to be outputs. This caused the build to never be clean (on
Windows) when this flag is used.

This change gets these DLLs to use minimal_symbols (on Windows builds
where symbol_level != 0) so that a PDB is always generated when
expected.

Doing test builds on my Windows machine of blink_core.dll I see the
following full-link times and chrome.dll.pdb sizes when I do a debug
component build with symbol_level = 2 when remove_webcore_debug_symbols
is set to the following configs:

        symbols: 71 s, 348 MB
minimal_symbols: 49 s,  82 MB
     no_symbols: 49 s, missing - builds never clean

This suggests that minimal_symbols is a reasonable choice (no increase
in build times) and a good fix for the not-clean builds.

This change does not alter the behavior on other platforms because
they don't have the not-clean issue.

In addition to avoiding constant building this also means that there
will be enough symbols for full call stacks.

BUG=647525

Review-Url: https://codereview.chromium.org/2948363004
Cr-Commit-Position: refs/heads/master@{#482917}
Committed:
https://chromium.googlesource.com/chromium/src/+/b919d95a7f8be9ca291d6bbff942...
==========

[commit-bot: I haz the power, 2017-06-28 08:20:21]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b919d95a7f8be9ca291d6bbff942...