PPAPI: Make PPB_MessageLoop_Dev::GetForMainThread work

ppapi/shared_impl/resource_tracker.cc

Index: ppapi/shared_impl/resource_tracker.cc
diff --git a/ppapi/shared_impl/resource_tracker.cc b/ppapi/shared_impl/resource_tracker.cc
index f3632d89cdddd8fe3d93d8203a7161dfaa4bf9f6..8cfcfb52c35c387cc5fe758535b6afaf528fcd6d 100644
--- a/ppapi/shared_impl/resource_tracker.cc
+++ b/ppapi/shared_impl/resource_tracker.cc
@@ -92,7 +92,7 @@ void ResourceTracker::DidCreateInstance(PP_Instance instance) {
 void ResourceTracker::DidDeleteInstance(PP_Instance instance) {
   InstanceMap::iterator found_instance = instance_map_.find(instance);
 
-  // Due to the infrastructure of some tests, the instance is uyregistered
+  // 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())
@@ -155,20 +155,27 @@ PP_Resource ResourceTracker::AddResource(Resource* object) {
   if (last_resource_value_ == kMaxPPId)
     return 0;
 
-  InstanceMap::iterator found = instance_map_.find(object->pp_instance());
-  if (found == instance_map_.end()) {
-    // If you hit this, it's likely somebody forgot to call DidCreateInstance,
-    // the resource was created with an invalid PP_Instance, or the renderer
-    // side tried to create a resource for a plugin that crashed/exited. This
-    // could happen for OOP plugins where due to reentrancies in context of
-    // outgoing sync calls the renderer can send events after a plugin has
-    // exited.
-    DLOG(INFO) << "Failed to find plugin instance in instance map";
-    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);
-  found->second->resources.insert(new_id);
+
+  // 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)
+  // only true of the PPB_MessageLoop resource for the main thread.

[bbudge, 2012/08/29 22:26:58]

Close ')' or just delete '('

Could the key '0' be used for resources not associated with instances? Would
there be any benefit to tracking those like that?

[dmichael (off chromium), 2012/08/30 15:59:10]

I think it's closed?
"(as of this writing)"
I thought about that. I can't think of any benefit for the 1 resource like this
we have so far, and it might be slightly more complex. E.g., I'd have to make
sure at shutdown to call DidDeleteInstance somewhere so that the ResourceTracker
doesn't leak them.

As far as I can tell, it's almost 6 of one, half dozen of the other. We can
change later if we come up with a reason to put these things in the tracker.

+  if (object->pp_instance()) {
+    InstanceMap::iterator found = instance_map_.find(object->pp_instance());
+    if (found == instance_map_.end()) {
+      // If you hit this, it's likely somebody forgot to call DidCreateInstance,
+      // the resource was created with an invalid PP_Instance, or the renderer
+      // side tried to create a resource for a plugin that crashed/exited. This
+      // could happen for OOP plugins where due to reentrancies in context of
+      // outgoing sync calls the renderer can send events after a plugin has
+      // exited.
+      DLOG(INFO) << "Failed to find plugin instance in instance map";
+      return 0;
+    }
+    found->second->resources.insert(new_id);
+  }
 
   live_resources_[new_id] = ResourceAndRefCount(object, 0);
   return new_id;
@@ -185,16 +192,22 @@ void ResourceTracker::RemoveResource(Resource* object) {
 void ResourceTracker::LastPluginRefWasDeleted(Resource* object) {
   // 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.
+  // 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.
-  CHECK(object->pp_instance());
+  // 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);
-  callback_tracker->PostAbortForResource(object->pp_resource());
+  CHECK(callback_tracker || is_message_loop);
+  if (callback_tracker)
+    callback_tracker->PostAbortForResource(object->pp_resource());
   object->LastPluginRefWasDeleted();
 }