gpu: Add a util method to set crash-keys from a GPUInfo.

gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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

Earlier landed in r458276, reverted in r458968 because of size regression.

Review-Url: https://codereview.chromium.org/2751103008
Cr-Commit-Position: refs/heads/master@{#459202}
Committed: https://chromium.googlesource.com/chromium/src/+/c05f3475ecaae3f033bb4ee8cfe139b8c4d0b897

[sadrul, 2017-03-18 03:33:02]

Description was changed from

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
==========

to

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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
==========

[sadrul, 2017-03-18 03:33:12]

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

[commit-bot: I haz the power, 2017-03-18 03:33:25]

Dry run: 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-03-18 04:05:24]

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

[commit-bot: I haz the power, 2017-03-18 04:05:25]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sadrul, 2017-03-18 17:04:40]

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

[commit-bot: I haz the power, 2017-03-18 17:04:44]

Dry run: 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-03-18 18:13:00]

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

[commit-bot: I haz the power, 2017-03-18 18:13:01]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-19 00:59:46]

sadrul@chromium.org changed reviewers:
+ boliu@chromium.org, bradnelson@chromium.org, piman@chromium.org,
rsesek@chromium.org

[sadrul, 2017-03-19 00:59:47]

piman@ for gpu/ changes

rsesek@ for crash_keys changes

boliu@ for change in android_webview

bradnelson@ for components/nacl change

Thank you!

[Robert Sesek, 2017-03-20 15:22:24]

lgtm

[boliu, 2017-03-20 16:59:30]

lgtm

[piman, 2017-03-20 20:57:28]

piman@chromium.org changed reviewers:
+ jam@chromium.org

[piman, 2017-03-20 20:57:29]

+jam
So far we have left crash-related stuff on layers above content/ (mainly chrome/
and android_webview/). Moving it to gpu means it's below content/ and I'm not
sure if that's generally desirable, in particular wrt embedders etc.

[Robert Sesek, 2017-03-20 20:59:32]

On 2017/03/20 20:57:29, piman wrote:
> +jam
> So far we have left crash-related stuff on layers above content/ (mainly
chrome/
> and android_webview/). Moving it to gpu means it's below content/ and I'm not
> sure if that's generally desirable, in particular wrt embedders etc.

The function could move to crash_keys.h/cc, which is where other
crash-key-setting functions live (for extensions, switches, etc.).

[sadrul, 2017-03-20 21:03:28]

On 2017/03/20 20:57:29, piman wrote:
> +jam
> So far we have left crash-related stuff on layers above content/ (mainly
chrome/
> and android_webview/). Moving it to gpu means it's below content/ and I'm not
> sure if that's generally desirable, in particular wrt embedders etc.

On that note, I actually considered removing ContentClient::SetGpuInfo(), but
then realized that aw generates a fingerprint out of the GPUInfo. So I left that
alone. So embedders can still do additional things if they want with the
GPUInfo. If embedders are not interested to collect these keys, then they can
ignore these keys by excluding them from the white-list installed during
initialization using base::debug::InitCrashKeys(). So I feel like this should be
OK, although if there's a better way of reusing the code, I can look into that
too.

[sadrul, 2017-03-20 21:04:12]

On 2017/03/20 20:59:32, Robert Sesek wrote:
> On 2017/03/20 20:57:29, piman wrote:
> > +jam
> > So far we have left crash-related stuff on layers above content/ (mainly
> chrome/
> > and android_webview/). Moving it to gpu means it's below content/ and I'm
not
> > sure if that's generally desirable, in particular wrt embedders etc.
> 
> The function could move to crash_keys.h/cc, which is where other
> crash-key-setting functions live (for extensions, switches, etc.).

Ah, you mean gpu/config/crash_keys.h|cc? Sure, I can make that change if you
want.

[Robert Sesek, 2017-03-20 21:04:48]

