Add an histogram to differentiate between hang types of the renderer

Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is happening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/639fdffbabc7a64ba11160c923af46b18d945650
Cr-Commit-Position: refs/heads/master@{#406039}

[clamy, 2016-07-12 16:50:21]

Description was changed from

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is haapening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
==========

to

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is haapening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-07-12 16:56:26]

Description was changed from

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is haapening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is haapening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-07-12 16:56:26]

clamy@chromium.org changed reviewers:
+ nasko@chromium.org

[clamy, 2016-07-12 16:57:22]

clamy@chromium.org changed reviewers:
+ asvitkine@chromium.org

[clamy, 2016-07-12 16:57:50]

@nasko: PTAL
@asvitkine: PTAL at the changes in histograms

[nasko, 2016-07-12 17:18:03]

s/haapening/happening/ in the CL description.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_delegate.h (right):

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_delegate.h:136: // Enum used in
histograms to differentiate between the different
nit: Enum is redundant, as it is obvious from the type definition below.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_delegate.h:139: // Only add
values at the end, do not delete values.
Does that mean we will keep this enum/UMA forever? Is it possible to use some
other mechanism which we aren't stuck with? Maybe DumpWithoutCrashing?

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.h:292:
RenderWidgetHostDelegate::RendererUnresponsiveType type);
Shouldn't this be the hang monitor reason or type? Unresponsive is a state that
is gotten into later on, not at the start of the monitor.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.h:822: // Stores the
reason the hang_monitor_timeout_ i sbeing used, to report
nit: "is being" or "has been started. Used to ..."

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.h:824:
RenderWidgetHostDelegate::RendererUnresponsiveType hang_type_;
nit: hang_monitor_type_ or hang_monitor_reason_?

[Alexei Svitkine (slow), 2016-07-12 18:36:41]

lgtm

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_delegate.h (right):

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_delegate.h:139: // Only add
values at the end, do not delete values.
On 2016/07/12 17:18:02, nasko wrote:
> Does that mean we will keep this enum/UMA forever? Is it possible to use some
> other mechanism which we aren't stuck with? Maybe DumpWithoutCrashing?

You can always delete histograms. That's fine.

The point is you can't renumber an enum or re-use enum values (without at least
renaming the histogram), otherwise values will be inconsistent between versions.

[clamy, 2016-07-13 10:02:01]

Description was changed from

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is haapening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is happening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[clamy, 2016-07-13 10:02:22]

Thanks! The description was updated.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_delegate.h (right):

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_delegate.h:136: // Enum used in
histograms to differentiate between the different
On 2016/07/12 17:18:02, nasko wrote:
> nit: Enum is redundant, as it is obvious from the type definition below.

Done.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_delegate.h:139: // Only add
values at the end, do not delete values.
On 2016/07/12 18:36:40, Alexei Svitkine (slow) wrote:
> On 2016/07/12 17:18:02, nasko wrote:
> > Does that mean we will keep this enum/UMA forever? Is it possible to use
some
> > other mechanism which we aren't stuck with? Maybe DumpWithoutCrashing?

I'm not sure how DumpWithoutCrashing works, and it's not very clear from the
description either. I need reports from the field on the distribution of the
different types of renderer hang, to see where the huge spike we're seeing is
coming from.

> 
> You can always delete histograms. That's fine.
> 
> The point is you can't renumber an enum or re-use enum values (without at
least
> renaming the histogram), otherwise values will be inconsistent between
versions.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_impl.h (right):

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.h:292:
RenderWidgetHostDelegate::RendererUnresponsiveType type);
On 2016/07/12 17:18:02, nasko wrote:
> Shouldn't this be the hang monitor reason or type? Unresponsive is a state
that
> is gotten into later on, not at the start of the monitor.

Done.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.h:822: // Stores the
reason the hang_monitor_timeout_ i sbeing used, to report
On 2016/07/12 17:18:03, nasko wrote:
> nit: "is being" or "has been started. Used to ..."

Done.

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_impl.h:824:
RenderWidgetHostDelegate::RendererUnresponsiveType hang_type_;
On 2016/07/12 17:18:02, nasko wrote:
> nit: hang_monitor_type_ or hang_monitor_reason_?

Done.

[nasko, 2016-07-15 17:32:08]

LGTM

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host_delegate.h (right):

https://codereview.chromium.org/2136173003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host_delegate.h:139: // Only add
values at the end, do not delete values.
On 2016/07/13 10:02:21, clamy (ooo until Jul 18) wrote:
> On 2016/07/12 18:36:40, Alexei Svitkine (slow) wrote:
> > On 2016/07/12 17:18:02, nasko wrote:
> > > Does that mean we will keep this enum/UMA forever? Is it possible to use
> some
> > > other mechanism which we aren't stuck with? Maybe DumpWithoutCrashing?
> 
> I'm not sure how DumpWithoutCrashing works, and it's not very clear from the
> description either. I need reports from the field on the distribution of the
> different types of renderer hang, to see where the huge spike we're seeing is
> coming from.

DumpWithoutCrashing literally does what it says - generates a crash report, as
thought we had a crash at the line we call it on, but doesn't really crash the
process itself. It is useful tool to debug issues that are hard to understand,
but it does add overhead.

> > You can always delete histograms. That's fine.
> > 
> > The point is you can't renumber an enum or re-use enum values (without at
> least
> > renaming the histogram), otherwise values will be inconsistent between
> versions.

Ah, good to know. Thanks!
Let's use a histogram then, as it doesn't have much overhead and we might want
to keep it for a while.

[nasko, 2016-07-15 17:32:25]

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

[commit-bot: I haz the power, 2016-07-15 17:33:02]

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-07-15 17:33:03]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-15 17:42:23]

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

[commit-bot: I haz the power, 2016-07-15 17:42:24]

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

[clamy, 2016-07-18 13:34:32]

The CQ bit was checked by clamy@chromium.org

[clamy, 2016-07-18 13:34:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2136173003/#ps40001
(title: "Rebase + fixed compilation error in unit tests")

[commit-bot: I haz the power, 2016-07-18 13:34: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, 2016-07-18 13:34:51]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-18 15:20:43]

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

[commit-bot: I haz the power, 2016-07-18 15:20:45]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[clamy, 2016-07-18 16:39:50]

The CQ bit was checked by clamy@chromium.org

[clamy, 2016-07-18 16:39:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2136173003/#ps60001
(title: "Fixed asan issue")

[commit-bot: I haz the power, 2016-07-18 16:40:22]

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-07-18 16:40:24]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.chromium.linux
   For more details, see http://crbug.com/617627.

[commit-bot: I haz the power, 2016-07-18 18:13:48]

Description was changed from

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is happening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is happening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-07-18 18:13:49]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-18 18:14:01]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-18 18:16:38]

Description was changed from

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is happening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add an histogram to differentiate between hang types of the renderer

We've seen a sharp augmentation of renderer hangs on Android. This histogram is
meant to help identify the root cause of this renderer hangs, since it's
currently hard to distinguish whether this is happening during normal browsing
or during beforeunload/unload/when the page is closing.

BUG=621303
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/639fdffbabc7a64ba11160c923af46b18d945650
Cr-Commit-Position: refs/heads/master@{#406039}
==========

[commit-bot: I haz the power, 2016-07-18 18:16:39]

Patchset 4 (id:??) landed as
https://crrev.com/639fdffbabc7a64ba11160c923af46b18d945650
Cr-Commit-Position: refs/heads/master@{#406039}