Delete redundant MobileRendererCrashed count.

Delete redundant MobileRendererCrashed count.

Since UmaSessionStats.logRendererCrash is only called if
the activityState is not PAUSED, STOPPED, or DESTROYED, its
redundant boolean is_paused will always be false, thus
this count is the same value as
kStabilityRendererCrashCount, thus making it redundant.

BUG=510327
Committed: https://crrev.com/7e0c45b2f63b8d2cab47c3830ce6301067e36564
Cr-Commit-Position: refs/heads/master@{#348700}

[Peter Wen, 2015-09-03 17:56:16]

wnwen@chromium.org changed reviewers:
+ acleung@chromium.org, jaekyun@chromium.org

[Peter Wen, 2015-09-03 17:56:17]

Hi Alan and Jaekyun,

While compiling stats for http://go/clank-renderer-crash, Yaron and I found that
MobileRendererCrashed was counting the same value as
kStabilityRendererCrashCount.

Also, incrementing kStabilityRendererCrashCount is necessary even though it
should be the same number as "Shown in foreground app", since this way we can
correlate histogram values (which only tell us about renderers that crashed)
with stability crash rates (which tell us how many renderer crashed overall).

Thanks,

Peter

[acleung1, 2015-09-03 18:40:35]

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:593:
rendererCrashStatus = TAB_RENDERER_CRASH_STATUS_SHOWN_IN_FOREGROUND_APP;
Can a render process be OOM protected while the activity is not in foreground
and still get OOM killed?

Is it possible where we have a case that Clank is playing music in the
background while getting OOM killed?

[Jaekyun Seok (inactive), 2015-09-03 23:21:02]

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:593:
rendererCrashStatus = TAB_RENDERER_CRASH_STATUS_SHOWN_IN_FOREGROUND_APP;
On 2015/09/03 18:40:35, acleung1 wrote:
> Can a render process be OOM protected while the activity is not in foreground
> and still get OOM killed?

That is possible because Chrome itself could be killed by OOM killer in
background.

> 
> Is it possible where we have a case that Clank is playing music in the
> background while getting OOM killed?

[Peter Wen, 2015-09-08 18:07:11]

Any concerns about this CL to remove redundant counting? Talking to Robert from
uma-team, since stability counts are universal across platforms and are more
crash resistant, he recommends we keep both stability and histogram as specified
in this CL.

Thanks.

[acleung1, 2015-09-08 19:49:23]

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:593:
rendererCrashStatus = TAB_RENDERER_CRASH_STATUS_SHOWN_IN_FOREGROUND_APP;
On 2015/09/03 23:21:02, Jaekyun Seok wrote:
> On 2015/09/03 18:40:35, acleung1 wrote:
> > Can a render process be OOM protected while the activity is not in
foreground
> > and still get OOM killed?
> 
> That is possible because Chrome itself could be killed by OOM killer in
> background.
> 
> > 
> > Is it possible where we have a case that Clank is playing music in the
> > background while getting OOM killed?
> 

If that's the case wouldn't this be incorrectly logged as crashed?

[Peter Wen, 2015-09-08 20:06:30]

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:593:
rendererCrashStatus = TAB_RENDERER_CRASH_STATUS_SHOWN_IN_FOREGROUND_APP;
On 2015/09/08 19:49:23, acleung1 wrote:
> On 2015/09/03 23:21:02, Jaekyun Seok wrote:
> > That is possible because Chrome itself could be killed by OOM killer in
> > background.
> If that's the case wouldn't this be incorrectly logged as crashed?

I don't quite understand what you mean. If the browser process gets OOM killed
would this code even run?

If the activity is not in the foreground or if the renderer process was not OOM
protected, we would fall into the earlier if block and not log a renderer crash
in stability.

[acleung1, 2015-09-08 20:10:47]

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:593:
rendererCrashStatus = TAB_RENDERER_CRASH_STATUS_SHOWN_IN_FOREGROUND_APP;
On 2015/09/08 20:06:30, Peter Wen wrote:
> On 2015/09/08 19:49:23, acleung1 wrote:
> > On 2015/09/03 23:21:02, Jaekyun Seok wrote:
> > > That is possible because Chrome itself could be killed by OOM killer in
> > > background.
> > If that's the case wouldn't this be incorrectly logged as crashed?
> 
> I don't quite understand what you mean. If the browser process gets OOM killed
> would this code even run?
> 
> If the activity is not in the foreground or if the renderer process was not
OOM
> protected, we would fall into the earlier if block and not log a renderer
crash
> in stability.

My concern was for the first question.

Can a render process be OOM protected while not in foreground and get OOM
killed. Jaekyun's reply was yes so wouldn't this make a invalid call to
logRendereerCrash?

[Peter Wen, 2015-09-08 20:17:43]

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1313503004/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:593:
rendererCrashStatus = TAB_RENDERER_CRASH_STATUS_SHOWN_IN_FOREGROUND_APP;
On 2015/09/08 20:10:47, acleung1 wrote:
> My concern was for the first question.
> 
> Can a render process be OOM protected while not in foreground and get OOM
> killed. Jaekyun's reply was yes so wouldn't this make a invalid call to
> logRendereerCrash?

Hmm... even if the renderer process is OOM protected, if the activity is not in
the foreground, the first if block will be executed since the activityState
conditions are ||'ed, and so we will not log a renderer crash. Perhaps I am
misunderstanding?

[Peter Wen, 2015-09-10 21:55:41]

Friendly ping. :)

[acleung1, 2015-09-10 22:00:26]

On 2015/09/10 21:55:41, Peter Wen wrote:
> Friendly ping. :)

Ok. Yeah. That make sense. LGTM.

[Peter Wen, 2015-09-14 13:49:24]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2015-09-14 13:49:25]

The CQ bit was unchecked by wnwen@chromium.org

[Peter Wen, 2015-09-14 13:51:40]

wnwen@chromium.org changed reviewers:
+ mariakhomenko@chromium.org

[Peter Wen, 2015-09-14 13:51:41]

+mariakhomenko for OWNERs android and metrics.

[Maria, 2015-09-14 18:23:27]

lgtm

[Peter Wen, 2015-09-14 18:28:27]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2015-09-14 18:29:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313503004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313503004/1

[commit-bot: I haz the power, 2015-09-14 18:40:52]

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

[commit-bot: I haz the power, 2015-09-14 18:40:53]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Peter Wen, 2015-09-14 19:25:45]

The CQ bit was checked by wnwen@chromium.org

[Peter Wen, 2015-09-14 19:25:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
acleung@chromium.org, mariakhomenko@chromium.org
Link to the patchset: https://codereview.chromium.org/1313503004/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2015-09-14 19:26:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313503004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313503004/20001

[commit-bot: I haz the power, 2015-09-14 20:14:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-14 20:15:09]

Patchset 2 (id:??) landed as
https://crrev.com/7e0c45b2f63b8d2cab47c3830ce6301067e36564
Cr-Commit-Position: refs/heads/master@{#348700}

[commit-bot: I haz the power, 2015-09-23 12:36:31]

Patchset 2 (id:??) landed as
https://crrev.com/7e0c45b2f63b8d2cab47c3830ce6301067e36564
Cr-Commit-Position: refs/heads/master@{#348700}