Add CHECKs to ensure that PPAPI resources are created on the right thread

These CHECKs could be converted to DCHECKs at some point. But we should get a
clear idea of how often this is happening and what issues it is responsible
for first.

BUG=146415,92909
TEST=Ran test case in http://code.google.com/p/chromium/issues/detail?id=143183
and checked that the CHECK fired.
NOTRY=true


Review URL: https://chromiumcodereview.appspot.com/10912086

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@155257 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 25 insertions, 1 deletions

diff --git a/ppapi/shared_impl/resource_tracker.cc b/ppapi/shared_impl/resource_tracker.cc
index 8cfcfb5..da89774 100644
--- a/ppapi/shared_impl/resource_tracker.cc
+++ b/ppapi/shared_impl/resource_tracker.cc
@@ -23,6 +23,7 @@ ResourceTracker::~ResourceTracker() {
 }
 
 Resource* ResourceTracker::GetResource(PP_Resource res) const {
+  CHECK(thread_checker_.CalledOnValidThread());
   ResourceMap::const_iterator i = live_resources_.find(res);
   if (i == live_resources_.end())
     return NULL;
@@ -30,6 +31,7 @@ Resource* ResourceTracker::GetResource(PP_Resource res) const {
 }
 
 void ResourceTracker::AddRefResource(PP_Resource res) {
+  CHECK(thread_checker_.CalledOnValidThread());
   DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
       << res << " is not a PP_Resource.";
   ResourceMap::iterator i = live_resources_.find(res);
@@ -51,6 +53,7 @@ void ResourceTracker::AddRefResource(PP_Resource res) {
 }
 
 void ResourceTracker::ReleaseResource(PP_Resource res) {
+  CHECK(thread_checker_.CalledOnValidThread());
   DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE))
       << res << " is not a PP_Resource.";
   ResourceMap::iterator i = live_resources_.find(res);
@@ -81,6 +84,7 @@ void ResourceTracker::ReleaseResourceSoon(PP_Resource res) {
 }
 
 void ResourceTracker::DidCreateInstance(PP_Instance instance) {
+  CHECK(thread_checker_.CalledOnValidThread());
   // 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.
@@ -90,6 +94,7 @@ void ResourceTracker::DidCreateInstance(PP_Instance instance) {
 }
 
 void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
+  CHECK(thread_checker_.CalledOnValidThread());
   InstanceMap::iterator found_instance = instance_map_.find(instance);
 
   // Due to the infrastructure of some tests, the instance is unregistered
@@ -144,6 +149,7 @@ void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
 }
 
 int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
+  CHECK(thread_checker_.CalledOnValidThread());
   InstanceMap::const_iterator found = instance_map_.find(instance);
   if (found == instance_map_.end())
     return 0;
@@ -151,6 +157,7 @@ int ResourceTracker::GetLiveObjectsForInstance(PP_Instance instance) const {
 }
 
 PP_Resource ResourceTracker::AddResource(Resource* object) {
+  CHECK(thread_checker_.CalledOnValidThread());
   // If the plugin manages to create too many resources, don't do crazy stuff.
   if (last_resource_value_ == kMaxPPId)
     return 0;
@@ -182,6 +189,7 @@ PP_Resource ResourceTracker::AddResource(Resource* object) {
 }
 
 void ResourceTracker::RemoveResource(Resource* object) {
+  CHECK(thread_checker_.CalledOnValidThread());
   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 1c46bf5..2f1c54e 100644
--- a/ppapi/shared_impl/resource_tracker.h
+++ b/ppapi/shared_impl/resource_tracker.h
@@ -11,6 +11,8 @@
 #include "base/hash_tables.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "base/threading/thread_checker_impl.h"
 #include "ppapi/c/pp_instance.h"
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/shared_impl/ppapi_shared_export.h"
@@ -63,11 +65,11 @@ class PPAPI_SHARED_EXPORT ResourceTracker {
   // the given resource. It's called from the resource destructor.
   virtual void RemoveResource(Resource* object);
 
+ private:
   // Calls LastPluginRefWasDeleted on the given resource object and cancels
   // pending callbacks for the resource.
   void LastPluginRefWasDeleted(Resource* object);
 
- private:
   typedef std::set<PP_Resource> ResourceSet;
 
   struct InstanceData {
@@ -96,6 +98,20 @@ 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
+
   DISALLOW_COPY_AND_ASSIGN(ResourceTracker);
 };