summaryrefslogtreecommitdiffstats
path: root/ppapi/shared_impl
diff options
context:
space:
mode:
authordmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-12 00:41:08 +0000
committerdmichael@chromium.org <dmichael@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-12 00:41:08 +0000
commit6fb8da6d97a3bbdb0df022ea201bf91aa36a001b (patch)
tree26d3b38ccb77f4fdb5dc2538c67e42703295bde5 /ppapi/shared_impl
parent5e93c47a282ed564e6a56af10f4097599fe41def (diff)
downloadchromium_src-6fb8da6d97a3bbdb0df022ea201bf91aa36a001b.zip
chromium_src-6fb8da6d97a3bbdb0df022ea201bf91aa36a001b.tar.gz
chromium_src-6fb8da6d97a3bbdb0df022ea201bf91aa36a001b.tar.bz2
PPAPI: Remove threading options; it's always on
This also re-enables thread checking for the host side resource and var trackers. Before, checking was disabled everywhere. BUG=159240,92909 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186925 Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=186939 due to build errors Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187340 Review URL: https://chromiumcodereview.appspot.com/12378050 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@187427 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ppapi/shared_impl')
-rw-r--r--ppapi/shared_impl/resource_tracker.cc26
-rw-r--r--ppapi/shared_impl/resource_tracker.h30
-rw-r--r--ppapi/shared_impl/test_globals.cc2
-rw-r--r--ppapi/shared_impl/test_globals.h2
-rw-r--r--ppapi/shared_impl/var_tracker.cc48
-rw-r--r--ppapi/shared_impl/var_tracker.h29
6 files changed, 75 insertions, 62 deletions
diff --git a/ppapi/shared_impl/resource_tracker.cc b/ppapi/shared_impl/resource_tracker.cc
index a448ce4..a46c823 100644
--- a/ppapi/shared_impl/resource_tracker.cc
+++ b/ppapi/shared_impl/resource_tracker.cc
@@ -15,17 +15,23 @@
namespace ppapi {
-ResourceTracker::ResourceTracker()
+ResourceTracker::ResourceTracker(ThreadMode thread_mode)
: last_resource_value_(0),
ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+ if (thread_mode == SINGLE_THREADED)
+ thread_checker_.reset(new base::ThreadChecker);
}
ResourceTracker::~ResourceTracker() {
}
-Resource* ResourceTracker::GetResource(PP_Resource res) const {
- CHECK(thread_checker_.CalledOnValidThread());
+void ResourceTracker::CheckThreadingPreconditions() const {
+ DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
ProxyLock::AssertAcquired();
+}
+
+Resource* ResourceTracker::GetResource(PP_Resource res) const {
+ CheckThreadingPreconditions();
ResourceMap::const_iterator i = live_resources_.find(res);
if (i == live_resources_.end())
return NULL;
@@ -33,7 +39,7 @@ Resource* ResourceTracker::GetResource(PP_Resource res) const {
}
void ResourceTracker::AddRefResource(PP_Resource res) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
<< res << " is not a PP_Resource.";
ResourceMap::iterator i = live_resources_.find(res);
@@ -55,7 +61,7 @@ void ResourceTracker::AddRefResource(PP_Resource res) {
}
void ResourceTracker::ReleaseResource(PP_Resource res) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
<< res << " is not a PP_Resource.";
ResourceMap::iterator i = live_resources_.find(res);
@@ -86,7 +92,7 @@ void ResourceTracker::ReleaseResourceSoon(PP_Resource res) {
}
void ResourceTracker::DidCreateInstance(PP_Instance instance) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
// Due to the infrastructure of some tests, the instance is registered
// twice in a few cases. It would be nice not to do that and assert here
// instead.
@@ -96,7 +102,7 @@ void ResourceTracker::DidCreateInstance(PP_Instance instance) {
}
void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::iterator found_instance = instance_map_.find(instance);
// Due to the infrastructure of some tests, the instance is unregistered
@@ -151,7 +157,7 @@ void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
}
int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
InstanceMap::const_iterator found = instance_map_.find(instance);
if (found == instance_map_.end())
return 0;
@@ -159,7 +165,7 @@ int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
}
PP_Resource ResourceTracker::AddResource(Resource* object) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
// If the plugin manages to create too many resources, don't do crazy stuff.
if (last_resource_value_ == kMaxPPId)
return 0;
@@ -191,7 +197,7 @@ PP_Resource ResourceTracker::AddResource(Resource* object) {
}
void ResourceTracker::RemoveResource(Resource* object) {
- CHECK(thread_checker_.CalledOnValidThread());
+ CheckThreadingPreconditions();
PP_Resource pp_resource = object->pp_resource();
InstanceMap::iterator found = instance_map_.find(object->pp_instance());
if (found != instance_map_.end())
diff --git a/ppapi/shared_impl/resource_tracker.h b/ppapi/shared_impl/resource_tracker.h
index f5f790c..54e6daf 100644
--- a/ppapi/shared_impl/resource_tracker.h
+++ b/ppapi/shared_impl/resource_tracker.h
@@ -10,6 +10,7 @@
#include "base/basictypes.h"
#include "base/hash_tables.h"
#include "base/memory/linked_ptr.h"
+#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h"
#include "base/threading/thread_checker_impl.h"
@@ -23,7 +24,12 @@ class Resource;
class PPAPI_SHARED_EXPORT ResourceTracker {
public:
- ResourceTracker();
+ // A SINGLE_THREADED ResourceTracker will use a thread-checker to make sure
+ // it's always invoked on the same thread on which it was constructed. A
+ // THREAD_SAFE ResourceTracker will check that the ProxyLock is held. See
+ // CheckThreadingPreconditions() for more details.
+ enum ThreadMode { SINGLE_THREADED, THREAD_SAFE };
+ explicit ResourceTracker(ThreadMode thread_mode);
virtual ~ResourceTracker();
// The returned pointer will be NULL if there is no resource. The reference
@@ -53,6 +59,10 @@ class PPAPI_SHARED_EXPORT ResourceTracker {
// This calls AddResource and RemoveResource.
friend class Resource;
+ // On the host-side, make sure we are called on the right thread. On the
+ // plugin side, make sure we have the proxy lock.
+ void CheckThreadingPreconditions() const;
+
// Adds the given resource to the tracker, associating it with the instance
// stored in the resource object. The new resource ID is returned, and the
// resource will have 0 plugin refcount. This is called by the resource
@@ -98,19 +108,11 @@ class PPAPI_SHARED_EXPORT ResourceTracker {
base::WeakPtrFactory<ResourceTracker> weak_ptr_factory_;
- // TODO(raymes): We won't need to do thread checks once pepper calls are
- // allowed off of the main thread.
- // See http://code.google.com/p/chromium/issues/detail?id=92909.
-#ifdef ENABLE_PEPPER_THREADING
- base::ThreadCheckerDoNothing thread_checker_;
-#else
- // TODO(raymes): We've seen plugins (Flash) creating resources from random
- // threads. Let's always crash for now in this case. Once we have a handle
- // over how common this is, we can change ThreadCheckerImpl->ThreadChecker
- // so that we only crash in debug mode. See
- // https://code.google.com/p/chromium/issues/detail?id=146415.
- base::ThreadCheckerImpl thread_checker_;
-#endif
+ // On the host side, we want to check that we are only called on the main
+ // thread. This is to protect us from accidentally using the tracker from
+ // other threads (especially the IO thread). On the plugin side, the tracker
+ // is protected by the proxy lock and is thread-safe, so this will be NULL.
+ scoped_ptr<base::ThreadChecker> thread_checker_;
DISALLOW_COPY_AND_ASSIGN(ResourceTracker);
};
diff --git a/ppapi/shared_impl/test_globals.cc b/ppapi/shared_impl/test_globals.cc
index 913d53c..6c6af5b 100644
--- a/ppapi/shared_impl/test_globals.cc
+++ b/ppapi/shared_impl/test_globals.cc
@@ -8,11 +8,13 @@ namespace ppapi {
TestGlobals::TestGlobals()
: ppapi::PpapiGlobals(),
+ resource_tracker_(ResourceTracker::THREAD_SAFE),
callback_tracker_(new CallbackTracker) {
}
TestGlobals::TestGlobals(PpapiGlobals::PerThreadForTest per_thread_for_test)
: ppapi::PpapiGlobals(per_thread_for_test),
+ resource_tracker_(ResourceTracker::THREAD_SAFE),
callback_tracker_(new CallbackTracker) {
}
diff --git a/ppapi/shared_impl/test_globals.h b/ppapi/shared_impl/test_globals.h
index 68a997a..75e3a90 100644
--- a/ppapi/shared_impl/test_globals.h
+++ b/ppapi/shared_impl/test_globals.h
@@ -15,7 +15,7 @@ namespace ppapi {
class TestVarTracker : public VarTracker {
public:
- TestVarTracker() {}
+ TestVarTracker() : VarTracker(THREAD_SAFE) {}
virtual ~TestVarTracker() {}
virtual ArrayBufferVar* CreateArrayBuffer(uint32 size_in_bytes) OVERRIDE {
return NULL;
diff --git a/ppapi/shared_impl/var_tracker.cc b/ppapi/shared_impl/var_tracker.cc
index 6bc0261..5e33a23 100644
--- a/ppapi/shared_impl/var_tracker.cc
+++ b/ppapi/shared_impl/var_tracker.cc
@@ -27,22 +27,27 @@ VarTracker::VarInfo::VarInfo(Var* v, int input_ref_count)
track_with_no_reference_count(0) {
}
-VarTracker::VarTracker() : last_var_id_(0) {
+VarTracker::VarTracker(ThreadMode thread_mode) : last_var_id_(0) {
+ if (thread_mode == SINGLE_THREADED)
+ thread_checker_.reset(new base::ThreadChecker);
}
VarTracker::~VarTracker() {
}
-int32 VarTracker::AddVar(Var* var) {
- DCHECK(CalledOnValidThread());
+void VarTracker::CheckThreadingPreconditions() const {
+ DCHECK(!thread_checker_ || thread_checker_->CalledOnValidThread());
ProxyLock::AssertAcquired();
+}
+
+int32 VarTracker::AddVar(Var* var) {
+ CheckThreadingPreconditions();
return AddVarInternal(var, ADD_VAR_TAKE_ONE_REFERENCE);
}
Var* VarTracker::GetVar(int32 var_id) const {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
VarMap::const_iterator result = live_vars_.find(var_id);
if (result == live_vars_.end())
@@ -51,8 +56,7 @@ Var* VarTracker::GetVar(int32 var_id) const {
}
Var* VarTracker::GetVar(const PP_Var& var) const {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
if (!IsVarTypeRefcounted(var.type))
return NULL;
@@ -60,8 +64,7 @@ Var* VarTracker::GetVar(const PP_Var& var) const {
}
bool VarTracker::AddRefVar(int32 var_id) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
@@ -86,8 +89,7 @@ bool VarTracker::AddRefVar(int32 var_id) {
}
bool VarTracker::AddRefVar(const PP_Var& var) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
if (!IsVarTypeRefcounted(var.type))
return false;
@@ -95,8 +97,7 @@ bool VarTracker::AddRefVar(const PP_Var& var) {
}
bool VarTracker::ReleaseVar(int32 var_id) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
@@ -127,8 +128,7 @@ bool VarTracker::ReleaseVar(int32 var_id) {
}
bool VarTracker::ReleaseVar(const PP_Var& var) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
if (!IsVarTypeRefcounted(var.type))
return false;
@@ -152,8 +152,7 @@ VarTracker::VarMap::iterator VarTracker::GetLiveVar(int32 id) {
}
int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
@@ -163,8 +162,7 @@ int VarTracker::GetRefCountForObject(const PP_Var& plugin_object) {
int VarTracker::GetTrackedWithNoReferenceCountForObject(
const PP_Var& plugin_object) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
VarMap::iterator found = GetLiveVar(plugin_object);
if (found == live_vars_.end())
@@ -186,8 +184,7 @@ bool VarTracker::IsVarTypeRefcounted(PP_VarType type) const {
}
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
scoped_refptr<ArrayBufferVar> array_buffer(CreateArrayBuffer(size_in_bytes));
if (!array_buffer)
@@ -197,8 +194,7 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes) {
PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
const void* data) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
ArrayBufferVar* array_buffer = MakeArrayBufferVar(size_in_bytes, data);
return array_buffer ? array_buffer->GetPPVar() : PP_MakeNull();
@@ -206,8 +202,7 @@ PP_Var VarTracker::MakeArrayBufferPPVar(uint32 size_in_bytes,
ArrayBufferVar* VarTracker::MakeArrayBufferVar(uint32 size_in_bytes,
const void* data) {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
ArrayBufferVar* array_buffer(CreateArrayBuffer(size_in_bytes));
if (!array_buffer)
@@ -217,8 +212,7 @@ ArrayBufferVar* VarTracker::MakeArrayBufferVar(uint32 size_in_bytes,
}
std::vector<PP_Var> VarTracker::GetLiveVars() {
- DCHECK(CalledOnValidThread());
- ProxyLock::AssertAcquired();
+ CheckThreadingPreconditions();
std::vector<PP_Var> var_vector;
var_vector.reserve(live_vars_.size());
diff --git a/ppapi/shared_impl/var_tracker.h b/ppapi/shared_impl/var_tracker.h
index 1d0c0d6..7d544da 100644
--- a/ppapi/shared_impl/var_tracker.h
+++ b/ppapi/shared_impl/var_tracker.h
@@ -10,7 +10,8 @@
#include "base/basictypes.h"
#include "base/hash_tables.h"
#include "base/memory/ref_counted.h"
-#include "base/threading/non_thread_safe.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/thread_checker.h"
#include "ppapi/c/pp_instance.h"
#include "ppapi/c/pp_module.h"
#include "ppapi/c/pp_var.h"
@@ -33,16 +34,14 @@ class Var;
// This class maintains the "track_with_no_reference_count" but doesn't do
// anything with it other than call virtual functions. The interesting parts
// are added by the PluginObjectVar derived from this class.
-class PPAPI_SHARED_EXPORT VarTracker
-#ifdef ENABLE_PEPPER_THREADING
- : NON_EXPORTED_BASE(public base::NonThreadSafeDoNothing) {
-#else
- // TODO(dmichael): Remove the thread checking when calls are allowed off the
- // main thread (crbug.com/92909).
- : NON_EXPORTED_BASE(public base::NonThreadSafe) {
-#endif
+class PPAPI_SHARED_EXPORT VarTracker {
public:
- VarTracker();
+ // A SINGLE_THREADED VarTracker will use a thread-checker to make sure it's
+ // always invoked on the same thread on which it was constructed. A
+ // THREAD_SAFE VarTracker will check that the ProxyLock is held. See
+ // CheckThreadingPreconditions() for more details.
+ enum ThreadMode { SINGLE_THREADED, THREAD_SAFE };
+ explicit VarTracker(ThreadMode thread_mode);
virtual ~VarTracker();
// Called by the Var object to add a new var to the tracker.
@@ -125,6 +124,10 @@ class PPAPI_SHARED_EXPORT VarTracker
ADD_VAR_CREATE_WITH_NO_REFERENCE
};
+ // On the host-side, make sure we are called on the right thread. On the
+ // plugin side, make sure we have the proxy lock.
+ void CheckThreadingPreconditions() const;
+
// Implementation of AddVar that allows the caller to specify whether the
// initial refcount of the added object will be 0 or 1.
//
@@ -172,6 +175,12 @@ class PPAPI_SHARED_EXPORT VarTracker
// a real WebKit ArrayBuffer on the host side.
virtual ArrayBufferVar* CreateArrayBuffer(uint32 size_in_bytes) = 0;
+ // On the host side, we want to check that we are only called on the main
+ // thread. This is to protect us from accidentally using the tracker from
+ // other threads (especially the IO thread). On the plugin side, the tracker
+ // is protected by the proxy lock and is thread-safe, so this will be NULL.
+ scoped_ptr<base::ThreadChecker> thread_checker_;
+
DISALLOW_COPY_AND_ASSIGN(VarTracker);
};
generated by cgit v1.1 at 2025-03-24 02:09:25 +0000