Replace ASSERT by RTC_DCHECK in all non-test code.

Replace ASSERT by RTC_DCHECK in all non-test code.

Bulk of the changes were produced using

  git grep -l ' ASSERT(' | grep -v test | grep -v 'common\.h' |\
    xargs -n1 sed -i 's/ ASSERT(/ RTC_DCHECK(/'

followed by additional includes of base/checks.h in affected files,
and git cl format.

Also had to do some tweaks to #if !defined(NDEBUG) logic in the
taskrunner code (webrtc/base/task.cc, webrtc/base/taskparent.cc,
webrtc/base/taskparent.h, webrtc/base/taskrunner.cc), replaced to
consistently use RTC_DCHECK_IS_ON, and some of the checks needed
additional #if protection.

Test code was excluded, because it should probably use RTC_CHECK
rather than RTC_DCHECK.

BUG=webrtc:6424
Review-Url: https://codereview.webrtc.org/2620303003
Cr-Commit-Position: refs/heads/master@{#16030}
Committed: https://chromium.googlesource.com/external/webrtc/+/ede5da49606bf57888755058f354df240d37ef3c

[nisse-webrtc, 2017-01-11 15:15:18]

nisse@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[nisse-webrtc, 2017-01-11 15:15:19]

Next step...

[nisse-webrtc, 2017-01-11 15:49:15]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 15:49:32]

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

[commit-bot: I haz the power, 2017-01-11 17:02:43]

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

[commit-bot: I haz the power, 2017-01-11 17:02:44]

Dry run: This issue passed the CQ dry run.

[kwiberg-webrtc, 2017-01-12 02:10:41]

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/peerconnection.cc
File webrtc/api/peerconnection.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/peerconnection...
webrtc/api/peerconnection.cc:1947: RTC_DCHECK(false && "Invalid media type");
RTC_NOTREACHED with a message?

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/videocapturert...
File webrtc/api/videocapturertracksource.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/videocapturert...
webrtc/api/videocapturertracksource.cc:54: RTC_DCHECK(false && "GetReadyState
unknown state");
RTC_NOTREACHED?

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/webrtcsession.cc
File webrtc/api/webrtcsession.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/webrtcsession....
webrtc/api/webrtcsession.cc:947: RTC_DCHECK(false && "unknown action type");
RTC_NOTREACHED?

(This is the last one I'll comment on; you can find all of them easily with
grep.)

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/rollingaccumu...
File webrtc/base/rollingaccumulator.h (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/rollingaccumu...
webrtc/base/rollingaccumulator.h:102: "It shouldn't be possible for max_stale_
&& count_ == 0");
Remove the message, or use << on the DCHECK.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/rollingaccumu...
webrtc/base/rollingaccumulator.h:115: "It shouldn't be possible for min_stale_
&& count_ == 0");
Ditto.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc
File webrtc/base/taskrunner.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc...
webrtc/base/taskrunner.cc:27: ,
You may want to use default member initializers to avoid this syntax wart...

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
File webrtc/p2p/base/relayserver.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
webrtc/p2p/base/relayserver.cc:535: #if ENABLE_DEBUG
Should this be DCHECK_IS_ON?

[nisse-webrtc, 2017-01-12 10:53:24]

Comments addressed. Thanks for the review.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/webrtcsession.cc
File webrtc/api/webrtcsession.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/api/webrtcsession....
webrtc/api/webrtcsession.cc:947: RTC_DCHECK(false && "unknown action type");
On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> RTC_NOTREACHED?
> 
> (This is the last one I'll comment on; you can find all of them easily with
> grep.)

Done. Only three occurrences.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/rollingaccumu...
File webrtc/base/rollingaccumulator.h (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/rollingaccumu...
webrtc/base/rollingaccumulator.h:102: "It shouldn't be possible for max_stale_
&& count_ == 0");
On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> Remove the message, or use << on the DCHECK.

Done. Two occurrences.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc
File webrtc/base/taskrunner.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc...
webrtc/base/taskrunner.cc:27: ,
On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> You may want to use default member initializers to avoid this syntax wart...

Could make sense, but not in this cl.

Not sure if there's a consensus on using explicit constructor or initializers at
the declaration. At least, we shouldn't mix in the same class.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
File webrtc/p2p/base/relayserver.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
webrtc/p2p/base/relayserver.cc:535: #if ENABLE_DEBUG
On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> Should this be DCHECK_IS_ON?

It should. Fixed.

[nisse-webrtc, 2017-01-12 10:58:57]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 10:59:03]

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

[kwiberg-webrtc, 2017-01-12 11:00:38]

lgtm

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc
File webrtc/base/taskrunner.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc...
webrtc/base/taskrunner.cc:27: ,
On 2017/01/12 10:53:24, nisse-webrtc wrote:
> On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> > You may want to use default member initializers to avoid this syntax wart...
> 
> Could make sense, but not in this cl.
> 
> Not sure if there's a consensus on using explicit constructor or initializers
at
> the declaration. At least, we shouldn't mix in the same class. 

I believe you can use either (but preferably not a mix, as you say). And in this
case using default member initializers is clearly a win, since you can entirely
get rid of this #if block with the ugly comma issue.

Follow-up CL? :-)

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
File webrtc/p2p/base/relayserver.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
webrtc/p2p/base/relayserver.cc:535: #if ENABLE_DEBUG
On 2017/01/12 10:53:24, nisse-webrtc wrote:
> On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> > Should this be DCHECK_IS_ON?
> 
> It should. Fixed.

Actually, do you need it at all? DCHECK is carefully written so that you won't
get warnings about that constant not being used in non-debug mode, but ASSERT
wasn't.

[nisse-webrtc, 2017-01-12 11:09:42]

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc
File webrtc/base/taskrunner.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc...
webrtc/base/taskrunner.cc:27: ,
On 2017/01/12 11:00:38, kwiberg-webrtc wrote:
> On 2017/01/12 10:53:24, nisse-webrtc wrote:
> > On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> > > You may want to use default member initializers to avoid this syntax
wart...
> > 
> > Could make sense, but not in this cl.
> > 
> > Not sure if there's a consensus on using explicit constructor or
initializers
> at
> > the declaration. At least, we shouldn't mix in the same class. 
> 
> I believe you can use either (but preferably not a mix, as you say). And in
this
> case using default member initializers is clearly a win, since you can
entirely
> get rid of this #if block with the ugly comma issue.
> 
> Follow-up CL? :-)

