summaryrefslogtreecommitdiffstats
path: root/webkit
diff options
context:
space:
mode:
Diffstat (limited to 'webkit')
-rw-r--r--webkit/plugins/ppapi/ppapi_plugin_instance.cc4
-rw-r--r--webkit/plugins/ppapi/ppapi_unittest.cc2
-rw-r--r--webkit/plugins/ppapi/resource_tracker.cc145
-rw-r--r--webkit/plugins/ppapi/resource_tracker.h38
-rw-r--r--webkit/plugins/ppapi/resource_tracker_unittest.cc106
-rw-r--r--webkit/plugins/ppapi/var.cc11
-rw-r--r--webkit/plugins/ppapi/var.h10
7 files changed, 226 insertions, 90 deletions
diff --git a/webkit/plugins/ppapi/ppapi_plugin_instance.cc b/webkit/plugins/ppapi/ppapi_plugin_instance.cc
index 474c2dd..7ab4713 100644
--- a/webkit/plugins/ppapi/ppapi_plugin_instance.cc
+++ b/webkit/plugins/ppapi/ppapi_plugin_instance.cc
@@ -327,8 +327,8 @@ PluginInstance::~PluginInstance() {
// unregister themselves inside the delete call).
PluginObjectSet plugin_object_copy;
live_plugin_objects_.swap(plugin_object_copy);
- for (PluginObjectSet::iterator i = live_plugin_objects_.begin();
- i != live_plugin_objects_.end(); ++i)
+ for (PluginObjectSet::iterator i = plugin_object_copy.begin();
+ i != plugin_object_copy.end(); ++i)
delete *i;
delegate_->InstanceDeleted(this);
diff --git a/webkit/plugins/ppapi/ppapi_unittest.cc b/webkit/plugins/ppapi/ppapi_unittest.cc
index bc3039d..7ad58c3 100644
--- a/webkit/plugins/ppapi/ppapi_unittest.cc
+++ b/webkit/plugins/ppapi/ppapi_unittest.cc
@@ -100,6 +100,8 @@ void PpapiUnittest::SetUp() {
}
void PpapiUnittest::TearDown() {
+ instance_ = NULL;
+ module_ = NULL;
}
const void* PpapiUnittest::GetMockInterface(const char* interface_name) const {
diff --git a/webkit/plugins/ppapi/resource_tracker.cc b/webkit/plugins/ppapi/resource_tracker.cc
index 6507e2d..4874c79 100644
--- a/webkit/plugins/ppapi/resource_tracker.cc
+++ b/webkit/plugins/ppapi/resource_tracker.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -11,6 +11,7 @@
#include "base/logging.h"
#include "base/rand_util.h"
#include "ppapi/c/pp_resource.h"
+#include "ppapi/c/pp_var.h"
#include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
#include "webkit/plugins/ppapi/resource.h"
#include "webkit/plugins/ppapi/var.h"
@@ -82,18 +83,31 @@ PP_Resource ResourceTracker::AddResource(Resource* resource) {
// Add the resource with plugin use-count 1.
PP_Resource new_id = MakeTypedId(++last_resource_id_, PP_ID_TYPE_RESOURCE);
live_resources_.insert(std::make_pair(new_id, std::make_pair(resource, 1)));
- instance_to_resources_[resource->instance()->pp_instance()].insert(new_id);
+
+ // Track associated with the instance.
+ PP_Instance pp_instance = resource->instance()->pp_instance();
+ DCHECK(instance_map_.find(pp_instance) != instance_map_.end());
+ instance_map_[pp_instance].resources.insert(new_id);
return new_id;
}
int32 ResourceTracker::AddVar(Var* var) {
// If the plugin manages to create 1B strings...
- if (last_var_id_ == std::numeric_limits<int32>::max() >> kPPIdTypeBits) {
+ if (last_var_id_ == std::numeric_limits<int32>::max() >> kPPIdTypeBits)
return 0;
- }
+
// Add the resource with plugin use-count 1.
int32 new_id = MakeTypedId(++last_var_id_, PP_ID_TYPE_VAR);
live_vars_.insert(std::make_pair(new_id, std::make_pair(var, 1)));
+
+ // Object vars must be associated with the instance.
+ ObjectVar* object_var = var->AsObjectVar();
+ if (object_var) {
+ PP_Instance instance = object_var->instance()->pp_instance();
+ DCHECK(instance_map_.find(instance) != instance_map_.end());
+ instance_map_[instance].object_vars.insert(new_id);
+ }
+
return new_id;
}
@@ -119,13 +133,12 @@ bool ResourceTracker::UnrefResource(PP_Resource res) {
if (i != live_resources_.end()) {
if (!--i->second.second) {
Resource* to_release = i->second.first;
+ // LastPluginRefWasDeleted will clear the instance pointer, so save it
+ // first.
+ PP_Instance instance = to_release->instance()->pp_instance();
to_release->LastPluginRefWasDeleted(false);
- ResourceSet& instance_resource_set =
- instance_to_resources_[to_release->instance()->pp_instance()];
- DCHECK(instance_resource_set.find(res) != instance_resource_set.end());
- instance_resource_set.erase(res);
-
+ instance_map_[instance].resources.erase(res);
live_resources_.erase(i);
}
return true;
@@ -134,43 +147,21 @@ bool ResourceTracker::UnrefResource(PP_Resource res) {
}
}
-void ResourceTracker::ForceDeletePluginResourceRefs(PP_Resource res) {
- 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; // Nothing to do.
-
- i->second.second = 0;
- Resource* resource = i->second.first;
-
- // Must delete from the resource set first since the resource's instance
- // pointer will get zeroed out in LastPluginRefWasDeleted.
- ResourceSet& resource_set = instance_to_resources_[
- resource->instance()->pp_instance()];
- DCHECK(resource_set.find(res) != resource_set.end());
- resource_set.erase(res);
-
- resource->LastPluginRefWasDeleted(true);
- live_resources_.erase(i);
-}
-
uint32 ResourceTracker::GetLiveObjectsForInstance(
PP_Instance instance) const {
- InstanceToResourceMap::const_iterator found =
- instance_to_resources_.find(instance);
- if (found == instance_to_resources_.end())
+ InstanceMap::const_iterator found = instance_map_.find(instance);
+ if (found == instance_map_.end())
return 0;
- return static_cast<uint32>(found->second.size());
+ return static_cast<uint32>(found->second.resources.size() +
+ found->second.object_vars.size());
}
scoped_refptr<Var> ResourceTracker::GetVar(int32 var_id) const {
DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR))
<< var_id << " is not a PP_Var ID.";
VarMap::const_iterator result = live_vars_.find(var_id);
- if (result == live_vars_.end()) {
+ if (result == live_vars_.end())
return scoped_refptr<Var>();
- }
return result->second.first;
}
@@ -193,26 +184,24 @@ bool ResourceTracker::UnrefVar(int32 var_id) {
<< var_id << " is not a PP_Var ID.";
VarMap::iterator i = live_vars_.find(var_id);
if (i != live_vars_.end()) {
- if (!--i->second.second)
+ if (!--i->second.second) {
+ ObjectVar* object_var = i->second.first->AsObjectVar();
+ if (object_var) {
+ instance_map_[object_var->instance()->pp_instance()].object_vars.erase(
+ var_id);
+ }
live_vars_.erase(i);
+ }
return true;
}
return false;
}
PP_Instance ResourceTracker::AddInstance(PluginInstance* instance) {
-#ifndef NDEBUG
- // Make sure we're not adding one more than once.
- for (InstanceMap::const_iterator i = instance_map_.begin();
- i != instance_map_.end(); ++i)
- DCHECK(i->second != instance);
-#endif
+ DCHECK(instance_map_.find(instance->pp_instance()) == instance_map_.end());
- // Use a random 64-bit number for the instance ID. This helps prevent some
- // mischeif where you could misallocate resources if you gave a different
- // instance ID.
- //
- // See also AddModule below.
+ // Use a random number for the instance ID. This helps prevent some
+ // accidents. See also AddModule below.
//
// Need to make sure the random number isn't a duplicate or 0.
PP_Instance new_instance;
@@ -221,32 +210,64 @@ PP_Instance ResourceTracker::AddInstance(PluginInstance* instance) {
PP_ID_TYPE_INSTANCE);
} while (!new_instance ||
instance_map_.find(new_instance) != instance_map_.end());
- instance_map_[new_instance] = instance;
+
+ instance_map_[new_instance].instance = instance;
return new_instance;
}
void ResourceTracker::InstanceDeleted(PP_Instance instance) {
DLOG_IF(ERROR, !CheckIdType(instance, PP_ID_TYPE_INSTANCE))
<< instance << " is not a PP_Instance.";
+ InstanceMap::iterator found = instance_map_.find(instance);
+ if (found == instance_map_.end()) {
+ NOTREACHED();
+ return;
+ }
+ InstanceData& data = found->second;
+
// Force release all plugin references to resources associated with the
// deleted instance.
- ResourceSet& resource_set = instance_to_resources_[instance];
- ResourceSet::iterator i = resource_set.begin();
- while (i != resource_set.end()) {
+ ResourceSet::iterator cur_res = data.resources.begin();
+ while (cur_res != data.resources.end()) {
+ ResourceMap::iterator found_resource = live_resources_.find(*cur_res);
+ if (found_resource == live_resources_.end()) {
+ NOTREACHED();
+ } else {
+ Resource* resource = found_resource->second.first;
+
+ // Must delete from the resource set first since the resource's instance
+ // pointer will get zeroed out in LastPluginRefWasDeleted.
+ resource->LastPluginRefWasDeleted(true);
+ live_resources_.erase(*cur_res);
+ }
+
// Iterators to a set are stable so we can iterate the set while the items
// are being deleted as long as we're careful not to delete the item we're
// holding an iterator to.
- ResourceSet::iterator current = i++;
- ForceDeletePluginResourceRefs(*current);
+ ResourceSet::iterator current = cur_res++;
+ data.resources.erase(current);
}
- DCHECK(resource_set.empty());
- instance_to_resources_.erase(instance);
-
- InstanceMap::iterator found = instance_map_.find(instance);
- if (found == instance_map_.end()) {
- NOTREACHED();
- return;
+ DCHECK(data.resources.empty());
+
+ // Force delete all var references.
+ VarSet::iterator cur_var = data.object_vars.begin();
+ while (cur_var != data.object_vars.end()) {
+ VarSet::iterator current = cur_var++;
+
+ // Tell the corresponding ObjectVar that the instance is gone.
+ PP_Var object_pp_var;
+ object_pp_var.type = PP_VARTYPE_OBJECT;
+ object_pp_var.value.as_id = *current;
+ scoped_refptr<ObjectVar> object_var(ObjectVar::FromPPVar(object_pp_var));
+ if (object_var.get())
+ object_var->InstanceDeleted();
+
+ // Clear the object from the var mapping and the live instance object list.
+ live_vars_.erase(*current);
+ data.object_vars.erase(*current);
}
+ DCHECK(data.object_vars.empty());
+
instance_map_.erase(found);
}
@@ -256,7 +277,7 @@ PluginInstance* ResourceTracker::GetInstance(PP_Instance instance) {
InstanceMap::iterator found = instance_map_.find(instance);
if (found == instance_map_.end())
return NULL;
- return found->second;
+ return found->second.instance;
}
PP_Module ResourceTracker::AddModule(PluginModule* module) {
diff --git a/webkit/plugins/ppapi/resource_tracker.h b/webkit/plugins/ppapi/resource_tracker.h
index ce577c3..32a4c7b 100644
--- a/webkit/plugins/ppapi/resource_tracker.h
+++ b/webkit/plugins/ppapi/resource_tracker.h
@@ -53,15 +53,6 @@ class ResourceTracker {
bool AddRefResource(PP_Resource res);
bool UnrefResource(PP_Resource res);
- // Forces the plugin refcount of the given resource to 0. This is used when
- // the instance is destroyed and we want to free all resources.
- //
- // Note that this may not necessarily delete the resource object since the
- // regular refcount is maintained separately from the plugin refcount and
- // random components in the Pepper implementation could still have
- // references to it.
- void ForceDeletePluginResourceRefs(PP_Resource res);
-
// Returns the number of resources associated with this module.
uint32 GetLiveObjectsForInstance(PP_Instance instance) const;
@@ -106,6 +97,24 @@ class ResourceTracker {
friend class ResourceTrackerTest;
friend class Var;
+ typedef std::set<PP_Resource> ResourceSet;
+
+ // Indexed by the var ID.
+ typedef std::set<int32> VarSet;
+
+ // Per-instance data we track.
+ struct InstanceData {
+ InstanceData() : instance(0) {}
+
+ // Non-owning pointer to the instance object. When a PluginInstance is
+ // destroyed, it will notify us and we'll delete all associated data.
+ PluginInstance* instance;
+
+ // Resources and object vars associated with the instance.
+ ResourceSet resources;
+ VarSet object_vars;
+ };
+
// Prohibit creation other then by the Singleton class.
ResourceTracker();
~ResourceTracker();
@@ -146,15 +155,8 @@ class ResourceTracker {
typedef base::hash_map<int32, VarAndRefCount> VarMap;
VarMap live_vars_;
- // Tracks all resources associated with each instance. This is used to
- // delete resources when the instance has been destroyed to avoid leaks.
- typedef std::set<PP_Resource> ResourceSet;
- typedef std::map<PP_Instance, ResourceSet> InstanceToResourceMap;
- InstanceToResourceMap instance_to_resources_;
-
- // Tracks all live instances. The pointers are non-owning, the PluginInstance
- // destructor will notify us when the instance is deleted.
- typedef std::map<PP_Instance, PluginInstance*> InstanceMap;
+ // Tracks all live instances and their associated data.
+ typedef std::map<PP_Instance, InstanceData> InstanceMap;
InstanceMap instance_map_;
// Tracks all live modules. The pointers are non-owning, the PluginModule
diff --git a/webkit/plugins/ppapi/resource_tracker_unittest.cc b/webkit/plugins/ppapi/resource_tracker_unittest.cc
index 6e7b2a4..ead6b57 100644
--- a/webkit/plugins/ppapi/resource_tracker_unittest.cc
+++ b/webkit/plugins/ppapi/resource_tracker_unittest.cc
@@ -1,20 +1,26 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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 "webkit/plugins/ppapi/ppapi_unittest.h"
+#include "base/scoped_ptr.h"
+#include "ppapi/c/pp_var.h"
#include "ppapi/c/ppp_instance.h"
+#include "third_party/npapi/bindings/npruntime.h"
#include "webkit/plugins/ppapi/mock_plugin_delegate.h"
#include "webkit/plugins/ppapi/mock_resource.h"
#include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
#include "webkit/plugins/ppapi/resource_tracker.h"
+#include "webkit/plugins/ppapi/var.h"
namespace webkit {
namespace ppapi {
namespace {
+// Tracked Resources -----------------------------------------------------------
+
class TrackedMockResource : public MockResource {
public:
static int tracked_objects_alive;
@@ -29,20 +35,59 @@ class TrackedMockResource : public MockResource {
int TrackedMockResource::tracked_objects_alive = 0;
+// Tracked NPObjects -----------------------------------------------------------
+
+int g_npobjects_alive = 0;
+
+void TrackedClassDeallocate(NPObject* npobject) {
+ g_npobjects_alive--;
+}
+
+NPClass g_tracked_npclass = {
+ NP_CLASS_STRUCT_VERSION,
+ NULL,
+ &TrackedClassDeallocate,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+};
+
+// Returns a new tracked NPObject with a refcount of 1.
+NPObject* NewTrackedNPObject() {
+ NPObject* object = new NPObject;
+ object->_class = &g_tracked_npclass;
+ object->referenceCount = 1;
+
+ g_npobjects_alive++;
+ return object;
+}
+
} // namespace
+// ResourceTrackerTest ---------------------------------------------------------
+
class ResourceTrackerTest : public PpapiUnittest {
public:
ResourceTrackerTest() {
}
virtual void SetUp() {
- PpapiUnittest::SetUp();
+ // The singleton override must be installed before the generic setup because
+ // that creates an instance, etc. which uses the tracker.
ResourceTracker::SetSingletonOverride(&tracker_);
+ PpapiUnittest::SetUp();
}
virtual void TearDown() {
- ResourceTracker::ClearSingletonOverride();
+ // Must do normal tear down before clearing the override for the same rason
+ // as the SetUp.
PpapiUnittest::TearDown();
+ ResourceTracker::ClearSingletonOverride();
}
ResourceTracker& tracker() { return tracker_; }
@@ -93,7 +138,7 @@ TEST_F(ResourceTrackerTest, Ref) {
ASSERT_EQ(0, TrackedMockResource::tracked_objects_alive);
}
-TEST_F(ResourceTrackerTest, ForceDeleteWithInstance) {
+TEST_F(ResourceTrackerTest, DeleteResourceWithInstance) {
// Make a second instance (the test harness already creates & manages one).
scoped_refptr<PluginInstance> instance2(
new PluginInstance(delegate(), module(),
@@ -128,5 +173,58 @@ TEST_F(ResourceTrackerTest, ForceDeleteWithInstance) {
ASSERT_EQ(0, TrackedMockResource::tracked_objects_alive);
}
+TEST_F(ResourceTrackerTest, DeleteObjectVarWithInstance) {
+ // Make a second instance (the test harness already creates & manages one).
+ scoped_refptr<PluginInstance> instance2(
+ new PluginInstance(delegate(), module(),
+ static_cast<const PPP_Instance*>(
+ GetMockInterface(PPP_INSTANCE_INTERFACE))));
+ PP_Instance pp_instance2 = instance2->pp_instance();
+
+ // Make an object var.
+ scoped_ptr<NPObject> npobject(NewTrackedNPObject());
+ PP_Var pp_object = ObjectVar::NPObjectToPPVar(instance2.get(),
+ npobject.get());
+
+ EXPECT_EQ(1, g_npobjects_alive);
+ EXPECT_EQ(1u, tracker().GetLiveObjectsForInstance(pp_instance2));
+
+ // Free the instance, this should release the ObjectVar.
+ instance2 = NULL;
+ EXPECT_EQ(0u, tracker().GetLiveObjectsForInstance(pp_instance2));
+}
+
+// Make sure that using the same NPObject should give the same PP_Var
+// each time.
+TEST_F(ResourceTrackerTest, ReuseVar) {
+ scoped_ptr<NPObject> npobject(NewTrackedNPObject());
+
+ PP_Var pp_object1 = ObjectVar::NPObjectToPPVar(instance(), npobject.get());
+ PP_Var pp_object2 = ObjectVar::NPObjectToPPVar(instance(), npobject.get());
+
+ // The two results should be the same.
+ EXPECT_EQ(pp_object1.value.as_id, pp_object2.value.as_id);
+
+ // The objects should be able to get us back to the associated NPObject.
+ // This ObjectVar must be released before we do NPObjectToPPVar again
+ // below so it gets freed and we get a new identifier.
+ {
+ scoped_refptr<ObjectVar> check_object(ObjectVar::FromPPVar(pp_object1));
+ ASSERT_TRUE(check_object.get());
+ EXPECT_EQ(instance(), check_object->instance());
+ EXPECT_EQ(npobject.get(), check_object->np_object());
+ }
+
+ // Remove both of the refs we made above.
+ tracker().UnrefVar(static_cast<int32_t>(pp_object2.value.as_id));
+ tracker().UnrefVar(static_cast<int32_t>(pp_object1.value.as_id));
+
+ // Releasing the resource should free the internal ref, and so making a new
+ // one now should generate a new ID.
+ PP_Var pp_object3 = ObjectVar::NPObjectToPPVar(instance(), npobject.get());
+ EXPECT_NE(pp_object1.value.as_id, pp_object3.value.as_id);
+ tracker().UnrefVar(static_cast<int32_t>(pp_object3.value.as_id));
+}
+
} // namespace ppapi
} // namespace webkit
diff --git a/webkit/plugins/ppapi/var.cc b/webkit/plugins/ppapi/var.cc
index f5cae25..b8071ed 100644
--- a/webkit/plugins/ppapi/var.cc
+++ b/webkit/plugins/ppapi/var.cc
@@ -825,7 +825,8 @@ ObjectVar::ObjectVar(PluginInstance* instance, NPObject* np_object)
}
ObjectVar::~ObjectVar() {
- instance_->RemoveNPObjectVar(this);
+ if (instance_)
+ instance_->RemoveNPObjectVar(this);
WebBindings::releaseObject(np_object_);
}
@@ -833,8 +834,14 @@ ObjectVar* ObjectVar::AsObjectVar() {
return this;
}
+void ObjectVar::InstanceDeleted() {
+ DCHECK(instance_);
+ instance_ = NULL;
+}
+
// static
PP_Var ObjectVar::NPObjectToPPVar(PluginInstance* instance, NPObject* object) {
+ DCHECK(object);
scoped_refptr<ObjectVar> object_var(instance->ObjectVarForNPObject(object));
if (!object_var) // No object for this module yet, make a new one.
object_var = new ObjectVar(instance, object);
@@ -842,7 +849,7 @@ PP_Var ObjectVar::NPObjectToPPVar(PluginInstance* instance, NPObject* object) {
if (!object_var)
return PP_MakeUndefined();
- // Convert to a PP_Var, GetReference will AddRef for us.
+ // Convert to a PP_Var, GetID will AddRef for us.
PP_Var result;
result.type = PP_VARTYPE_OBJECT;
result.value.as_id = object_var->GetID();
diff --git a/webkit/plugins/ppapi/var.h b/webkit/plugins/ppapi/var.h
index ef3613f..4571698 100644
--- a/webkit/plugins/ppapi/var.h
+++ b/webkit/plugins/ppapi/var.h
@@ -179,6 +179,13 @@ class ObjectVar : public Var {
// Guaranteed non-NULL.
NPObject* np_object() const { return np_object_; }
+ // Notification that the instance was deleted, the internal pointer will be
+ // NULLed out.
+ void InstanceDeleted();
+
+ // Possibly NULL if the object has outlived its instance.
+ PluginInstance* instance() const { return instance_; }
+
// Helper function to create a PP_Var of type object that contains the given
// NPObject for use byt he given module. Calling this function multiple times
// given the same module + NPObject results in the same PP_Var, assuming that
@@ -196,8 +203,6 @@ class ObjectVar : public Var {
// if the PP_Var is not of object type or the object is invalid.
static scoped_refptr<ObjectVar> FromPPVar(PP_Var var);
- PluginInstance* instance() const { return instance_; }
-
protected:
// You should always use FromNPObject to create an ObjectVar. This function
// guarantees that we maintain the 1:1 mapping between NPObject and
@@ -205,6 +210,7 @@ class ObjectVar : public Var {
ObjectVar(PluginInstance* instance, NPObject* np_object);
private:
+ // Possibly NULL if the object has outlived its instance.
PluginInstance* instance_;
// Guaranteed non-NULL, this is the underlying object used by WebKit. We
generated by cgit v1.1 at 2025-03-02 23:57:26 +0000