Better crash stacktraces for ExtensionFunction bad messages.

Better crash stacktraces for ExtensionFunction bad messages.

Record crash trace right where we set bad_message in ExtensionFunction

This will give us clue about which particular extension functions are
crashing. Previously, we used to record the trace from common response
callback where we already lost track of which function actually set
a bad_message.

Turn ExtensionFunction::set_bad_message(bool) to EF::SetBadMessage(void)
since one should only use this function to set bad_message state, not
clear it.

BUG=642794
Test=No visible change. Beware though, similar to
https://codereview.chromium.org/2629783002, this might show up as a
blame CL for extension function crashes. These will not really
be new crashes, so file a (non-release-blocking) bug.

Review-Url: https://codereview.chromium.org/2656013005
Cr-Commit-Position: refs/heads/master@{#446865}
Committed: https://chromium.googlesource.com/chromium/src/+/5e465202abc260c9aa8319f8e77bf721e0453ed2

[lazyboy, 2017-01-27 03:51:21]

Description was changed from

==========
Record crash trace right where we set bad_message in ExtensionFunction

This will give us clue about which particular extension functions are
crashing. Previously, we used to record the trace from common response
callback where we already lost track of which function actually set
a bad_message.

Turn ExtensionFunction::set_bad_message(bool) to EF::SetBadMessage(void)
since one should only use this function to set bad_message state, not
clear it.

BUG=642794
Test=No visible change. Beware though, similar to
https://codereview.chromium.org/2629783002, this might show up as a
blame CL for extension function crashes. These will not really
be new crashes, so file a (non-release-blocking) bug.
==========

to

==========
Better crash stacktraces for ExtensionFunction bad messages.

Record crash trace right where we set bad_message in ExtensionFunction

This will give us clue about which particular extension functions are
crashing. Previously, we used to record the trace from common response
callback where we already lost track of which function actually set
a bad_message.

Turn ExtensionFunction::set_bad_message(bool) to EF::SetBadMessage(void)
since one should only use this function to set bad_message state, not
clear it.

BUG=642794
Test=No visible change. Beware though, similar to
https://codereview.chromium.org/2629783002, this might show up as a
blame CL for extension function crashes. These will not really
be new crashes, so file a (non-release-blocking) bug.
==========

[lazyboy, 2017-01-27 03:52:59]

lazyboy@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Devlin, 2017-01-27 17:38:58]

lgtm

https://codereview.chromium.org/2656013005/diff/1/extensions/browser/extensio...
File extensions/browser/extension_function.cc (right):

https://codereview.chromium.org/2656013005/diff/1/extensions/browser/extensio...
extensions/browser/extension_function.cc:345: void
ExtensionFunction::SetBadMessage() {
Does this compile?  ExtensionFunction::SetBadMessage() is pure virtual in the
header.

https://codereview.chromium.org/2656013005/diff/1/extensions/browser/extensio...
extensions/browser/extension_function.cc:622:
ReceivedBadMessage((content::BrowserMessageFilter*)ipc_sender_.get(),
nit: prefer static_cast<> over c-style casts

[lazyboy, 2017-01-27 18:17:26]

https://codereview.chromium.org/2656013005/diff/1/extensions/browser/extensio...
File extensions/browser/extension_function.cc (right):

https://codereview.chromium.org/2656013005/diff/1/extensions/browser/extensio...
extensions/browser/extension_function.cc:345: void
ExtensionFunction::SetBadMessage() {
On 2017/01/27 17:38:58, Devlin (catching up) wrote:
> Does this compile?  ExtensionFunction::SetBadMessage() is pure virtual in the
> header.

Ah, I've put a mental note to fix it after noticing that this surprisingly
compiled locally: both chrome & browser_tests target were fine. But forgot.

Needs a separate look into this issue. Will do.

Done.

https://codereview.chromium.org/2656013005/diff/1/extensions/browser/extensio...
extensions/browser/extension_function.cc:622:
ReceivedBadMessage((content::BrowserMessageFilter*)ipc_sender_.get(),
On 2017/01/27 17:38:58, Devlin (catching up) wrote:
> nit: prefer static_cast<> over c-style casts

Done.

[lazyboy, 2017-01-27 18:41:17]

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

[commit-bot: I haz the power, 2017-01-27 18:41:47]

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-01-27 19:33:49]

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

[commit-bot: I haz the power, 2017-01-27 19:33:50]

Dry run: This issue passed the CQ dry run.

[lazyboy, 2017-01-28 01:07:40]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2017-01-28 01:07:40]

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

[commit-bot: I haz the power, 2017-01-28 01:08:23]

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-01-28 01:13:51]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485565660689940,
"parent_rev": "57e530fab3e3d9be05de503a6066bb89d5741bf1", "commit_rev":
"5e465202abc260c9aa8319f8e77bf721e0453ed2"}

[commit-bot: I haz the power, 2017-01-28 01:15:36]

Description was changed from

==========
Better crash stacktraces for ExtensionFunction bad messages.

Record crash trace right where we set bad_message in ExtensionFunction

This will give us clue about which particular extension functions are
crashing. Previously, we used to record the trace from common response
callback where we already lost track of which function actually set
a bad_message.

Turn ExtensionFunction::set_bad_message(bool) to EF::SetBadMessage(void)
since one should only use this function to set bad_message state, not
clear it.

BUG=642794
Test=No visible change. Beware though, similar to
https://codereview.chromium.org/2629783002, this might show up as a
blame CL for extension function crashes. These will not really
be new crashes, so file a (non-release-blocking) bug.
==========

to

==========
Better crash stacktraces for ExtensionFunction bad messages.

Record crash trace right where we set bad_message in ExtensionFunction

This will give us clue about which particular extension functions are
crashing. Previously, we used to record the trace from common response
callback where we already lost track of which function actually set
a bad_message.

Turn ExtensionFunction::set_bad_message(bool) to EF::SetBadMessage(void)
since one should only use this function to set bad_message state, not
clear it.

BUG=642794
Test=No visible change. Beware though, similar to
https://codereview.chromium.org/2629783002, this might show up as a
blame CL for extension function crashes. These will not really
be new crashes, so file a (non-release-blocking) bug.

Review-Url: https://codereview.chromium.org/2656013005
Cr-Commit-Position: refs/heads/master@{#446865}
Committed:
https://chromium.googlesource.com/chromium/src/+/5e465202abc260c9aa8319f8e77b...
==========

[commit-bot: I haz the power, 2017-01-28 01:15:40]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/5e465202abc260c9aa8319f8e77b...