From f258ba2e2403ada7c5c72fb9950637ddea86a014 Mon Sep 17 00:00:00 2001 From: "willchan@chromium.org" Date: Wed, 15 Dec 2010 09:31:33 +0000 Subject: Revert 69243 - Reland 69237 - Fix raw_scoped_refptr_mismatch_checker.h. Damn, missed a unittest. BUG=28083 TEST=none Review URL: http://codereview.chromium.org/3549010 TBR=willchan@chromium.org Review URL: http://codereview.chromium.org/5885001 TBR=phajdan.jr@chromium.org Review URL: http://codereview.chromium.org/5887001 TBR=willchan@chromium.org Review URL: http://codereview.chromium.org/5888001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69244 0039d316-1c4b-4281-b951-d872f2087c98 --- base/callback.h | 5 +- base/raw_scoped_refptr_mismatch_checker.h | 225 ++++++++++++++++++------------ base/task.h | 15 +- base/template_util.h | 47 ------- 4 files changed, 141 insertions(+), 151 deletions(-) diff --git a/base/callback.h b/base/callback.h index e5ea771..7f2eb70 100644 --- a/base/callback.h +++ b/base/callback.h @@ -206,9 +206,8 @@ template class UnboundMethod { public: UnboundMethod(Method m, const Params& p) : m_(m), p_(p) { - COMPILE_ASSERT( - (base::internal::ParamsUseScopedRefptrCorrectly::value), - badunboundmethodparams); + COMPILE_ASSERT((MethodUsesScopedRefptrCorrectly::value), + badunboundmethodparams); } void Run(T* obj) const { DispatchToMethod(obj, m_, p_); diff --git a/base/raw_scoped_refptr_mismatch_checker.h b/base/raw_scoped_refptr_mismatch_checker.h index b79cfb5..d2913e7 100644 --- a/base/raw_scoped_refptr_mismatch_checker.h +++ b/base/raw_scoped_refptr_mismatch_checker.h @@ -7,124 +7,165 @@ #pragma once #include "base/ref_counted.h" -#include "base/template_util.h" #include "base/tuple.h" -#include "build/build_config.h" -// It is dangerous to post a task with a T* argument where T is a subtype of -// RefCounted(Base|ThreadSafeBase), since by the time the parameter is used, the -// object may already have been deleted since it was not held with a -// scoped_refptr. Example: http://crbug.com/27191 +// It is dangerous to post a task with a raw pointer argument to a function +// that expects a scoped_refptr<>. The compiler will happily accept the +// situation, but it will not attempt to increase the refcount until the task +// runs. Callers expecting the argument to be refcounted up at post time are +// in for a nasty surprise! Example: http://crbug.com/27191 // The following set of traits are designed to generate a compile error // whenever this antipattern is attempted. - -namespace base { - -// This is a base internal implementation file used by task.h and callback.h. -// Not for public consumption, so we wrap it in namespace internal. -namespace internal { - -template -struct NeedsScopedRefptrButGetsRawPtr { -#if defined(OS_WIN) - enum { - value = base::false_type::value - }; -#else - enum { - // Human readable translation: you needed to be a scoped_refptr if you are a - // raw pointer type and are convertible to a RefCounted(Base|ThreadSafeBase) - // type. - value = (is_pointer::value && - (is_convertible::value || - is_convertible::value)) - }; -#endif -}; - -template -struct ParamsUseScopedRefptrCorrectly { +template +struct ExpectsScopedRefptrButGetsRawPtr { enum { value = 0 }; }; -template <> -struct ParamsUseScopedRefptrCorrectly { +template +struct ExpectsScopedRefptrButGetsRawPtr, B*> { enum { value = 1 }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !NeedsScopedRefptrButGetsRawPtr::value }; +template +struct FunctionUsesScopedRefptrCorrectly { + enum { value = 1 }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !ExpectsScopedRefptrButGetsRawPtr::value }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; }; -template -struct ParamsUseScopedRefptrCorrectly > { - enum { value = !(NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value || - NeedsScopedRefptrButGetsRawPtr::value) }; +template +struct FunctionUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; }; -} // namespace internal +template +struct MethodUsesScopedRefptrCorrectly { + enum { value = 1 }; +}; -} // namespace base +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !ExpectsScopedRefptrButGetsRawPtr::value }; +}; + +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; +}; + +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; +}; + +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; +}; + +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; +}; + +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; +}; + +template +struct MethodUsesScopedRefptrCorrectly > { + enum { value = !(ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value || + ExpectsScopedRefptrButGetsRawPtr::value) }; +}; #endif // BASE_RAW_SCOPED_REFPTR_MISMATCH_CHECKER_H_ diff --git a/base/task.h b/base/task.h index 1949e74..b21ccd8 100644 --- a/base/task.h +++ b/base/task.h @@ -149,9 +149,8 @@ class ScopedRunnableMethodFactory { : obj_(obj), meth_(meth), params_(params) { - COMPILE_ASSERT( - (base::internal::ParamsUseScopedRefptrCorrectly::value), - badscopedrunnablemethodparams); + COMPILE_ASSERT((MethodUsesScopedRefptrCorrectly::value), + badscopedrunnablemethodparams); } virtual void Run() { @@ -318,9 +317,8 @@ class RunnableMethod : public CancelableTask { RunnableMethod(T* obj, Method meth, const Params& params) : obj_(obj), meth_(meth), params_(params) { traits_.RetainCallee(obj_); - COMPILE_ASSERT( - (base::internal::ParamsUseScopedRefptrCorrectly::value), - badrunnablemethodparams); + COMPILE_ASSERT((MethodUsesScopedRefptrCorrectly::value), + badrunnablemethodparams); } ~RunnableMethod() { @@ -431,9 +429,8 @@ class RunnableFunction : public CancelableTask { public: RunnableFunction(Function function, const Params& params) : function_(function), params_(params) { - COMPILE_ASSERT( - (base::internal::ParamsUseScopedRefptrCorrectly::value), - badrunnablefunctionparams); + COMPILE_ASSERT((FunctionUsesScopedRefptrCorrectly::value), + badrunnablefunctionparams); } ~RunnableFunction() { diff --git a/base/template_util.h b/base/template_util.h index 27bdb73..2cfe04c 100644 --- a/base/template_util.h +++ b/base/template_util.h @@ -6,8 +6,6 @@ #define BASE_TEMPLATE_UTIL_H_ #pragma once -#include "build/build_config.h" - namespace base { // template definitions from tr1 @@ -27,51 +25,6 @@ typedef integral_constant false_type; template struct is_pointer : false_type {}; template struct is_pointer : true_type {}; -namespace internal { - -// Types small_ and big_ are guaranteed such that sizeof(small_) < -// sizeof(big_) -typedef char small_; - -struct big_ { - small_ dummy[2]; -}; - -#if !defined(OS_WIN) - -// This class is an implementation detail for is_convertible, and you -// don't need to know how it works to use is_convertible. For those -// who care: we declare two different functions, one whose argument is -// of type To and one with a variadic argument list. We give them -// return types of different size, so we can use sizeof to trick the -// compiler into telling us which function it would have chosen if we -// had called it with an argument of type From. See Alexandrescu's -// _Modern C++ Design_ for more details on this sort of trick. - -template -struct ConvertHelper { - static small_ Test(To); - static big_ Test(...); - static From Create(); -}; - -#endif // !defined(OS_WIN) - -} // namespace internal - -#if !defined(OS_WIN) - -// Inherits from true_type if From is convertible to To, false_type otherwise. -template -struct is_convertible - : integral_constant::Test( - internal::ConvertHelper::Create())) - == sizeof(internal::small_)> { -}; - -#endif // !defined(OS_WIN) - } // namespace base #endif // BASE_TEMPLATE_UTIL_H_ -- cgit v1.1