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

ppapi/shared_impl/resource_tracker.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ppapi/shared_impl/resource_tracker.h"
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop.h"
 #include "ppapi/shared_impl/callback_tracker.h"
 #include "ppapi/shared_impl/id_assignment.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/resource.h"
 
 namespace ppapi {
 
 ResourceTracker::ResourceTracker()
     : last_resource_value_(0),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
 
 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;
   return i->second.first;
 }
 
 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);
   if (i == live_resources_.end())
     return;
 
   // Prevent overflow of refcount.
   if (i->second.second ==
       std::numeric_limits<ResourceAndRefCount::second_type>::max())
     return;
 
   // When we go from 0 to 1 plugin ref count, keep an additional "real" ref
   // on its behalf.
   if (i->second.second == 0)
     i->second.first->AddRef();
 
   i->second.second++;
   return;
 }
 
 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);
   if (i == live_resources_.end())
     return;
 
   // Prevent underflow of refcount.
   if (i->second.second == 0)
     return;
 
@@ skipping 10 matching lines @@
 
 void ResourceTracker::ReleaseResourceSoon(PP_Resource res) {
   MessageLoop::current()->PostNonNestableTask(
       FROM_HERE,
       base::Bind(&ResourceTracker::ReleaseResource,
              weak_ptr_factory_.GetWeakPtr(),
              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.
   if (instance_map_.find(instance) != instance_map_.end())
     return;
   instance_map_[instance] = linked_ptr<InstanceData>(new InstanceData);
 }
 
 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
   // twice in a few cases. It would be nice not to do that and assert here
   // instead.
   if (found_instance == instance_map_.end())
     return;
 
   InstanceData& data = *found_instance->second;
 
@@ skipping 34 matching lines @@
     ResourceMap::iterator found_resource = live_resources_.find(*cur);
     if (found_resource != live_resources_.end())
       found_resource->second.first->InstanceWasDeleted();
     cur++;
   }
 
   instance_map_.erase(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;
   return static_cast<int>(found->second->resources.size());
 }
 
 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;
 
   // Allocate an ID. Note there's a rare error condition below that means we
   // could end up not using |new_id|, but that's harmless.
   PP_Resource new_id = MakeTypedId(++last_resource_value_, PP_ID_TYPE_RESOURCE);
 
   // Some objects have a 0 instance, meaning they aren't associated with any
   // instance, so they won't be in |instance_map_|. This is (as of this writing)
@@ skipping 11 matching lines @@
       return 0;
     }
     found->second->resources.insert(new_id);
   }
 
   live_resources_[new_id] = ResourceAndRefCount(object, 0);
   return new_id;
 }
 
 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())
     found->second->resources.erase(pp_resource);
   live_resources_.erase(pp_resource);
 }
 
 void ResourceTracker::LastPluginRefWasDeleted(Resource* object) {

[dmichael (off chromium), 2012/09/06 02:57:26]

Is there any reason not to check here too? Poking around, it does look like it's
only called from other ResourceTracker functions. Maybe you could make it
private, and then it would be more obvious that you don't have to check here?

[raymes, 2012/09/06 16:23:59]

Done.

   // Bug http://crbug.com/134611 indicates that sometimes the resource tracker
   // is null here. This should never be the case since if we have a resource in
   // the tracker, it should always have a valid instance associated with it
   // (except for the resource for the main thread's message loop, which has
   // instance set to 0).
   // As a result, we do some CHECKs here to see what types of problems the
   // instance might have before dispatching.
   //
   // TODO(brettw) remove these checks when this bug is no longer relevant.
   // Note, we do an imperfect check here; this might be a loop that's not the
   // main one.
   const bool is_message_loop = (object->AsPPB_MessageLoop_API() != NULL);
   CHECK(object->pp_instance() || is_message_loop);
   CallbackTracker* callback_tracker =
       PpapiGlobals::Get()->GetCallbackTrackerForInstance(object->pp_instance());
   CHECK(callback_tracker || is_message_loop);
   if (callback_tracker)
     callback_tracker->PostAbortForResource(object->pp_resource());
   object->LastPluginRefWasDeleted();
 }
 
 }  // namespace ppapi