ThreadWatcher - On Android, don't generate the crash dump if

ThreadWatcher - On Android, don't generate the crash dump if
StartupTimebomb goes off.

When StartupTimebomb goes off, collect the duration stats (difference
between when the timebomb went off and when it is started). We collect
this duration in terms of clock time, montonic clock time and thread CPU
time.

We wanted to see when the browser process is stopped, does
TimeTicks::Now() account for the time the browser process was stopped or
not. If the TimeTicks doesn't account this time properly, then we could
have false positive jank related crashes.

BUG=366699
R=jar@chromium.org

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

[jar (doing other things), 2014-05-03 02:00:21]

A nit/suggestion, and one question to confirm... then LGTM.

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
File chrome/browser/metrics/thread_watcher.cc (left):

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
chrome/browser/metrics/thread_watcher.cc:895:
base::Bind(&base::debug::DumpWithoutCrashing));
Is this a deliberate change to crashing?  Did you turn off crashing... and now
this will turn it back on for non-Android?

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
File chrome/browser/metrics/thread_watcher.cc (right):

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
chrome/browser/metrics/thread_watcher.cc:918: #ifndef NDEBUG
nit: I like less ifdef protected, and especially nested, blocks.  One small
change that might help is to change:

#else
#ifndef NDEBUG

into

#elif !defined(NDEBUG)


Maybe even nicer would be list the three block, each with a return, and to list
the smallest blocks first (for easy reading).  Something like

#if !defined(NDEBUG)
  DCHECK(false);
  return;
#elif !defined(OS_ANDROID)
  PostTask(to crash);
  return;
#else  // Android release: gather stats to figure out when to crash.
  UMA_HISTOGRAMS(...)
  return;
#endif

[ramant (doing other things), 2014-05-03 04:11:36]

PTAL. thanks.

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
File chrome/browser/metrics/thread_watcher.cc (left):

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
chrome/browser/metrics/thread_watcher.cc:895:
base::Bind(&base::debug::DumpWithoutCrashing));
On 2014/05/03 02:00:21, jar wrote:
> Is this a deliberate change to crashing?  Did you turn off crashing... and now
> this will turn it back on for non-Android?

In DEBUG builds we used to crash and we do the same.

For non DEBUG builds, instead of calling DumpWithoutCrashing directly, called
StartupCrash which calls DumpWithoutCrashing. This would show StartupCrash in
callstack (it would help in isolating if there are other watchdog based
crashes).

void NullPointerCrash(int line_number) {
#ifndef NDEBUG
  *NullPointer() = line_number;  // Crash.
#else
  base::debug::DumpWithoutCrashing();
#endif
}

NOINLINE void StartupCrash() {
  NullPointerCrash(__LINE__);
}

WDYT?

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
File chrome/browser/metrics/thread_watcher.cc (right):

https://codereview.chromium.org/269763002/diff/20001/chrome/browser/metrics/t...
chrome/browser/metrics/thread_watcher.cc:918: #ifndef NDEBUG
On 2014/05/03 02:00:21, jar wrote:
> nit: I like less ifdef protected, and especially nested, blocks.  One small
> change that might help is to change:
> 
> #else
> #ifndef NDEBUG
> 
> into
> 
> #elif !defined(NDEBUG)
> 
> 
> Maybe even nicer would be list the three block, each with a return, and to
list
> the smallest blocks first (for easy reading).  Something like
> 
> #if !defined(NDEBUG)
>   DCHECK(false);
>   return;
> #elif !defined(OS_ANDROID)
>   PostTask(to crash);
>   return;
> #else  // Android release: gather stats to figure out when to crash.
>   UMA_HISTOGRAMS(...)
>   return;
> #endif

Done.

[jar (doing other things), 2014-05-05 07:06:11]

lgtm

[commit-bot: I haz the power, 2014-05-05 07:06:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rtenneti@chromium.org/269763002/60001

[commit-bot: I haz the power, 2014-05-05 07:13:34]

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

[commit-bot: I haz the power, 2014-05-05 07:13:35]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium

[ramant (doing other things), 2014-05-05 13:39:40]

The CQ bit was checked by rtenneti@chromium.org

[commit-bot: I haz the power, 2014-05-05 13:40:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rtenneti@chromium.org/269763002/60001

[commit-bot: I haz the power, 2014-05-05 13:52:04]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
  android_dbg on tryserver.chromium

[commit-bot: I haz the power, 2014-05-05 13:59:33]

FYI, CQ is re-trying this CL (attempt #2).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
  android_dbg on tryserver.chromium

[commit-bot: I haz the power, 2014-05-05 14:07:41]

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

[commit-bot: I haz the power, 2014-05-05 14:07:42]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
  android_dbg on tryserver.chromium

[ramant (doing other things), 2014-05-05 17:01:37]

The CQ bit was checked by rtenneti@chromium.org

[commit-bot: I haz the power, 2014-05-05 17:02:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rtenneti@chromium.org/269763002/80001

[commit-bot: I haz the power, 2014-05-05 21:15:30]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
  win_chromium_x64_rel on tryserver.chromium

[commit-bot: I haz the power, 2014-05-06 00:26:31]

Change committed as 268338