CHECK tasks posted by TaskRunner::PostTask

CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.

After this CL, the crash happens earlier and the crash report will
have meaningful trace.

BUG=711167, 709149
Review-Url: https://codereview.chromium.org/2815573002
Cr-Commit-Position: refs/heads/master@{#464333}
Committed: https://chromium.googlesource.com/chromium/src/+/498d42bc7c665044b713fc9cbc86b459d58f0b50

[tzik, 2017-04-11 05:14:21]

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

[commit-bot: I haz the power, 2017-04-11 05:14:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2017-04-11 05:20:31]

Description was changed from

==========
CHECK tasks posted by TaskRunner::PostTask

BUG=
==========

to

==========
CHECK tasks posted by TaskRunner::PostTask
==========

[tzik, 2017-04-11 05:28:21]

Description was changed from

==========
CHECK tasks posted by TaskRunner::PostTask
==========

to

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.
==========

[tzik, 2017-04-11 05:28:21]

tzik@chromium.org changed reviewers:
+ gab@chromium.org, kinuko@chromium.org, sky@chromium.org, skyostil@chromium.org

[tzik, 2017-04-11 05:29:28]

Description was changed from

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.
==========

to

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.

After this CL, the crash happens earlier and the crash report will
have meaningful trace.
==========

[tzik, 2017-04-11 05:45:15]

This is a reraise of the previous DCHECK addition last week.

As that crash happens again and again, and stability sheriffs does never look
into the local variable in the minidump to identify the culprit. I'd like to use
CHECK instead of DCHECK persistently rather than temporarily.

all: Do you guys have any concern about using CHECK instead of DCHECK?
I believe this doesn't hurt performance, binary size nor complexity.

PTAL to:
gab: //base
sky: //chrome
kinuko: //content
skyostil: task_queue_impl.cc

[commit-bot: I haz the power, 2017-04-11 06:49:04]

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

[commit-bot: I haz the power, 2017-04-11 06:49:04]

Dry run: This issue passed the CQ dry run.

[kinuko, 2017-04-11 14:26:04]

On 2017/04/11 05:45:15, tzik wrote:
> This is a reraise of the previous DCHECK addition last week.
> 
> As that crash happens again and again, and stability sheriffs does never look
> into the local variable in the minidump to identify the culprit. I'd like to
use
> CHECK instead of DCHECK persistently rather than temporarily.
> 
> all: Do you guys have any concern about using CHECK instead of DCHECK?
> I believe this doesn't hurt performance, binary size nor complexity.
> 
> PTAL to:
> gab: //base
> sky: //chrome
> kinuko: //content
> skyostil: task_queue_impl.cc

Thanks for working on the tough crashes.

I'm fine with landing this, but it doesn't seem clear if we need this
permanently yet.  At some point those mysterious bugs in existing code should
cease to happen and blame range will become more appropriate?

[sky, 2017-04-11 17:20:32]

LGTM

[sky, 2017-04-11 17:21:00]

Could you also add a pointer to the bug where you are adding CHECKs so that we
know why it's a CHECK.

[Sami, 2017-04-11 17:23:21]

lgtm.

[kinuko, 2017-04-12 08:31:13]

On 2017/04/11 17:21:00, sky wrote:
> Could you also add a pointer to the bug where you are adding CHECKs so that we
> know why it's a CHECK.

Well, I think this lgtm if you add the pointer (so that people can find out the
context)

[gab, 2017-04-12 18:49:29]

On 2017/04/12 08:31:13, kinuko wrote:
> On 2017/04/11 17:21:00, sky wrote:
> > Could you also add a pointer to the bug where you are adding CHECKs so that
we
> > know why it's a CHECK.
> 
> Well, I think this lgtm if you add the pointer (so that people can find out
the
> context)

//base lgtm w/ bug # added to CL desc.

[tzik, 2017-04-13 06:03:55]

Description was changed from

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.

After this CL, the crash happens earlier and the crash report will
have meaningful trace.
==========

to

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.

After this CL, the crash happens earlier and the crash report will
have meaningful trace.

BUG=711167, 709149
==========

[tzik, 2017-04-13 06:05:31]

On 2017/04/12 18:49:29, gab wrote:
> On 2017/04/12 08:31:13, kinuko wrote:
> > On 2017/04/11 17:21:00, sky wrote:
> > > Could you also add a pointer to the bug where you are adding CHECKs so
that
> we
> > > know why it's a CHECK.
> > 
> > Well, I think this lgtm if you add the pointer (so that people can find out
> the
> > context)
> 
> //base lgtm w/ bug # added to CL desc.

OK, I filed a bug for it and added comments as the pointer. Plus add it and the
original issue to the CL description.

[tzik, 2017-04-13 06:05:38]

The CQ bit was checked by tzik@chromium.org

[tzik, 2017-04-13 06:05:39]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
gab@chromium.org, skyostil@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2815573002/#ps20001
(title: "+comment")

[commit-bot: I haz the power, 2017-04-13 06:05:56]

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, 2017-04-13 07:43:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492063538642330,
"parent_rev": "e4b9a821a3e13d5bc49431173664dfd24ae3fb59", "commit_rev":
"498d42bc7c665044b713fc9cbc86b459d58f0b50"}

[commit-bot: I haz the power, 2017-04-13 07:43:58]

Description was changed from

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.

After this CL, the crash happens earlier and the crash report will
have meaningful trace.

BUG=711167, 709149
==========

to

==========
CHECK tasks posted by TaskRunner::PostTask

This CL is a reraise of http://crrev.com/2785943004 PS4.

A null callback posted to PostTask family eventually causes a null pointer
access crash. And its crash report doesn't indicate the culprit directly
without looking into the local variable of the minidump.

After this CL, the crash happens earlier and the crash report will
have meaningful trace.

BUG=711167, 709149

Review-Url: https://codereview.chromium.org/2815573002
Cr-Commit-Position: refs/heads/master@{#464333}
Committed:
https://chromium.googlesource.com/chromium/src/+/498d42bc7c665044b713fc9cbc86...
==========

[commit-bot: I haz the power, 2017-04-13 07:44:05]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/498d42bc7c665044b713fc9cbc86...