On 2017/03/20 21:04:12, sadrul wrote:
> On 2017/03/20 20:59:32, Robert Sesek wrote:
> > On 2017/03/20 20:57:29, piman wrote:
> > > +jam
> > > So far we have left crash-related stuff on layers above content/ (mainly
> > chrome/
> > > and android_webview/). Moving it to gpu means it's below content/ and I'm
> not
> > > sure if that's generally desirable, in particular wrt embedders etc.
> > 
> > The function could move to crash_keys.h/cc, which is where other
> > crash-key-setting functions live (for extensions, switches, etc.).
> 
> Ah, you mean gpu/config/crash_keys.h|cc? Sure, I can make that change if you
> want.

No, I mean chrome/common/crash_keys.h. e.g. SetActiveExtensions

[sadrul, 2017-03-20 21:06:15]

On 2017/03/20 21:04:48, Robert Sesek wrote:
> On 2017/03/20 21:04:12, sadrul wrote:
> > On 2017/03/20 20:59:32, Robert Sesek wrote:
> > > On 2017/03/20 20:57:29, piman wrote:
> > > > +jam
> > > > So far we have left crash-related stuff on layers above content/ (mainly
> > > chrome/
> > > > and android_webview/). Moving it to gpu means it's below content/ and
I'm
> > not
> > > > sure if that's generally desirable, in particular wrt embedders etc.
> > > 
> > > The function could move to crash_keys.h/cc, which is where other
> > > crash-key-setting functions live (for extensions, switches, etc.).
> > 
> > Ah, you mean gpu/config/crash_keys.h|cc? Sure, I can make that change if you
> > want.
> 
> No, I mean chrome/common/crash_keys.h. e.g. SetActiveExtensions

Places like android_webview, or mus, won't be able to use that though, since
they don't have a dependency on //chrome

[jam, 2017-03-20 22:19:52]

On 2017/03/20 20:57:29, piman wrote:
> +jam
> So far we have left crash-related stuff on layers above content/ (mainly
chrome/
> and android_webview/). Moving it to gpu means it's below content/ and I'm not
> sure if that's generally desirable, in particular wrt embedders etc.

You're right we keep the specific crash reporter out of src/content (other than
shell). However this is just calling a base method to set the crash key value,
which we already have in src/content so that part lgtm

[piman, 2017-03-20 22:52:50]

lgtm

[sadrul, 2017-03-21 00:26:12]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-03-21 00:26:49]

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-03-21 02:03:45]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490055972393790,
"parent_rev": "cba28fc12c0dfe73592d9a6b08f5464d4aa468f0", "commit_rev":
"1432276fb82ae8e9b732faa03ab65f35d686017f"}

[commit-bot: I haz the power, 2017-03-21 02:04:24]

Description was changed from

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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
==========

