Set active extension IDs for crash reports using the crash key logging system.

Set active extension IDs for crash reports using the crash key logging system.

BUG=77656
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221867

[Robert Sesek, 2013-09-06 20:20:59]

mark: the one-line change in base/
thestig: generally more of the same cleanup

https://codereview.chromium.org/23471007/diff/27001/base/debug/crash_logging.h
File base/debug/crash_logging.h (left):

https://codereview.chromium.org/23471007/diff/27001/base/debug/crash_logging....
base/debug/crash_logging.h:59: const char* const key_name;
Not a huge fan of this, but it's necessary for it to be copyable for use in
std::vector.

[Robert Sesek, 2013-09-06 20:21:10]

mark: the one-line change in base/
thestig: generally more of the same cleanup

[Mark Mentovai, 2013-09-06 20:50:05]

LGTM in base.

https://codereview.chromium.org/23471007/diff/27001/base/debug/crash_logging.h
File base/debug/crash_logging.h (left):

https://codereview.chromium.org/23471007/diff/27001/base/debug/crash_logging....
base/debug/crash_logging.h:59: const char* const key_name;
rsesek wrote:
> Not a huge fan of this, but it's necessary for it to be copyable for use in
> std::vector.

Yup, vector sucks like that.

[Lei Zhang, 2013-09-06 21:15:22]

lgtm

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys.cc
File chrome/common/crash_keys.cc (right):

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys....
chrome/common/crash_keys.cc:168: base::debug::SetCrashKeyValue(key, *(it++));
Pre-increment, even if that means adding another line and some more braces?

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys.h
File chrome/common/crash_keys.h (right):

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys....
chrome/common/crash_keys.h:32: // Installed extensions. kExtensionID should be
formatted with an integer,
nit: |kExtensionID|

[Mark Mentovai, 2013-09-06 21:21:35]

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys.cc
File chrome/common/crash_keys.cc (right):

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys....
chrome/common/crash_keys.cc:168: base::debug::SetCrashKeyValue(key, *(it++));
Lei Zhang wrote:
> Pre-increment, even if that means adding another line and some more braces?

Why?

[Lei Zhang, 2013-09-06 21:23:06]

On 2013/09/06 21:21:35, Mark Mentovai wrote:
>
https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys.cc
> File chrome/common/crash_keys.cc (right):
> 
>
https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys....
> chrome/common/crash_keys.cc:168: base::debug::SetCrashKeyValue(key, *(it++));
> Lei Zhang wrote:
> > Pre-increment, even if that means adding another line and some more braces?
> 
> Why?

C++ style guide says to do so to avoid copying.

[Robert Sesek, 2013-09-06 21:32:01]

Thanks!

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys.cc
File chrome/common/crash_keys.cc (right):

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys....
chrome/common/crash_keys.cc:168: base::debug::SetCrashKeyValue(key, *(it++));
On 2013/09/06 21:15:22, Lei Zhang wrote:
> Pre-increment, even if that means adding another line and some more braces?

Done.

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys.h
File chrome/common/crash_keys.h (right):

https://codereview.chromium.org/23471007/diff/27001/chrome/common/crash_keys....
chrome/common/crash_keys.h:32: // Installed extensions. kExtensionID should be
formatted with an integer,
On 2013/09/06 21:15:22, Lei Zhang wrote:
> nit: |kExtensionID|

Done.

[commit-bot: I haz the power, 2013-09-06 21:32:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/23471007/43001

[commit-bot: I haz the power, 2013-09-06 21:42:45]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[commit-bot: I haz the power, 2013-09-06 21:43:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/23471007/43001

[commit-bot: I haz the power, 2013-09-07 02:53:53]

Change committed as 221867