zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of Test RTC_DCHECK_IS_ON instead of checking DCHECK_ALWAYS_ON everywhere (patchset #2 id:20001 of https://codereview.webrtc.org/2384693002/ )

Browse Source 
Reason for revert:
This CL breaks FYI bots with a compile error.

Sample error:
jingle/glue/thread_wrapper.cc -o obj/jingle/jingle_glue/thread_wrapper.o
In file included from ../../jingle/glue/thread_wrapper.cc:5:
In file included from ../../jingle/glue/thread_wrapper.h:16:
In file included from ../../base/message_loop/message_loop.h:17:
In file included from ../../base/memory/ref_counted.h:19:
../../base/logging.h:598:1: error: call to 'MakeCheckOpString' is ambiguous
DEFINE_CHECK_OP_IMPL(EQ, ==)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:592:17: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
    else return MakeCheckOpString(v1, v2, names); \
                ^~~~~~~~~~~~~~~~~
../../jingle/glue/thread_wrapper.cc:46:3: note: in instantiation of function template specialization 'logging::CheckEQImpl<rtc::Thread *, jingle_glue::JingleThreadWrapper *>' requested here
  DCHECK_EQ(rtc::Thread::Current(), current());
  ^
../../base/logging.h:748:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
                              ^
../../base/logging.h:721:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \
                 ^
<scratch space>:102:1: note: expanded from here
CheckEQImpl
^
../../base/logging.h:555:14: note: candidate function [with t1 = rtc::Thread *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
             ^
../../third_party/webrtc/base/checks.h:122:14: note: candidate function [with t1 = rtc::Thread *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
             ^
In file included from ../../jingle/glue/thread_wrapper.cc:5:
In file included from ../../jingle/glue/thread_wrapper.h:16:
In file included from ../../base/message_loop/message_loop.h:17:
In file included from ../../base/memory/ref_counted.h:19:
../../base/logging.h:598:1: error: call to 'MakeCheckOpString' is ambiguous
DEFINE_CHECK_OP_IMPL(EQ, ==)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:592:17: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
    else return MakeCheckOpString(v1, v2, names); \
                ^~~~~~~~~~~~~~~~~
../../jingle/glue/thread_wrapper.cc:81:3: note: in instantiation of function template specialization 'logging::CheckEQImpl<jingle_glue::JingleThreadWrapper *, jingle_glue::JingleThreadWrapper *>' requested here
  DCHECK_EQ(this, JingleThreadWrapper::current());
  ^
../../base/logging.h:748:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
                              ^
../../base/logging.h:721:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \
                 ^
<scratch space>:5:1: note: expanded from here
CheckEQImpl
^
../../base/logging.h:555:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
             ^
../../third_party/webrtc/base/checks.h:122:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = jingle_glue::JingleThreadWrapper *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
             ^
In file included from ../../jingle/glue/thread_wrapper.cc:5:
In file included from ../../jingle/glue/thread_wrapper.h:16:
In file included from ../../base/message_loop/message_loop.h:17:
In file included from ../../base/memory/ref_counted.h:19:
../../base/logging.h:598:1: error: call to 'MakeCheckOpString' is ambiguous
DEFINE_CHECK_OP_IMPL(EQ, ==)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../base/logging.h:592:17: note: expanded from macro 'DEFINE_CHECK_OP_IMPL'
    else return MakeCheckOpString(v1, v2, names); \
                ^~~~~~~~~~~~~~~~~
../../jingle/glue/thread_wrapper.cc:82:3: note: in instantiation of function template specialization 'logging::CheckEQImpl<jingle_glue::JingleThreadWrapper *, rtc::Thread *>' requested here
  DCHECK_EQ(this, rtc::Thread::Current());
  ^
../../base/logging.h:748:31: note: expanded from macro 'DCHECK_EQ'
#define DCHECK_EQ(val1, val2) DCHECK_OP(EQ, ==, val1, val2)
                              ^
../../base/logging.h:721:18: note: expanded from macro 'DCHECK_OP'
      ::logging::Check##name##Impl((val1), (val2),                     \
                 ^
<scratch space>:12:1: note: expanded from here
CheckEQImpl
^
../../base/logging.h:555:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = rtc::Thread *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
             ^
../../third_party/webrtc/base/checks.h:122:14: note: candidate function [with t1 = jingle_glue::JingleThreadWrapper *, t2 = rtc::Thread *]
std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
             ^
3 errors generated.

Original issue's description:
> Test RTC_DCHECK_IS_ON instead of checking DCHECK_ALWAYS_ON everywhere
>
> The former is always defined (by webrtc/base/checks.h) to either 0 or
> 1, whereas the latter isn't necessarily defined.
>
> NOTRY=true
> BUG=webrtc:6451
>
> Committed: https://crrev.com/ab0b929321d37669165d5795268fa10a8c97ec5b
> Cr-Commit-Position: refs/heads/master@{#14474}

TBR=ossu@webrtc.org,kwiberg@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6451

Review-Url: https://codereview.webrtc.org/2384083004
Cr-Commit-Position: refs/heads/master@{#14480}
This commit is contained in:
guidou
2016-10-03 08:32:31 -07:00
committed by Commit bot
parent 65b42c251f
commit 8f9010631c
15 changed files with 54 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
webrtc/api/statscollector.cc
View File

@ -398,7 +398,7 @@ void StatsCollector::AddLocalAudioTrack(AudioTrackInterface* audio_track,
uint32_t ssrc) {
RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent());
RTC_DCHECK(audio_track != NULL);
#if RTC_DCHECK_IS_ON
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
for (const auto& track : local_audio_tracks_)
RTC_DCHECK(track.first != audio_track || track.second != ssrc);
#endif

4
webrtc/api/statstypes.cc
View File

@ -276,7 +276,7 @@ bool StatsReport::Value::Equals(const Value& other) const {
case kFloat:
return value_.float_ == other.value_.float_;
case kStaticString: {
#if RTC_DCHECK_IS_ON
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
if (value_.static_string_ != other.value_.static_string_) {
RTC_DCHECK(strcmp(value_.static_string_, other.value_.static_string_) !=
0)
@ -306,7 +306,7 @@ bool StatsReport::Value::operator==(const char* value) const {
return value_.string_->compare(value) == 0;
if (type_ != kStaticString)
return false;
#if RTC_DCHECK_IS_ON
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
if (value_.static_string_ != value)
RTC_DCHECK(strcmp(value_.static_string_, value) != 0)
<< "Duplicate global?";

2
webrtc/base/buffer.h
View File

@ -349,7 +349,7 @@ class BufferT {
// Called when *this has been moved from. Conceptually it's a no-op, but we
// can mutate the state slightly to help subsequent sanity checks catch bugs.
void OnMovedFrom() {
#if RTC_DCHECK_IS_ON
#ifdef NDEBUG
// Make *this consistent and empty. Shouldn't be necessary, but better safe
// than sorry.
size_ = 0;

11
webrtc/base/checks.h
View File

@ -13,10 +13,7 @@
#include "webrtc/typedefs.h"
// If you for some reson need to know if DCHECKs are on, test the value of
// RTC_DCHECK_IS_ON. (Test its value, not if it's defined; it'll always be
// defined, to either a true or a false value.)
#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
#define RTC_DCHECK_IS_ON 1
#else
#define RTC_DCHECK_IS_ON 0
@ -93,9 +90,9 @@ namespace rtc {
? static_cast<void>(0) \
: rtc::FatalMessageVoidify() & rtc::FatalMessage("", 0).stream()
// RTC_CHECK dies with a fatal error if condition is not true. It is *not*
// controlled by NDEBUG or anything else, so the check will be executed
// regardless of compilation mode.
// RTC_CHECK dies with a fatal error if condition is not true. It is *not*
// controlled by NDEBUG, so the check will be executed regardless of
// compilation mode.
//
// We make sure RTC_CHECK et al. always evaluates their arguments, as
// doing RTC_CHECK(FunctionWithSideEffect()) is a common idiom.

7
webrtc/base/criticalsection.h
View File

@ -12,7 +12,6 @@
#define WEBRTC_BASE_CRITICALSECTION_H_
#include "webrtc/base/atomicops.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/base/platform_thread_types.h"
@ -38,7 +37,11 @@
#include <dispatch/dispatch.h>
#endif
#define CS_DEBUG_CHECKS RTC_DCHECK_IS_ON
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
#define CS_DEBUG_CHECKS 1
#else
#define CS_DEBUG_CHECKS 0
#endif
#if CS_DEBUG_CHECKS
#define CS_DEBUG_CODE(x) x

13
webrtc/base/sequenced_task_checker.h
View File

@ -12,9 +12,18 @@
#define WEBRTC_BASE_SEQUENCED_TASK_CHECKER_H_
// Apart from debug builds, we also enable the sequence checker in
// builds with RTC_DCHECK_IS_ON so that trybots and waterfall bots
// builds with DCHECK_ALWAYS_ON so that trybots and waterfall bots
// with this define will get the same level of checking as debug bots.
#define ENABLE_SEQUENCED_TASK_CHECKER RTC_DCHECK_IS_ON
//
// Note that this does not perfectly match situations where RTC_DCHECK is
// enabled. For example a non-official release build may have
// DCHECK_ALWAYS_ON undefined (and therefore SequencedTaskChecker would be
// disabled) but have RTC_DCHECKs enabled at runtime.
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
#define ENABLE_SEQUENCED_TASK_CHECKER 1
#else
#define ENABLE_SEQUENCED_TASK_CHECKER 0
#endif
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"

6
webrtc/base/sequenced_task_checker_unittest.cc
View File

@ -198,7 +198,7 @@ TEST(SequencedTaskCheckerTest, DetachFromTaskQueueAndUseOnThread) {
EXPECT_TRUE(done_event.Wait(1000));
}
#if RTC_DCHECK_IS_ON
#if !defined(NDEBUG) || DCHECK_ALWAYS_ON
TEST(SequencedTaskCheckerTest, MethodNotAllowedOnDifferentThreadInDebug) {
RunMethodOnDifferentThread(false);
}
@ -208,7 +208,7 @@ TEST(SequencedTaskCheckerTest, MethodAllowedOnDifferentThreadInRelease) {
}
#endif
#if RTC_DCHECK_IS_ON
#if !defined(NDEBUG) || DCHECK_ALWAYS_ON
TEST(SequencedTaskCheckerTest, MethodNotAllowedOnDifferentTaskQueueInDebug) {
RunMethodOnDifferentTaskQueue(false);
}
@ -218,7 +218,7 @@ TEST(SequencedTaskCheckerTest, MethodAllowedOnDifferentTaskQueueInRelease) {
}
#endif
#if RTC_DCHECK_IS_ON
#if !defined(NDEBUG) || DCHECK_ALWAYS_ON
TEST(SequencedTaskCheckerTest, DetachFromTaskQueueInDebug) {
DetachThenCallFromDifferentTaskQueue(false);
}

2
webrtc/base/stringutils.cc
View File

@ -77,7 +77,7 @@ size_t asccpyn(wchar_t* buffer, size_t buflen,
} else if (srclen >= buflen) {
srclen = buflen - 1;
}
#if RTC_DCHECK_IS_ON
#if !defined(NDEBUG)
// Double check that characters are not UTF-8
for (size_t pos = 0; pos < srclen; ++pos)
RTC_DCHECK_LT(static_cast<unsigned char>(source[pos]), 128);

13
webrtc/base/thread_checker.h
View File

@ -14,10 +14,19 @@
#define WEBRTC_BASE_THREAD_CHECKER_H_
// Apart from debug builds, we also enable the thread checker in
// builds with RTC_DCHECK_IS_ON so that trybots and waterfall bots
// builds with DCHECK_ALWAYS_ON so that trybots and waterfall bots
// with this define will get the same level of thread checking as
// debug bots.
#define ENABLE_THREAD_CHECKER RTC_DCHECK_IS_ON
//
// Note that this does not perfectly match situations where RTC_DCHECK is
// enabled. For example a non-official release build may have
// DCHECK_ALWAYS_ON undefined (and therefore ThreadChecker would be
// disabled) but have RTC_DCHECKs enabled at runtime.
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
#define ENABLE_THREAD_CHECKER 1
#else
#define ENABLE_THREAD_CHECKER 0
#endif
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"

6
webrtc/base/thread_checker_unittest.cc
View File

@ -21,7 +21,11 @@
// Duplicated from base/threading/thread_checker.h so that we can be
// good citizens there and undef the macro.
#define ENABLE_THREAD_CHECKER RTC_DCHECK_IS_ON
#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
#define ENABLE_THREAD_CHECKER 1
#else
#define ENABLE_THREAD_CHECKER 0
#endif
namespace rtc {

8
webrtc/common_audio/resampler/push_resampler.cc
View File

@ -29,7 +29,7 @@ void CheckValidInitParams(int src_sample_rate_hz, int dst_sample_rate_hz,
size_t num_channels) {
// The below checks are temporarily disabled on WEBRTC_WIN due to problems
// with clang debug builds.
#if !defined(WEBRTC_WIN) && defined(__clang__)
#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
RTC_DCHECK_GT(src_sample_rate_hz, 0);
RTC_DCHECK_GT(dst_sample_rate_hz, 0);
RTC_DCHECK_GT(num_channels, 0u);
@ -46,11 +46,11 @@ void CheckExpectedBufferSizes(size_t src_length,
// with clang debug builds.
// TODO(tommi): Re-enable when we've figured out what the problem is.
// http://crbug.com/615050
#if !defined(WEBRTC_WIN) && defined(__clang__)
#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
const size_t src_size_10ms = src_sample_rate * num_channels / 100;
const size_t dst_size_10ms = dst_sample_rate * num_channels / 100;
RTC_DCHECK_EQ(src_length, src_size_10ms);
RTC_DCHECK_GE(dst_capacity, dst_size_10ms);
RTC_CHECK_EQ(src_length, src_size_10ms);
RTC_CHECK_GE(dst_capacity, dst_size_10ms);
#endif
}
}

2
webrtc/media/sctp/sctpdataengine.cc
View File

@ -118,7 +118,7 @@ enum {
// Helper for logging SCTP messages.
void DebugSctpPrintf(const char* format, ...) {
#if RTC_DCHECK_IS_ON
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
char s[255];
va_list ap;
va_start(ap, format);

5
webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
View File

@ -317,8 +317,8 @@ TEST_F(AudioCodingModuleTestOldApi, VerifyOutputFrame) {
// with clang debug builds.
// TODO(tommi): Re-enable when we've figured out what the problem is.
// http://crbug.com/615050
#if !defined(WEBRTC_WIN) && defined(__clang__) && RTC_DCHECK_IS_ON && \
GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
#if !defined(WEBRTC_WIN) && defined(__clang__) && !defined(NDEBUG)
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST_F(AudioCodingModuleTestOldApi, FailOnZeroDesiredFrequency) {
AudioFrame audio_frame;
bool muted;
@ -326,6 +326,7 @@ TEST_F(AudioCodingModuleTestOldApi, FailOnZeroDesiredFrequency) {
"dst_sample_rate_hz");
}
#endif
#endif
// Checks that the transport callback is invoked once for each speech packet.
// Also checks that the frame type is kAudioFrameSpeech.

2
webrtc/modules/audio_device/android/opensles_player.cc
View File

@ -141,7 +141,7 @@ int OpenSLESPlayer::StopPlayout() {
RETURN_ON_ERROR((*player_)->SetPlayState(player_, SL_PLAYSTATE_STOPPED), -1);
// Clear the buffer queue to flush out any remaining data.
RETURN_ON_ERROR((*simple_buffer_queue_)->Clear(simple_buffer_queue_), -1);
#if RTC_DCHECK_IS_ON
#ifndef NDEBUG
// Verify that the buffer queue is in fact cleared as it should.
SLAndroidSimpleBufferQueueState buffer_queue_state;
(*simple_buffer_queue_)->GetState(simple_buffer_queue_, &buffer_queue_state);

2
webrtc/modules/utility/source/process_thread_impl.cc
View File

@ -133,7 +133,7 @@ void ProcessThreadImpl::RegisterModule(Module* module) {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
RTC_DCHECK(module);
#if RTC_DCHECK_IS_ON
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
{
// Catch programmer error.
rtc::CritScope lock(&lock_);