to

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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/2751103008
Cr-Commit-Position: refs/heads/master@{#458276}
Committed:
https://chromium.googlesource.com/chromium/src/+/1432276fb82ae8e9b732faa03ab6...
==========

[commit-bot: I haz the power, 2017-03-21 02:04:25]

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

[Will Harris, 2017-03-22 22:19:16]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2770953002/ by wfh@chromium.org.

The reason for reverting is: causes size regression see crbug.com/703622.

[sadrul, 2017-03-23 02:57:03]

Description was changed from

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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/2751103008
Cr-Commit-Position: refs/heads/master@{#458276}
Committed:
https://chromium.googlesource.com/chromium/src/+/1432276fb82ae8e9b732faa03ab6...
==========

to

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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/2751103008
Cr-Commit-Position: refs/heads/master@{#458276}
Committed:
https://chromium.googlesource.com/chromium/src/+/1432276fb82ae8e9b732faa03ab6...
==========

[sadrul, 2017-03-23 06:42:57]

Description was changed from

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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/2751103008
Cr-Commit-Position: refs/heads/master@{#458276}
Committed:
https://chromium.googlesource.com/chromium/src/+/1432276fb82ae8e9b732faa03ab6...
==========

to

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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

Earlier landed in r458276, reverted in r458968 because of size regression.
==========

[sadrul, 2017-03-23 06:43:13]

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

[commit-bot: I haz the power, 2017-03-23 06:43:21]

Dry run: 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-03-23 06:46:45]

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

[commit-bot: I haz the power, 2017-03-23 06:46:46]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[sadrul, 2017-03-23 06:54:19]

Patchset #5 (id:80001) has been deleted

[sadrul, 2017-03-23 06:55:33]

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

[commit-bot: I haz the power, 2017-03-23 06:55:48]

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

[sadrul, 2017-03-23 07:03:40]

chrome/common/crash_keys.cc|h only accesses the crash-keys from gpu/config. So I
have moved the keys into a separate //gpu/config:crash_keys component, which is
added as a dependency of chrome.

I did a local build in non-component build, and this change seems to address the
size regression (with patch: 1.2MB, without: 1.2MB, size with regression before
revert: 4.8MB)

Patchset 2 is the one that landed. Patchset 4 is a revert of the revert, and
patchset 5 has the new change with the new component. PTAL.

[commit-bot: I haz the power, 2017-03-23 07:03:43]

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

[commit-bot: I haz the power, 2017-03-23 07:03:44]

Dry run: Try jobs failed on following builders:
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[sadrul, 2017-03-23 07:34:25]

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

[commit-bot: I haz the power, 2017-03-23 07:34:42]

Dry run: 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-03-23 07:59:43]

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

[commit-bot: I haz the power, 2017-03-23 07:59:44]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Nico, 2017-03-23 12:43:44]

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

[Nico, 2017-03-23 12:43:46]

diff from 5 to 6 lgtm

https://codereview.chromium.org/2751103008/diff/120001/gpu/config/BUILD.gn
File gpu/config/BUILD.gn (right):

https://codereview.chromium.org/2751103008/diff/120001/gpu/config/BUILD.gn#ne...
gpu/config/BUILD.gn:21: source_set("crash_keys") {
add a comment that this is linked into both chrome.exe and chrome.dll and must
be small (and hence

[Nico, 2017-03-23 12:44:04]

On 2017/03/23 12:43:46, Nico wrote:
> diff from 5 to 6 lgtm
> 
> https://codereview.chromium.org/2751103008/diff/120001/gpu/config/BUILD.gn
> File gpu/config/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2751103008/diff/120001/gpu/config/BUILD.gn#ne...
> gpu/config/BUILD.gn:21: source_set("crash_keys") {
> add a comment that this is linked into both chrome.exe and chrome.dll and must
> be small (and hence

(ignore the "(and hence" part)

[sadrul, 2017-03-23 14:44:03]

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

[commit-bot: I haz the power, 2017-03-23 14:44:35]

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

[Robert Sesek, 2017-03-23 14:44:59]

lgtm

[sadrul, 2017-03-23 14:45:22]

https://codereview.chromium.org/2751103008/diff/120001/gpu/config/BUILD.gn
File gpu/config/BUILD.gn (right):

https://codereview.chromium.org/2751103008/diff/120001/gpu/config/BUILD.gn#ne...
gpu/config/BUILD.gn:21: source_set("crash_keys") {
On 2017/03/23 12:43:45, Nico wrote:
> add a comment that this is linked into both chrome.exe and chrome.dll and must
> be small (and hence 

Done.

[commit-bot: I haz the power, 2017-03-23 16:47:02]

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

[commit-bot: I haz the power, 2017-03-23 16:47:03]

Dry run: This issue passed the CQ dry run.

[piman, 2017-03-23 20:19:08]

lgtm

[sadrul, 2017-03-23 20:24:40]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2017-03-23 20:24:41]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
boliu@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/2751103008/#ps140001
(title: "comment")

[commit-bot: I haz the power, 2017-03-23 20:25:06]

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-03-23 20:30:13]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1490300680550820,
"parent_rev": "a038d334a0e1baae632e583dbc023f3383e8bf6f", "commit_rev":
"c05f3475ecaae3f033bb4ee8cfe139b8c4d0b897"}

[commit-bot: I haz the power, 2017-03-23 20:30:58]

Description was changed from

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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

Earlier landed in r458276, reverted in r458968 because of size regression.
==========

to

==========
gpu: Add a util method to set crash-keys from a GPUInfo.

Use the newly introduced util method to set crash-keys from a GPUInfo
in both chrome and android_webview. This will also be used in mus.

BUG=643746
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

Earlier landed in r458276, reverted in r458968 because of size regression.

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

[commit-bot: I haz the power, 2017-03-23 20:31:01]

Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/c05f3475ecaae3f033bb4ee8cfe1...