Better bad message reporting from IO thread

Better bad message reporting from IO thread

This makes crash dumping optional for RPH::ShutdownForBadMessage,
and changes the thread-safe bad_message::ReceivedBadMessage to
generate its own crash dump immediately, before potentially posting
a UI thread task.

All other call sites have been updated to opt into crash dumping,
preserving previous behavior.

This ensures that bad message reports from the IO thread have a
useful crash stack.

BUG=652755
R=jam@chromium.org

Committed: https://crrev.com/f050c89a2ba1799844b3615884000d5fa7a03e11
Cr-Commit-Position: refs/heads/master@{#423040}

[Ken Rockot(use gerrit already), 2016-10-04 20:06:21]

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

[commit-bot: I haz the power, 2016-10-04 20:06:49]

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, 2016-10-04 20:26:45]

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

[commit-bot: I haz the power, 2016-10-04 20:26:46]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[jam, 2016-10-04 23:57:22]

lgtm

[Ken Rockot(use gerrit already), 2016-10-04 23:59:24]

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

[commit-bot: I haz the power, 2016-10-04 23:59:54]

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

[Ken Rockot(use gerrit already), 2016-10-05 00:30:17]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-10-05 00:30:18]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-10-05 00:30:18]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2387113004/#ps20001
(title: "fix missed call site")

[commit-bot: I haz the power, 2016-10-05 00:30:37]

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

[Ken Rockot(use gerrit already), 2016-10-05 00:31:23]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-10-05 00:32:52]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-10-05 00:32:52]

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

[commit-bot: I haz the power, 2016-10-05 00:33:13]

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-10-05 01:28:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-05 01:31:27]

Description was changed from

==========
Better bad message reporting from IO thread

This makes crash dumping optional for RPH::ShutdownForBadMessage,
and changes the thread-safe bad_message::ReceivedBadMessage to
generate its own crash dump immediately, before potentially posting
a UI thread task.

All other call sites have been updated to opt into crash dumping,
preserving previous behavior.

This ensures that bad message reports from the IO thread have a
useful crash stack.

BUG=652755
R=jam@chromium.org
==========

to

==========
Better bad message reporting from IO thread

This makes crash dumping optional for RPH::ShutdownForBadMessage,
and changes the thread-safe bad_message::ReceivedBadMessage to
generate its own crash dump immediately, before potentially posting
a UI thread task.

All other call sites have been updated to opt into crash dumping,
preserving previous behavior.

This ensures that bad message reports from the IO thread have a
useful crash stack.

BUG=652755
R=jam@chromium.org

Committed: https://crrev.com/f050c89a2ba1799844b3615884000d5fa7a03e11
Cr-Commit-Position: refs/heads/master@{#423040}
==========

[commit-bot: I haz the power, 2016-10-05 01:31:28]

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

[Charlie Reis, 2017-01-12 00:39:29]

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

[Charlie Reis, 2017-01-12 00:39:30]

I know this has already landed, but it looks like it regressed some of the
renderer kill logic.  Can you take a look?

https://codereview.chromium.org/2387113004/diff/40001/content/browser/bad_mes...
File content/browser/bad_message.cc (right):

https://codereview.chromium.org/2387113004/diff/40001/content/browser/bad_mes...
content/browser/bad_message.cc:54: base::debug::DumpWithoutCrashing();
I just looked at a crash report that hit this (9fa49da580000000), and it's
missing the "bad_message_reason" crash key that gets set in LogBadMessage.  Can
you take a look at how to restore that key for IO thread crashes?

The key helps disambiguate when there are multiple renderer kills in a given
method, and it puts the reason code in the magic signature itself.

[Ken Rockot(use gerrit already), 2017-01-12 16:50:45]

Hmm. Yes, I'll take a look.

On Wed, Jan 11, 2017 at 4:39 PM, <creis@chromium.org> wrote:

> I know this has already landed, but it looks like it regressed some of the
> renderer kill logic. Can you take a look?
>
>
> https://codereview.chromium.org/2387113004/diff/40001/
> content/browser/bad_message.cc
> File content/browser/bad_message.cc (right):
>
> https://codereview.chromium.org/2387113004/diff/40001/
> content/browser/bad_message.cc#newcode54
> content/browser/bad_message.cc:54: base::debug::DumpWithoutCrashing();
> I just looked at a crash report that hit this (9fa49da580000000), and
> it's missing the "bad_message_reason" crash key that gets set in
> LogBadMessage. Can you take a look at how to restore that key for IO
> thread crashes?
>
> The key helps disambiguate when there are multiple renderer kills in a
> given method, and it puts the reason code in the magic signature itself.
>
> https://codereview.chromium.org/2387113004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.