Fix CPM casting issue; refactor renderer crash events

Fix CPM casting issue; refactor renderer crash events

Fix issue 152275 by removing the cast.  Combine
the now-identical RendererCrash and RendererFreeze events, and rename them for readability/consistency.

BUG=152275, 130212
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159844

[Yoyo Zhou, 2012-09-27 19:18:27]

Have you seen WebContents::FromRenderViewHost?

http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/memory_deta...

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
File chrome/browser/performance_monitor/event.h (right):

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/event.h:21: // Used only to indicate invalid
events and to generate database keys.
I don't think this is needed.

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/event.h:44: // Used only to indicate the
number of events (including the undefined type)
I don't think you need this comment.

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
File chrome/browser/performance_monitor/metric.h (right):

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/metric.h:20: // Used only to indicate invalid
metrics and to generate database keys.
ditto

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/metric.h:30: // in sequence). Test startup
time refers to startup times which are not the
I'd write "refers to non-normal startups, e.g."

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/metric.h:40: // Used only to indicate the
number of metrics (including the undefined type)
ditto

[Devlin, 2012-09-28 02:33:23]

On 2012/09/27 19:18:27, Yoyo Zhou wrote:
> Have you seen WebContents::FromRenderViewHost?

I had, but I mistakenly thought that would create a new WebContents from the
header. With that, we can also bring url back for the other crash events (though
it takes a bit more). Thanks for the tip! :)

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
File chrome/browser/performance_monitor/event.h (right):

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/event.h:21: // Used only to indicate invalid
events and to generate database keys.
On 2012/09/27 19:18:27, Yoyo Zhou wrote:
> I don't think this is needed.

Done.

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/event.h:44: // Used only to indicate the
number of events (including the undefined type)
On 2012/09/27 19:18:27, Yoyo Zhou wrote:
> I don't think you need this comment.

Done.

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
File chrome/browser/performance_monitor/metric.h (right):

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/metric.h:20: // Used only to indicate invalid
metrics and to generate database keys.
On 2012/09/27 19:18:27, Yoyo Zhou wrote:
> ditto

Done.

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/metric.h:30: // in sequence). Test startup
time refers to startup times which are not the
On 2012/09/27 19:18:27, Yoyo Zhou wrote:
> I'd write "refers to non-normal startups, e.g."

Done.

https://codereview.chromium.org/10987040/diff/5001/chrome/browser/performance...
chrome/browser/performance_monitor/metric.h:40: // Used only to indicate the
number of metrics (including the undefined type)
On 2012/09/27 19:18:27, Yoyo Zhou wrote:
> ditto

Done.

[Yoyo Zhou, 2012-09-29 01:19:25]

LGTM with nits

https://chromiumcodereview.appspot.com/10987040/diff/7003/chrome/browser/perf...
File chrome/browser/performance_monitor/performance_monitor.cc (right):

https://chromiumcodereview.appspot.com/10987040/diff/7003/chrome/browser/perf...
chrome/browser/performance_monitor/performance_monitor.cc:599: if (url.size())
I think !url.empty() is better.

https://chromiumcodereview.appspot.com/10987040/diff/7003/chrome/browser/perf...
File chrome/browser/performance_monitor/performance_monitor_util.cc (right):

https://chromiumcodereview.appspot.com/10987040/diff/7003/chrome/browser/perf...
chrome/browser/performance_monitor/performance_monitor_util.cc:58: LOG(WARNING)
<< "URL: " << url;
Won't this be spammy? Plus it's getting logged as an event anyway.

[Devlin, 2012-10-02 23:34:32]

https://codereview.chromium.org/10987040/diff/7003/chrome/browser/performance...
File chrome/browser/performance_monitor/performance_monitor.cc (right):

https://codereview.chromium.org/10987040/diff/7003/chrome/browser/performance...
chrome/browser/performance_monitor/performance_monitor.cc:599: if (url.size())
On 2012/09/29 01:19:25, Yoyo Zhou wrote:
> I think !url.empty() is better.

Done.

https://codereview.chromium.org/10987040/diff/7003/chrome/browser/performance...
File chrome/browser/performance_monitor/performance_monitor_util.cc (right):

https://codereview.chromium.org/10987040/diff/7003/chrome/browser/performance...
chrome/browser/performance_monitor/performance_monitor_util.cc:58: LOG(WARNING)
<< "URL: " << url;
On 2012/09/29 01:19:25, Yoyo Zhou wrote:
> Won't this be spammy? Plus it's getting logged as an event anyway.

Whoops; that wasn't supposed to stay in....

[commit-bot: I haz the power, 2012-10-02 23:34:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/10987040/1...

[commit-bot: I haz the power, 2012-10-03 05:51:13]

Change committed as 159844