Reorganizing few DCHECKs for better catching of failure.

Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

BUG=

Committed: https://crrev.com/67a627d5b5687ac16a66bb92b76cac1bb416124e
Cr-Commit-Position: refs/heads/master@{#407201}

[AKV, 2016-07-21 13:29:28]

ajith.v@chromium.org changed reviewers:
+ sievers@chromium.org

[AKV, 2016-07-21 13:29:28]

PTAL!

[no sievers, 2016-07-21 19:08:01]

https://codereview.chromium.org/2168093002/diff/1/content/browser/browser_thr...
File content/browser/browser_thread_impl.cc (right):

https://codereview.chromium.org/2168093002/diff/1/content/browser/browser_thr...
content/browser/browser_thread_impl.cc:296: DCHECK(identifier_ >= 0);
nit: you can even do DCHECK_GE(identifier, 0) which will additionally print the
value of |identifier_| if it fails.

Similarly for DCHECK_LT(identifier_, ID_COUNT);

https://codereview.chromium.org/2168093002/diff/1/content/browser/browser_thr...
content/browser/browser_thread_impl.cc:560: DCHECK(!old_pointer);
This is wrong. The current code basically checks that if you pass a non-null
delegate, you can't have an old_pointer (previously registered).

https://codereview.chromium.org/2168093002/diff/1/content/browser/download/sa...
File content/browser/download/save_item.cc (right):

https://codereview.chromium.org/2168093002/diff/1/content/browser/download/sa...
content/browser/download/save_item.cc:90: DCHECK(!is_success_);
same here, cannot be split.

[AKV, 2016-07-22 14:26:13]

PTAL!

https://codereview.chromium.org/2168093002/diff/1/content/browser/browser_thr...
File content/browser/browser_thread_impl.cc (right):

https://codereview.chromium.org/2168093002/diff/1/content/browser/browser_thr...
content/browser/browser_thread_impl.cc:296: DCHECK(identifier_ >= 0);
On 2016/07/21 19:08:01, sievers wrote:
> nit: you can even do DCHECK_GE(identifier, 0) which will additionally print
the
> value of |identifier_| if it fails.
> 
> Similarly for DCHECK_LT(identifier_, ID_COUNT);

Done. Thanks, that's very helpful in catching failures.

https://codereview.chromium.org/2168093002/diff/1/content/browser/browser_thr...
content/browser/browser_thread_impl.cc:560: DCHECK(!old_pointer);
On 2016/07/21 19:08:01, sievers wrote:
> This is wrong. The current code basically checks that if you pass a non-null
> delegate, you can't have an old_pointer (previously registered).

Done. Thanks.

https://codereview.chromium.org/2168093002/diff/1/content/browser/download/sa...
File content/browser/download/save_item.cc (right):

https://codereview.chromium.org/2168093002/diff/1/content/browser/download/sa...
content/browser/download/save_item.cc:90: DCHECK(!is_success_);
On 2016/07/21 19:08:01, sievers wrote:
> same here, cannot be split.

Done.

[AKV, 2016-07-22 15:22:40]

Description was changed from

==========
Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

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

to

==========
Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

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

[no sievers, 2016-07-22 17:11:53]

lgtm

[AKV, 2016-07-22 17:13:34]

The CQ bit was checked by ajith.v@chromium.org

[commit-bot: I haz the power, 2016-07-22 17:13:51]

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-22 18:05:25]

Description was changed from

==========
Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

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

to

==========
Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

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

[commit-bot: I haz the power, 2016-07-22 18:05:26]

Committed patchset #2 (id:20001)

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

Description was changed from

==========
Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

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

to

==========
Reorganizing few DCHECKs for better catching of failrue.

In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.

BUG=

Committed: https://crrev.com/67a627d5b5687ac16a66bb92b76cac1bb416124e
Cr-Commit-Position: refs/heads/master@{#407201}
==========

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

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