PPAPI: Make PPB_MessageLoop_Dev::GetForMainThread work

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"
@@ skipping 74 matching lines @@
   // 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) {
   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())
     return;
 
   InstanceData& data = *found_instance->second;
 
   // Force release all plugin references to resources associated with the
   // deleted instance. Make a copy since as we iterate through them, each one
   // will remove itself from the tracking info individually.
@@ skipping 42 matching lines @@
   if (found == instance_map_.end())
     return 0;
   return static_cast<int>(found->second->resources.size());
 }
 
 PP_Resource ResourceTracker::AddResource(Resource* object) {
   // If the plugin manages to create too many resources, don't do crazy stuff.
   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);
+
+  // 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);
   }
 
-  PP_Resource new_id = MakeTypedId(++last_resource_value_, PP_ID_TYPE_RESOURCE);
-  found->second->resources.insert(new_id);
-
   live_resources_[new_id] = ResourceAndRefCount(object, 0);
   return new_id;
 }
 
 void ResourceTracker::RemoveResource(Resource* object) {
   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) {
   // 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();
 }
 
 }  // namespace ppapi