I'll change to using initializers, to make it less ugly. But my preferred
followup cl is https://codereview.webrtc.org/2622923002/, deleting all of this,
it's totally unused in webrtc. Unfortunately, it's used by the xmpp code which
was recently moved from webrtc to chrome.

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
File webrtc/p2p/base/relayserver.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/p2p/base/relayserv...
webrtc/p2p/base/relayserver.cc:535: #if ENABLE_DEBUG
On 2017/01/12 11:00:38, kwiberg-webrtc wrote:
> On 2017/01/12 10:53:24, nisse-webrtc wrote:
> > On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> > > Should this be DCHECK_IS_ON?
> > 
> > It should. Fixed.
> 
> Actually, do you need it at all? DCHECK is carefully written so that you won't
> get warnings about that constant not being used in non-debug mode, but ASSERT
> wasn't.

Can probably be deleted, I'll give it a try.

[nisse-webrtc, 2017-01-12 11:10:00]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-01-12 11:10:00]

The patchset sent to the CQ was uploaded after l-g-t-m from kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2620303003/#ps100001 (title:
"Address final nits.")

[commit-bot: I haz the power, 2017-01-12 11:10:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[kwiberg-webrtc, 2017-01-12 12:54:31]

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc
File webrtc/base/taskrunner.cc (right):

https://codereview.webrtc.org/2620303003/diff/20001/webrtc/base/taskrunner.cc...
webrtc/base/taskrunner.cc:27: ,
On 2017/01/12 11:09:42, nisse-webrtc wrote:
> On 2017/01/12 11:00:38, kwiberg-webrtc wrote:
> > On 2017/01/12 10:53:24, nisse-webrtc wrote:
> > > On 2017/01/12 02:10:41, kwiberg-webrtc wrote:
> > > > You may want to use default member initializers to avoid this syntax
> wart...
> > > 
> > > Could make sense, but not in this cl.
> > > 
> > > Not sure if there's a consensus on using explicit constructor or
> initializers
> > at
> > > the declaration. At least, we shouldn't mix in the same class. 
> > 
> > I believe you can use either (but preferably not a mix, as you say). And in
> this
> > case using default member initializers is clearly a win, since you can
> entirely
> > get rid of this #if block with the ugly comma issue.
> > 
> > Follow-up CL? :-)
> 
> I'll change to using initializers, to make it less ugly. But my preferred
> followup cl is https://codereview.webrtc.org/2622923002/, deleting all of
this,
> it's totally unused in webrtc. Unfortunately, it's used by the xmpp code which
> was recently moved from webrtc to chrome.

Ah. Good that we're finally about to get serious with moving all supported stuff
to the api/ directory, then, so that we'll eventually stop running into this
sort of stuff.

[commit-bot: I haz the power, 2017-01-12 13:15:40]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484219400301870,
"parent_rev": "59e99b76da075e55d1f0204ecf9b2632a6593528", "commit_rev":
"ede5da49606bf57888755058f354df240d37ef3c"}

[commit-bot: I haz the power, 2017-01-12 13:15:43]

Description was changed from

==========
Replace ASSERT by RTC_DCHECK in all non-test code.

Bulk of the changes were produced using

  git grep -l ' ASSERT(' | grep -v test | grep -v 'common\.h' |\
    xargs -n1 sed -i 's/ ASSERT(/ RTC_DCHECK(/'

followed by additional includes of base/checks.h in affected files,
and git cl format.

Also had to do some tweaks to #if !defined(NDEBUG) logic in the
taskrunner code (webrtc/base/task.cc, webrtc/base/taskparent.cc,
webrtc/base/taskparent.h, webrtc/base/taskrunner.cc), replaced to
consistently use RTC_DCHECK_IS_ON, and some of the checks needed
additional #if protection.

Test code was excluded, because it should probably use RTC_CHECK
rather than RTC_DCHECK.

BUG=webrtc:6424
==========

to

==========
Replace ASSERT by RTC_DCHECK in all non-test code.

Bulk of the changes were produced using

  git grep -l ' ASSERT(' | grep -v test | grep -v 'common\.h' |\
    xargs -n1 sed -i 's/ ASSERT(/ RTC_DCHECK(/'

followed by additional includes of base/checks.h in affected files,
and git cl format.

Also had to do some tweaks to #if !defined(NDEBUG) logic in the
taskrunner code (webrtc/base/task.cc, webrtc/base/taskparent.cc,
webrtc/base/taskparent.h, webrtc/base/taskrunner.cc), replaced to
consistently use RTC_DCHECK_IS_ON, and some of the checks needed
additional #if protection.

Test code was excluded, because it should probably use RTC_CHECK
rather than RTC_DCHECK.

BUG=webrtc:6424

Review-Url: https://codereview.webrtc.org/2620303003
Cr-Commit-Position: refs/heads/master@{#16030}
Committed:
https://chromium.googlesource.com/external/webrtc/+/ede5da49606bf57888755058f...
==========

[commit-bot: I haz the power, 2017-01-12 13:15:44]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/ede5da49606bf57888755058f...