base: Change DCHECK_IS_ON to a macro DCHECK_IS_ON() to match Chromium

changes (see http://crrev.com/e649f573) and fix unit test runtime
errors when building with GYP_DEFINES=dcheck_always_on=1.
This commit is contained in:
Marshall Greenblatt 2015-10-14 09:40:56 -07:00
parent 3f4687a4cd
commit 70ed95bcca
7 changed files with 52 additions and 36 deletions

View File

@ -419,9 +419,9 @@ DEFINE_CHECK_OP_IMPL(GT, > )
#endif #endif
#if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON) #if defined(NDEBUG) && !defined(DCHECK_ALWAYS_ON)
#define DCHECK_IS_ON 0 #define DCHECK_IS_ON() 0
#else #else
#define DCHECK_IS_ON 1 #define DCHECK_IS_ON() 1
#endif #endif
// Definitions for DLOG et al. // Definitions for DLOG et al.
@ -475,14 +475,14 @@ enum { DEBUG_MODE = ENABLE_DLOG };
// Definitions for DCHECK et al. // Definitions for DCHECK et al.
#if DCHECK_IS_ON #if DCHECK_IS_ON()
#define COMPACT_GOOGLE_LOG_EX_DCHECK(ClassName, ...) \ #define COMPACT_GOOGLE_LOG_EX_DCHECK(ClassName, ...) \
COMPACT_GOOGLE_LOG_EX_FATAL(ClassName , ##__VA_ARGS__) COMPACT_GOOGLE_LOG_EX_FATAL(ClassName , ##__VA_ARGS__)
#define COMPACT_GOOGLE_LOG_DCHECK COMPACT_GOOGLE_LOG_FATAL #define COMPACT_GOOGLE_LOG_DCHECK COMPACT_GOOGLE_LOG_FATAL
const LogSeverity LOG_DCHECK = LOG_FATAL; const LogSeverity LOG_DCHECK = LOG_FATAL;
#else // DCHECK_IS_ON #else // DCHECK_IS_ON()
// These are just dummy values. // These are just dummy values.
#define COMPACT_GOOGLE_LOG_EX_DCHECK(ClassName, ...) \ #define COMPACT_GOOGLE_LOG_EX_DCHECK(ClassName, ...) \
@ -490,31 +490,29 @@ const LogSeverity LOG_DCHECK = LOG_FATAL;
#define COMPACT_GOOGLE_LOG_DCHECK COMPACT_GOOGLE_LOG_INFO #define COMPACT_GOOGLE_LOG_DCHECK COMPACT_GOOGLE_LOG_INFO
const LogSeverity LOG_DCHECK = LOG_INFO; const LogSeverity LOG_DCHECK = LOG_INFO;
#endif // DCHECK_IS_ON #endif // DCHECK_IS_ON()
// DCHECK et al. make sure to reference |condition| regardless of // DCHECK et al. make sure to reference |condition| regardless of
// whether DCHECKs are enabled; this is so that we don't get unused // whether DCHECKs are enabled; this is so that we don't get unused
// variable warnings if the only use of a variable is in a DCHECK. // variable warnings if the only use of a variable is in a DCHECK.
// This behavior is different from DLOG_IF et al. // This behavior is different from DLOG_IF et al.
#define DCHECK(condition) \ #define DCHECK(condition) \
LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON && !(condition)) \ LAZY_STREAM(LOG_STREAM(DCHECK), DCHECK_IS_ON() && !(condition)) \
<< "Check failed: " #condition ". " << "Check failed: " #condition ". "
#define DPCHECK(condition) \ #define DPCHECK(condition) \
LAZY_STREAM(PLOG_STREAM(DCHECK), DCHECK_IS_ON && !(condition)) \ LAZY_STREAM(PLOG_STREAM(DCHECK), DCHECK_IS_ON() && !(condition)) \
<< "Check failed: " #condition ". " << "Check failed: " #condition ". "
// Helper macro for binary operators. // Helper macro for binary operators.
// Don't use this macro directly in your code, use DCHECK_EQ et al below. // Don't use this macro directly in your code, use DCHECK_EQ et al below.
#define DCHECK_OP(name, op, val1, val2) \ #define DCHECK_OP(name, op, val1, val2) \
if (DCHECK_IS_ON) \ if (DCHECK_IS_ON()) \
if (std::string* _result = \ if (std::string* _result = cef::logging::Check##name##Impl( \
cef::logging::Check##name##Impl((val1), (val2), \ (val1), (val2), #val1 " " #op " " #val2)) \
#val1 " " #op " " #val2)) \ cef::logging::LogMessage(__FILE__, __LINE__, \
cef::logging::LogMessage( \ ::cef::logging::LOG_DCHECK, _result).stream()
__FILE__, __LINE__, ::cef::logging::LOG_DCHECK, \
_result).stream()
// Equality/Inequality checks - compare two values, and log a // Equality/Inequality checks - compare two values, and log a
// LOG_DCHECK message including the two values when the result is not // LOG_DCHECK message including the two values when the result is not

View File

@ -45,25 +45,24 @@
// If the Chromium implementation diverges the below implementation should be // If the Chromium implementation diverges the below implementation should be
// updated to match. // updated to match.
#include "include/base/cef_logging.h"
#include "include/base/internal/cef_thread_checker_impl.h"
// Apart from debug builds, we also enable the thread checker in // Apart from debug builds, we also enable the thread checker in
// builds with DCHECK_ALWAYS_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 // with this define will get the same level of thread checking as
// debug bots. // debug bots.
// #if DCHECK_IS_ON()
// Note that this does not perfectly match situations where DCHECK is
// enabled. For example a non-official release build may have
// DCHECK_ALWAYS_ON undefined (and therefore ThreadChecker would be
// disabled) but have DCHECKs enabled at runtime.
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
#define ENABLE_THREAD_CHECKER 1 #define ENABLE_THREAD_CHECKER 1
#else #else
#define ENABLE_THREAD_CHECKER 0 #define ENABLE_THREAD_CHECKER 0
#endif #endif
#include "include/base/internal/cef_thread_checker_impl.h"
namespace base { namespace base {
namespace cef_internal {
// Do nothing implementation, for use in release mode. // Do nothing implementation, for use in release mode.
// //
// Note: You should almost always use the ThreadChecker class to get the // Note: You should almost always use the ThreadChecker class to get the
@ -77,6 +76,8 @@ class ThreadCheckerDoNothing {
void DetachFromThread() {} void DetachFromThread() {}
}; };
} // namespace cef_internal
// ThreadChecker is a helper class used to help verify that some methods of a // ThreadChecker is a helper class used to help verify that some methods of a
// class are called from the same thread. It provides identical functionality to // class are called from the same thread. It provides identical functionality to
// base::NonThreadSafe, but it is meant to be held as a member variable, rather // base::NonThreadSafe, but it is meant to be held as a member variable, rather
@ -109,10 +110,10 @@ class ThreadCheckerDoNothing {
// //
// In Release mode, CalledOnValidThread will always return true. // In Release mode, CalledOnValidThread will always return true.
#if ENABLE_THREAD_CHECKER #if ENABLE_THREAD_CHECKER
class ThreadChecker : public ThreadCheckerImpl { class ThreadChecker : public cef_internal::ThreadCheckerImpl {
}; };
#else #else
class ThreadChecker : public ThreadCheckerDoNothing { class ThreadChecker : public cef_internal::ThreadCheckerDoNothing {
}; };
#endif // ENABLE_THREAD_CHECKER #endif // ENABLE_THREAD_CHECKER

View File

@ -42,6 +42,7 @@
// //
// class Controller { // class Controller {
// public: // public:
// Controller() : weak_factory_(this) {}
// void SpawnWorker() { Worker::StartNew(weak_factory_.GetWeakPtr()); } // void SpawnWorker() { Worker::StartNew(weak_factory_.GetWeakPtr()); }
// void WorkComplete(const Result& result) { ... } // void WorkComplete(const Result& result) { ... }
// private: // private:
@ -83,8 +84,13 @@
// WeakPtrs can still be handed off to other threads, e.g. to use to post tasks // WeakPtrs can still be handed off to other threads, e.g. to use to post tasks
// back to object on the bound thread. // back to object on the bound thread.
// //
// Invalidating the factory's WeakPtrs un-binds it from the thread, allowing it // If all WeakPtr objects are destroyed or invalidated then the factory is
// to be passed for a different thread to use or delete it. // unbound from the SequencedTaskRunner/Thread. The WeakPtrFactory may then be
// destroyed, or new WeakPtr objects may be used, from a different sequence.
//
// Thus, at least one WeakPtr object must exist and have been dereferenced on
// the correct thread to enforce that other WeakPtr objects will enforce they
// are used on the desired thread.
#ifndef CEF_INCLUDE_BASE_CEF_WEAK_PTR_H_ #ifndef CEF_INCLUDE_BASE_CEF_WEAK_PTR_H_
#define CEF_INCLUDE_BASE_CEF_WEAK_PTR_H_ #define CEF_INCLUDE_BASE_CEF_WEAK_PTR_H_

View File

@ -37,6 +37,7 @@
#include "include/base/cef_platform_thread.h" #include "include/base/cef_platform_thread.h"
namespace base { namespace base {
namespace cef_internal {
// Real implementation of ThreadChecker, for use in debug mode, or // Real implementation of ThreadChecker, for use in debug mode, or
// for temporary use in release mode (e.g. to CHECK on a threading issue // for temporary use in release mode (e.g. to CHECK on a threading issue
@ -65,6 +66,7 @@ class ThreadCheckerImpl {
mutable PlatformThreadRef valid_thread_id_; mutable PlatformThreadRef valid_thread_id_;
}; };
} // namespace cef_internal
} // namespace base } // namespace base
#endif // CEF_INCLUDE_BASE_INTERNAL_THREAD_CHECKER_IMPL_H_ #endif // CEF_INCLUDE_BASE_INTERNAL_THREAD_CHECKER_IMPL_H_

View File

@ -363,8 +363,8 @@ class CefResourceManager :
UrlFilter url_filter_; UrlFilter url_filter_;
MimeTypeResolver mime_type_resolver_; MimeTypeResolver mime_type_resolver_;
// Must be the last member. // Must be the last member. Created and accessed on the IO thread.
base::WeakPtrFactory<CefResourceManager> weak_ptr_factory_; scoped_ptr<base::WeakPtrFactory<CefResourceManager> > weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(CefResourceManager); DISALLOW_COPY_AND_ASSIGN(CefResourceManager);
}; };

View File

@ -5,6 +5,7 @@
#include "include/base/internal/cef_thread_checker_impl.h" #include "include/base/internal/cef_thread_checker_impl.h"
namespace base { namespace base {
namespace cef_internal {
ThreadCheckerImpl::ThreadCheckerImpl() ThreadCheckerImpl::ThreadCheckerImpl()
: valid_thread_id_() { : valid_thread_id_() {
@ -31,4 +32,5 @@ void ThreadCheckerImpl::EnsureThreadIdAssigned() const {
} }
} }
} // namespace cef_internal
} // namespace base } // namespace base

View File

@ -435,8 +435,7 @@ void CefResourceManager::Request::StopOnIOThread(
CefResourceManager::CefResourceManager() CefResourceManager::CefResourceManager()
: url_filter_(base::Bind(GetFilteredUrl)), : url_filter_(base::Bind(GetFilteredUrl)),
mime_type_resolver_(base::Bind(GetMimeType)), mime_type_resolver_(base::Bind(GetMimeType)) {
weak_ptr_factory_(this) {
} }
CefResourceManager::~CefResourceManager() { CefResourceManager::~CefResourceManager() {
@ -589,7 +588,15 @@ cef_return_value_t CefResourceManager::OnBeforeResourceLoad(
scoped_ptr<RequestState> state(new RequestState); scoped_ptr<RequestState> state(new RequestState);
state->manager_ = weak_ptr_factory_.GetWeakPtr(); if (!weak_ptr_factory_.get()) {
// WeakPtrFactory instances need to be created and destroyed on the same
// thread. This object performs most of its work on the IO thread and will
// be destroyed on the IO thread so, now that we're on the IO thread,
// properly initialize the WeakPtrFactory.
weak_ptr_factory_.reset(new base::WeakPtrFactory<CefResourceManager>(this));
}
state->manager_ = weak_ptr_factory_->GetWeakPtr();
state->callback_ = callback; state->callback_ = callback;
state->params_.url_ = state->params_.url_ =
@ -684,7 +691,7 @@ void CefResourceManager::StopRequest(scoped_ptr<RequestState> state) {
// Move state to the next provider if any and return true if there are more // Move state to the next provider if any and return true if there are more
// providers. // providers.
bool CefResourceManager::IncrementProvider(RequestState* state) { bool CefResourceManager::IncrementProvider(RequestState* state) {
// Identify the next provider. // Identify the next provider.
ProviderEntryList::iterator next_entry_pos = state->current_entry_pos_; ProviderEntryList::iterator next_entry_pos = state->current_entry_pos_;
GetNextValidProvider(++next_entry_pos); GetNextValidProvider(++next_entry_pos);