diff options
author | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-08-09 23:14:13 +0000 |
---|---|---|
committer | brettw@chromium.org <brettw@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-08-09 23:14:13 +0000 |
commit | 2bbd2c670008e30aaaef6c3c25ae37e0c17f8c3f (patch) | |
tree | 79595eeb026dd29841a7380fd7753f992c2e466b | |
parent | 32131b9030d8313f7adc9b765f706ffbee7ca709 (diff) | |
download | chromium_src-2bbd2c670008e30aaaef6c3c25ae37e0c17f8c3f.zip chromium_src-2bbd2c670008e30aaaef6c3c25ae37e0c17f8c3f.tar.gz chromium_src-2bbd2c670008e30aaaef6c3c25ae37e0c17f8c3f.tar.bz2 |
Unify var tracking between webkit and the proxy.
This replaces the var tracking in the proxy with the var tracking in the
shared_impl that's used by the implementation. It adds a new ProxyObjectVar
to be the proxied plugin analog of NPObjectVar in the impl. This new object
just keeps track of the host data.
The tricky part is to make the var tracker able to do all the crazy messaging.
This adds some virtual functions to the shared var tracker that we override
in the plugin in PluginVarTracker.
This removes the calls to the GetLiveObjectsForInstance in the var deprecated
test. It turns out this function really can't be implemented properly in the
proxy, and I don't know why it even worked before. A Release() call posts a
non-nestable task so the object isn't released until later. So to implement
the proxy for GetLiveObjectsForInstance we would also need to post a
non-nestable task. But when the test runs we're getting called from within
the plugin, so blocking on a non-nestable task deadlocks. So I just gave up
and deleted the parts of the test that uses it.
TEST=included
BUG=none
Review URL: http://codereview.chromium.org/7578001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@96094 0039d316-1c4b-4281-b951-d872f2087c98
41 files changed, 987 insertions, 844 deletions
diff --git a/ppapi/ppapi_proxy.gypi b/ppapi/ppapi_proxy.gypi index c354980..07ceae2 100644 --- a/ppapi/ppapi_proxy.gypi +++ b/ppapi/ppapi_proxy.gypi @@ -148,6 +148,8 @@ 'proxy/proxy_channel.h', 'proxy/proxy_module.cc', 'proxy/proxy_module.h', + 'proxy/proxy_object_var.cc', + 'proxy/proxy_object_var.h', 'proxy/resource_creation_proxy.cc', 'proxy/resource_creation_proxy.h', 'proxy/serialized_flash_menu.cc', diff --git a/ppapi/ppapi_shared.gypi b/ppapi/ppapi_shared.gypi index 01862b4..6f3caf5 100644 --- a/ppapi/ppapi_shared.gypi +++ b/ppapi/ppapi_shared.gypi @@ -38,6 +38,8 @@ 'shared_impl/function_group_base.h', 'shared_impl/graphics_3d_impl.cc', 'shared_impl/graphics_3d_impl.h', + 'shared_impl/id_assignment.cc', + 'shared_impl/id_assignment.h', 'shared_impl/image_data_impl.cc', 'shared_impl/image_data_impl.h', 'shared_impl/input_event_impl.cc', @@ -60,6 +62,8 @@ 'shared_impl/url_util_impl.h', 'shared_impl/var.cc', 'shared_impl/var.h', + 'shared_impl/var_tracker.cc', + 'shared_impl/var_tracker.h', 'shared_impl/video_decoder_impl.cc', 'shared_impl/video_decoder_impl.h', 'shared_impl/webkit_forwarding.cc', diff --git a/ppapi/proxy/plugin_resource_tracker.cc b/ppapi/proxy/plugin_resource_tracker.cc index 85f8a7d..cabf9cb 100644 --- a/ppapi/proxy/plugin_resource_tracker.cc +++ b/ppapi/proxy/plugin_resource_tracker.cc @@ -57,7 +57,8 @@ PluginResourceTracker::ResourceInfo::operator=( // Start counting resources at a high number to avoid collisions with vars (to // help debugging). PluginResourceTracker::PluginResourceTracker() - : last_resource_id_(0x00100000) { + : var_tracker_test_override_(NULL), + last_resource_id_(0x00100000) { } PluginResourceTracker::~PluginResourceTracker() { @@ -148,32 +149,8 @@ PP_Instance PluginResourceTracker::GetInstanceForResource( return found->second.resource->instance(); } -int32 PluginResourceTracker::AddVar(ppapi::Var* var) { - // TODO(brettw) implement this when the proxy uses the Var object in the - // plugin process. - NOTREACHED(); - return 0; -} - -scoped_refptr<ppapi::Var> PluginResourceTracker::GetVar(int32 var_id) const { - // TODO(brettw) implement this when the proxy uses the Var object in the - // plugin process. - NOTREACHED(); - return scoped_refptr<ppapi::Var>(); -} - -bool PluginResourceTracker::AddRefVar(int32 var_id) { - // TODO(brettw) implement this when the proxy uses the Var object in the - // plugin process. - NOTREACHED(); - return false; -} - -bool PluginResourceTracker::UnrefVar(int32 var_id) { - // TODO(brettw) implement this when the proxy uses the Var object in the - // plugin process. - NOTREACHED(); - return false; +ppapi::VarTracker* PluginResourceTracker::GetVarTracker() { + return &var_tracker(); } void PluginResourceTracker::ReleasePluginResourceRef( diff --git a/ppapi/proxy/plugin_resource_tracker.h b/ppapi/proxy/plugin_resource_tracker.h index 9da3902..b96831c 100644 --- a/ppapi/proxy/plugin_resource_tracker.h +++ b/ppapi/proxy/plugin_resource_tracker.h @@ -16,6 +16,7 @@ #include "ppapi/c/pp_resource.h" #include "ppapi/c/pp_var.h" #include "ppapi/proxy/host_resource.h" +#include "ppapi/proxy/plugin_var_tracker.h" #include "ppapi/shared_impl/tracker_base.h" template<typename T> struct DefaultSingletonTraits; @@ -59,17 +60,23 @@ class PluginResourceTracker : public ::ppapi::TrackerBase { PP_Resource PluginResourceForHostResource( const HostResource& resource) const; + PluginVarTracker& var_tracker() { + return var_tracker_test_override_ ? *var_tracker_test_override_ + : var_tracker_; + } + + void set_var_tracker_test_override(PluginVarTracker* t) { + var_tracker_test_override_ = t; + } + // TrackerBase. - virtual ::ppapi::ResourceObjectBase* GetResourceAPI( + virtual ppapi::ResourceObjectBase* GetResourceAPI( PP_Resource res) OVERRIDE; - virtual ::ppapi::FunctionGroupBase* GetFunctionAPI( + virtual ppapi::FunctionGroupBase* GetFunctionAPI( PP_Instance inst, pp::proxy::InterfaceID id) OVERRIDE; virtual PP_Instance GetInstanceForResource(PP_Resource resource) OVERRIDE; - virtual int32 AddVar(ppapi::Var* var); - virtual scoped_refptr< ::ppapi::Var > GetVar(int32 var_id) const; - virtual bool AddRefVar(int32 var_id); - virtual bool UnrefVar(int32 var_id); + virtual ppapi::VarTracker* GetVarTracker() OVERRIDE; private: friend struct DefaultSingletonTraits<PluginResourceTracker>; @@ -94,6 +101,17 @@ class PluginResourceTracker : public ::ppapi::TrackerBase { void ReleasePluginResourceRef(const PP_Resource& var, bool notify_browser_on_release); + // Use the var_tracker_test_override_ instead if it's non-NULL. + // + // TODO(brettw) this should be somehow separated out from here. I'm thinking + // of some global object that manages PPAPI globals, including separate var + // and resource trackers. + PluginVarTracker var_tracker_; + + // Non-owning pointer to a var tracker mock used by tests. NULL when no + // test implementation is provided. + PluginVarTracker* var_tracker_test_override_; + // Map of plugin resource IDs to the information tracking that resource. typedef std::map<PP_Resource, ResourceInfo> ResourceMap; ResourceMap resource_map_; diff --git a/ppapi/proxy/plugin_var_serialization_rules.cc b/ppapi/proxy/plugin_var_serialization_rules.cc index 8ba5574..8140969 100644 --- a/ppapi/proxy/plugin_var_serialization_rules.cc +++ b/ppapi/proxy/plugin_var_serialization_rules.cc @@ -6,13 +6,17 @@ #include "base/logging.h" #include "ppapi/proxy/plugin_dispatcher.h" +#include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/plugin_var_tracker.h" +#include "ppapi/shared_impl/var.h" + +using ppapi::StringVar; namespace pp { namespace proxy { PluginVarSerializationRules::PluginVarSerializationRules() - : var_tracker_(PluginVarTracker::GetInstance()) { + : var_tracker_(&PluginResourceTracker::GetInstance()->var_tracker()) { } PluginVarSerializationRules::~PluginVarSerializationRules() { @@ -26,9 +30,9 @@ PP_Var PluginVarSerializationRules::SendCallerOwned(const PP_Var& var, // Retrieve the string to use for IPC. if (var.type == PP_VARTYPE_STRING) { - const std::string* var_string = var_tracker_->GetExistingString(var); - if (var_string) - *str_val = *var_string; + scoped_refptr<StringVar> string_var(StringVar::FromPPVar(var)); + if (string_var.get()) + *str_val = string_var->value(); else NOTREACHED() << "Trying to send unknown string over IPC."; } @@ -39,13 +43,8 @@ PP_Var PluginVarSerializationRules::BeginReceiveCallerOwned( const PP_Var& var, const std::string* str_val, Dispatcher* dispatcher) { - if (var.type == PP_VARTYPE_STRING) { - // Convert the string to the context of the current process. - PP_Var ret; - ret.type = PP_VARTYPE_STRING; - ret.value.as_id = var_tracker_->MakeString(*str_val); - return ret; - } + if (var.type == PP_VARTYPE_STRING) + return StringVar::StringToPPVar(0, *str_val); if (var.type == PP_VARTYPE_OBJECT) { DCHECK(dispatcher->IsPlugin()); @@ -59,7 +58,7 @@ PP_Var PluginVarSerializationRules::BeginReceiveCallerOwned( void PluginVarSerializationRules::EndReceiveCallerOwned(const PP_Var& var) { if (var.type == PP_VARTYPE_STRING) { // Destroy the string BeginReceiveCallerOwned created above. - var_tracker_->Release(var); + var_tracker_->ReleaseVar(var); } else if (var.type == PP_VARTYPE_OBJECT) { var_tracker_->StopTrackingObjectWithNoReference(var); } @@ -68,13 +67,8 @@ void PluginVarSerializationRules::EndReceiveCallerOwned(const PP_Var& var) { PP_Var PluginVarSerializationRules::ReceivePassRef(const PP_Var& var, const std::string& str_val, Dispatcher* dispatcher) { - if (var.type == PP_VARTYPE_STRING) { - // Convert the string to the context of the current process. - PP_Var ret; - ret.type = PP_VARTYPE_STRING; - ret.value.as_id = var_tracker_->MakeString(str_val); - return ret; - } + if (var.type == PP_VARTYPE_STRING) + return StringVar::StringToPPVar(0, str_val); // Overview of sending an object with "pass ref" from the browser to the // plugin: @@ -124,9 +118,9 @@ PP_Var PluginVarSerializationRules::BeginSendPassRef(const PP_Var& var, return var_tracker_->GetHostObject(var); if (var.type == PP_VARTYPE_STRING) { - const std::string* var_string = var_tracker_->GetExistingString(var); - if (var_string) - *str_val = *var_string; + scoped_refptr<StringVar> string_var(StringVar::FromPPVar(var)); + if (string_var.get()) + *str_val = string_var->value(); else NOTREACHED() << "Trying to send unknown string over IPC."; } @@ -146,7 +140,7 @@ void PluginVarSerializationRules::EndSendPassRef(const PP_Var& var, } void PluginVarSerializationRules::ReleaseObjectRef(const PP_Var& var) { - var_tracker_->Release(var); + var_tracker_->ReleaseVar(var); } } // namespace proxy diff --git a/ppapi/proxy/plugin_var_tracker.cc b/ppapi/proxy/plugin_var_tracker.cc index 16d5992..0c834a5 100644 --- a/ppapi/proxy/plugin_var_tracker.cc +++ b/ppapi/proxy/plugin_var_tracker.cc @@ -10,18 +10,16 @@ #include "ppapi/proxy/plugin_dispatcher.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/interface_id.h" +#include "ppapi/proxy/proxy_object_var.h" +#include "ppapi/shared_impl/var.h" + +using ppapi::ProxyObjectVar; +using ppapi::Var; namespace pp { namespace proxy { -namespace { - -// When non-NULL, this object overrides the VarTrackerSingleton. -PluginVarTracker* var_tracker_override = NULL; - -} // namespace - -PluginVarTracker::HostVar::HostVar(PluginDispatcher* d, VarID i) +PluginVarTracker::HostVar::HostVar(PluginDispatcher* d, int32 i) : dispatcher(d), host_object_id(i) { } @@ -34,160 +32,33 @@ bool PluginVarTracker::HostVar::operator<(const HostVar& other) const { return host_object_id < other.host_object_id; } -PluginVarTracker::PluginVarInfo::PluginVarInfo(const HostVar& host_var) - : host_var(host_var), - ref_count(0), - track_with_no_reference_count(0) { -} - -PluginVarTracker::PluginVarTracker() : last_plugin_var_id_(0) { +PluginVarTracker::PluginVarTracker() { } PluginVarTracker::~PluginVarTracker() { } -// static -void PluginVarTracker::SetInstanceForTest(PluginVarTracker* tracker) { - var_tracker_override = tracker; -} - -// static -PluginVarTracker* PluginVarTracker::GetInstance() { - if (var_tracker_override) - return var_tracker_override; - return Singleton<PluginVarTracker>::get(); -} - -PluginVarTracker::VarID PluginVarTracker::MakeString(const std::string& str) { - return MakeString(str.c_str(), str.length()); -} - -PluginVarTracker::VarID PluginVarTracker::MakeString(const char* str, - uint32_t len) { - std::pair<VarIDStringMap::iterator, bool> - iter_success_pair(var_id_to_string_.end(), false); - VarID new_id(0); - RefCountedStringPtr str_ptr(new RefCountedString(str, len)); - // Pick new IDs until one is successfully inserted. This loop is very unlikely - // to ever run a 2nd time, since we have ~2^63 possible IDs to exhaust. - while (!iter_success_pair.second) { - new_id = GetNewVarID(); - iter_success_pair = - var_id_to_string_.insert(VarIDStringMap::value_type(new_id, str_ptr)); - } - // Release the local pointer. - str_ptr = NULL; - // Now the map should have the only reference. - DCHECK(iter_success_pair.first->second->HasOneRef()); - iter_success_pair.first->second->AddRef(); - return new_id; -} - -const std::string* PluginVarTracker::GetExistingString( - const PP_Var& var) const { - if (var.type != PP_VARTYPE_STRING) - return NULL; - VarIDStringMap::const_iterator found = - var_id_to_string_.find(var.value.as_id); - if (found != var_id_to_string_.end()) - return &found->second->value(); - return NULL; -} - -void PluginVarTracker::AddRef(const PP_Var& var) { - if (var.type == PP_VARTYPE_STRING) { - VarIDStringMap::iterator found = var_id_to_string_.find(var.value.as_id); - if (found == var_id_to_string_.end()) { - NOTREACHED() << "Requesting to addref an unknown string."; - return; - } - found->second->AddRef(); - } else if (var.type == PP_VARTYPE_OBJECT && var.value.as_id != 0) { - PluginVarInfoMap::iterator found = plugin_var_info_.find(var.value.as_id); - if (found == plugin_var_info_.end()) { - NOTREACHED() << "Requesting to addref an unknown object."; - return; - } - - PluginVarInfo& info = found->second; - if (info.ref_count == 0) { - // Got an AddRef for an object we have no existing reference for. - // We need to tell the browser we've taken a ref. This comes up when the - // browser passes an object as an input param and holds a ref for us. - // This must be a sync message since otherwise the "addref" will actually - // occur after the return to the browser of the sync function that - // presumably sent the object. - SendAddRefObjectMsg(info.host_var); - } - info.ref_count++; - } -} - -void PluginVarTracker::Release(const PP_Var& var) { - if (var.type == PP_VARTYPE_STRING) { - VarIDStringMap::iterator found = var_id_to_string_.find(var.value.as_id); - if (found == var_id_to_string_.end()) { - NOTREACHED() << "Requesting to release an unknown string."; - return; - } - found->second->Release(); - // If there is only 1 reference left, it's the map's reference. Erase it - // from the map, which will delete the string. - if (found->second->HasOneRef()) - var_id_to_string_.erase(found); - } else if (var.type == PP_VARTYPE_OBJECT) { - PluginVarInfoMap::iterator found = plugin_var_info_.find(var.value.as_id); - if (found == plugin_var_info_.end()) { - NOTREACHED() << "Requesting to release an unknown object."; - return; - } - - PluginVarInfo& info = found->second; - if (info.ref_count == 0) { - NOTREACHED() << "Releasing an object with zero ref."; - return; - } - - info.ref_count--; - if (info.ref_count == 0) - SendReleaseObjectMsg(info.host_var); - DeletePluginVarInfoIfNecessary(found); - } -} - -PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& var, +PP_Var PluginVarTracker::ReceiveObjectPassRef(const PP_Var& host_var, PluginDispatcher* dispatcher) { - DCHECK(var.type == PP_VARTYPE_OBJECT); - - // Find the plugin info. - PluginVarInfoMap::iterator found = - FindOrMakePluginVarFromHostVar(var, dispatcher); - if (found == plugin_var_info_.end()) { - // The above code should have always made an entry in the map. - NOTREACHED(); - return PP_MakeUndefined(); - } + DCHECK(host_var.type == PP_VARTYPE_OBJECT); - // Fix up the references. The host (renderer) just sent us a ref. The - // renderer has addrefed the var in its tracker for us since it's returning - // it. - PluginVarInfo& info = found->second; - if (info.ref_count == 0) { - // We don't have a reference to this already, then we just add it to our - // tracker and use that one ref. - info.ref_count = 1; - } else { - // We already have a reference to it, that means the renderer now has two - // references on our behalf. We want to transfer that extra reference to - // our list. This means we addref in the plugin, and release the extra one - // in the renderer. - SendReleaseObjectMsg(info.host_var); - info.ref_count++; + // Get the object. + scoped_refptr<ProxyObjectVar> object( + FindOrMakePluginVarFromHostVar(host_var, dispatcher)); + + // Actually create the PP_Var, this will add all the tracking info but not + // adjust any refcounts. + PP_Var ret = GetOrCreateObjectVarID(object.get()); + + VarInfo& info = GetLiveVar(ret)->second; + if (info.ref_count > 0) { + // We already had a reference to it before. That means the renderer now has + // two references on our behalf. We want to transfer that extra reference + // to our list. This means we addref in the plugin, and release the extra + // one in the renderer. + SendReleaseObjectMsg(*object); } - - PP_Var ret; - ret.type = PP_VARTYPE_OBJECT; - ret.value.as_id = found->first; + info.ref_count++; return ret; } @@ -196,58 +67,66 @@ PP_Var PluginVarTracker::TrackObjectWithNoReference( PluginDispatcher* dispatcher) { DCHECK(host_var.type == PP_VARTYPE_OBJECT); - PluginVarInfoMap::iterator found = - FindOrMakePluginVarFromHostVar(host_var, dispatcher); - if (found == plugin_var_info_.end()) { - // The above code should have always made an entry in the map. - NOTREACHED(); - return PP_MakeUndefined(); - } + // Get the object. + scoped_refptr<ProxyObjectVar> object( + FindOrMakePluginVarFromHostVar(host_var, dispatcher)); - found->second.track_with_no_reference_count++; + // Actually create the PP_Var, this will add all the tracking info but not + // adjust any refcounts. + PP_Var ret = GetOrCreateObjectVarID(object.get()); - PP_Var ret; - ret.type = PP_VARTYPE_OBJECT; - ret.value.as_id = found->first; + VarInfo& info = GetLiveVar(ret)->second; + info.track_with_no_reference_count++; return ret; } void PluginVarTracker::StopTrackingObjectWithNoReference( const PP_Var& plugin_var) { DCHECK(plugin_var.type == PP_VARTYPE_OBJECT); - PluginVarInfoMap::iterator found = plugin_var_info_.find( - plugin_var.value.as_id); - if (found == plugin_var_info_.end()) { + VarMap::iterator found = GetLiveVar(plugin_var); + if (found == live_vars_.end()) { NOTREACHED(); return; } + DCHECK(found->second.track_with_no_reference_count > 0); found->second.track_with_no_reference_count--; - DeletePluginVarInfoIfNecessary(found); + DeleteObjectInfoIfNecessary(found); } PP_Var PluginVarTracker::GetHostObject(const PP_Var& plugin_object) const { - DCHECK(plugin_object.type == PP_VARTYPE_OBJECT); - PluginVarInfoMap::const_iterator found = plugin_var_info_.find( - plugin_object.value.as_id); - if (found == plugin_var_info_.end()) { + if (plugin_object.type != PP_VARTYPE_OBJECT) { NOTREACHED(); return PP_MakeUndefined(); } + + Var* var = GetVar(plugin_object); + ProxyObjectVar* object = var->AsProxyObjectVar(); + if (!object) { + NOTREACHED(); + return PP_MakeUndefined(); + } + + // Make a var with the host ID. PP_Var ret; ret.type = PP_VARTYPE_OBJECT; - ret.value.as_id = found->second.host_var.host_object_id; + ret.value.as_id = object->host_var_id(); return ret; } PluginDispatcher* PluginVarTracker::DispatcherForPluginObject( const PP_Var& plugin_object) const { - DCHECK(plugin_object.type == PP_VARTYPE_OBJECT); - PluginVarInfoMap::const_iterator found = plugin_var_info_.find( - plugin_object.value.as_id); - if (found != plugin_var_info_.end()) - return found->second.host_var.dispatcher; - return NULL; + if (plugin_object.type != PP_VARTYPE_OBJECT) + return NULL; + + VarMap::const_iterator found = GetLiveVar(plugin_object); + if (found == live_vars_.end()) + return NULL; + + ProxyObjectVar* object = found->second.var->AsProxyObjectVar(); + if (!object) + return NULL; + return object->dispatcher(); } void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher, @@ -255,85 +134,145 @@ void PluginVarTracker::ReleaseHostObject(PluginDispatcher* dispatcher, // Convert the host object to a normal var valid in the plugin. DCHECK(host_object.type == PP_VARTYPE_OBJECT); HostVarToPluginVarMap::iterator found = host_var_to_plugin_var_.find( - HostVar(dispatcher, host_object.value.as_id)); + HostVar(dispatcher, static_cast<int32>(host_object.value.as_id))); if (found == host_var_to_plugin_var_.end()) { NOTREACHED(); return; } - // Now just release the object like normal. - PP_Var plugin_object; - plugin_object.type = PP_VARTYPE_OBJECT; - plugin_object.value.as_id = found->second; - Release(plugin_object); + // Now just release the object given the plugin var ID. + ReleaseVar(found->second); } int PluginVarTracker::GetRefCountForObject(const PP_Var& plugin_object) { - PluginVarInfoMap::iterator found = plugin_var_info_.find( - plugin_object.value.as_id); - if (found == plugin_var_info_.end()) + VarMap::iterator found = GetLiveVar(plugin_object); + if (found == live_vars_.end()) return -1; return found->second.ref_count; } int PluginVarTracker::GetTrackedWithNoReferenceCountForObject( const PP_Var& plugin_object) { - PluginVarInfoMap::iterator found = plugin_var_info_.find( - plugin_object.value.as_id); - if (found == plugin_var_info_.end()) + VarMap::iterator found = GetLiveVar(plugin_object); + if (found == live_vars_.end()) return -1; return found->second.track_with_no_reference_count; } -void PluginVarTracker::SendAddRefObjectMsg(const HostVar& host_var) { +int32 PluginVarTracker::AddVarInternal(Var* var, AddVarRefMode mode) { + // Normal adding. + int32 new_id = VarTracker::AddVarInternal(var, mode); + + // Need to add proxy objects to the host var map. + ProxyObjectVar* proxy_object = var->AsProxyObjectVar(); + if (proxy_object) { + HostVar host_var(proxy_object->dispatcher(), proxy_object->host_var_id()); + DCHECK(host_var_to_plugin_var_.find(host_var) == + host_var_to_plugin_var_.end()); // Adding an object twice, use + // FindOrMakePluginVarFromHostVar. + host_var_to_plugin_var_[host_var] = new_id; + } + return new_id; +} + +void PluginVarTracker::TrackedObjectGettingOneRef(VarMap::const_iterator iter) { + ProxyObjectVar* object = iter->second.var->AsProxyObjectVar(); + if (!object) { + NOTREACHED(); + return; + } + + DCHECK(iter->second.ref_count == 0); + + // Got an AddRef for an object we have no existing reference for. + // We need to tell the browser we've taken a ref. This comes up when the + // browser passes an object as an input param and holds a ref for us. + // This must be a sync message since otherwise the "addref" will actually + // occur after the return to the browser of the sync function that + // presumably sent the object. + SendAddRefObjectMsg(*object); +} + +void PluginVarTracker::ObjectGettingZeroRef(VarMap::iterator iter) { + ProxyObjectVar* object = iter->second.var->AsProxyObjectVar(); + if (!object) { + NOTREACHED(); + return; + } + + // Notify the host we're no longer holding our ref. + DCHECK(iter->second.ref_count == 0); + SendReleaseObjectMsg(*object); + + // This will optionally delete the info from live_vars_. + VarTracker::ObjectGettingZeroRef(iter); +} + +bool PluginVarTracker::DeleteObjectInfoIfNecessary(VarMap::iterator iter) { + // Get the info before calling the base class's version of this function, + // which may delete the object. + ProxyObjectVar* object = iter->second.var->AsProxyObjectVar(); + HostVar host_var(object->dispatcher(), object->host_var_id()); + + if (!VarTracker::DeleteObjectInfoIfNecessary(iter)) + return false; + + // Clean up the host var mapping. + DCHECK(host_var_to_plugin_var_.find(host_var) != + host_var_to_plugin_var_.end()); + host_var_to_plugin_var_.erase(host_var); + return true; +} + +PP_Var PluginVarTracker::GetOrCreateObjectVarID(ProxyObjectVar* object) { + // We can't use object->GetPPVar() because we don't want to affect the + // refcount, so we have to add everything manually here. + int32 var_id = object->GetExistingVarID(); + if (!var_id) { + var_id = AddVarInternal(object, ADD_VAR_CREATE_WITH_NO_REFERENCE); + object->AssignVarID(var_id); + } + + PP_Var ret; + ret.type = PP_VARTYPE_OBJECT; + ret.value.as_id = var_id; + return ret; +} + +void PluginVarTracker::SendAddRefObjectMsg( + const ProxyObjectVar& proxy_object) { int unused; - host_var.dispatcher->Send(new PpapiHostMsg_PPBVar_AddRefObject( - INTERFACE_ID_PPB_VAR_DEPRECATED, host_var.host_object_id, &unused)); + proxy_object.dispatcher()->Send(new PpapiHostMsg_PPBVar_AddRefObject( + INTERFACE_ID_PPB_VAR_DEPRECATED, proxy_object.host_var_id(), &unused)); } -void PluginVarTracker::SendReleaseObjectMsg(const HostVar& host_var) { - host_var.dispatcher->Send(new PpapiHostMsg_PPBVar_ReleaseObject( - INTERFACE_ID_PPB_VAR_DEPRECATED, host_var.host_object_id)); +void PluginVarTracker::SendReleaseObjectMsg( + const ProxyObjectVar& proxy_object) { + proxy_object.dispatcher()->Send(new PpapiHostMsg_PPBVar_ReleaseObject( + INTERFACE_ID_PPB_VAR_DEPRECATED, proxy_object.host_var_id())); } -PluginVarTracker::PluginVarInfoMap::iterator -PluginVarTracker::FindOrMakePluginVarFromHostVar(const PP_Var& var, - PluginDispatcher* dispatcher) { +scoped_refptr<ProxyObjectVar> PluginVarTracker::FindOrMakePluginVarFromHostVar( + const PP_Var& var, + PluginDispatcher* dispatcher) { DCHECK(var.type == PP_VARTYPE_OBJECT); HostVar host_var(dispatcher, var.value.as_id); HostVarToPluginVarMap::iterator found = host_var_to_plugin_var_.find(host_var); - if (found != host_var_to_plugin_var_.end()) { - PluginVarInfoMap::iterator ret = plugin_var_info_.find(found->second); - DCHECK(ret != plugin_var_info_.end()); - return ret; // Already know about this var return the ID. + if (found == host_var_to_plugin_var_.end()) { + // Create a new object. + return scoped_refptr<ProxyObjectVar>( + new ProxyObjectVar(dispatcher, static_cast<int32>(var.value.as_id))); } - // Make a new var, adding references to both maps. - VarID new_plugin_var_id = GetNewVarID(); - host_var_to_plugin_var_[host_var] = new_plugin_var_id; - return plugin_var_info_.insert( - std::make_pair(new_plugin_var_id, PluginVarInfo(host_var))).first; -} - -void PluginVarTracker::DeletePluginVarInfoIfNecessary( - PluginVarInfoMap::iterator iter) { - if (iter->second.ref_count != 0 || - iter->second.track_with_no_reference_count != 0) - return; // Object still alive. - - // Object ref counts are all zero, delete from both maps. - DCHECK(host_var_to_plugin_var_.find(iter->second.host_var) != - host_var_to_plugin_var_.end()); - host_var_to_plugin_var_.erase(iter->second.host_var); - plugin_var_info_.erase(iter); -} + // Have this host var, look up the object. + VarMap::iterator ret = live_vars_.find(found->second); + DCHECK(ret != live_vars_.end()); -PluginVarTracker::VarID PluginVarTracker::GetNewVarID() { - if (last_plugin_var_id_ == std::numeric_limits<VarID>::max()) - last_plugin_var_id_ = 0; - return ++last_plugin_var_id_; + // All objects should be proxy objects. + DCHECK(ret->second.var->AsProxyObjectVar()); + return scoped_refptr<ProxyObjectVar>(ret->second.var->AsProxyObjectVar()); } } // namesace proxy diff --git a/ppapi/proxy/plugin_var_tracker.h b/ppapi/proxy/plugin_var_tracker.h index 748ec2d..c6f3c93 100644 --- a/ppapi/proxy/plugin_var_tracker.h +++ b/ppapi/proxy/plugin_var_tracker.h @@ -8,68 +8,46 @@ #include <map> #include <string> +#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "ppapi/c/pp_stdint.h" #include "ppapi/c/pp_var.h" +#include "ppapi/shared_impl/var_tracker.h" struct PPB_Var; template<typename T> struct DefaultSingletonTraits; +namespace ppapi { +class ProxyObjectVar; +} + namespace pp { namespace proxy { class PluginDispatcher; // Tracks live strings and objects in the plugin process. -// -// This object maintains its own object IDs that are used by the plugin. These -// IDs can be mapped to the renderer that created them, and that renderer's ID. -// This way, we can maintain multiple renderers each giving us objects, and the -// plugin can work with them using a uniform set of unique IDs. -// -// We maintain our own reference count for objects. a single ref in the -// renderer process whenever we have a nonzero refcount in the plugin process. -// This allows AddRef and Release to not initiate too much IPC chat. -// -// In addition to the local reference count, we also maintain "tracked objects" -// which are objects that the plugin is aware of, but doesn't hold a reference -// to. This will happen when the plugin is passed an object as an argument from -// the host (renderer) but where a reference is not passed. -class PluginVarTracker { +class PluginVarTracker : public ppapi::VarTracker { public: - typedef int64_t VarID; - - // Called by tests that want to specify a specific VarTracker. This allows - // them to use a unique one each time and avoids singletons sticking around - // across tests. - static void SetInstanceForTest(PluginVarTracker* tracker); - - // Returns the global var tracker for the plugin object. - static PluginVarTracker* GetInstance(); - - // Allocates a string and returns the ID of it. The refcount will be 1. - VarID MakeString(const std::string& str); - VarID MakeString(const char* str, uint32_t len); - - // Returns a pointer to the given string if it exists, or NULL if the var - // isn't a string var. - const std::string* GetExistingString(const PP_Var& plugin_var) const; - - void AddRef(const PP_Var& plugin_var); - void Release(const PP_Var& plugin_var); + PluginVarTracker(); + ~PluginVarTracker(); // Manages tracking for receiving a VARTYPE_OBJECT from the remote side // (either the plugin or the renderer) that has already had its reference // count incremented on behalf of the caller. PP_Var ReceiveObjectPassRef(const PP_Var& var, PluginDispatcher* dispatcher); + // See the comment in var_tracker.h for more about what a tracked object is. + // This adds and releases the "track_with_no_reference_count" for a given + // object. PP_Var TrackObjectWithNoReference(const PP_Var& host_var, PluginDispatcher* dispatcher); void StopTrackingObjectWithNoReference(const PP_Var& plugin_var); // Returns the host var for the corresponding plugin object var. The object - // should be a VARTYPE_OBJECT + // should be a VARTYPE_OBJECT. The reference count is not affeceted. PP_Var GetHostObject(const PP_Var& plugin_object) const; PluginDispatcher* DispatcherForPluginObject( @@ -86,34 +64,20 @@ class PluginVarTracker { int GetRefCountForObject(const PP_Var& plugin_object); int GetTrackedWithNoReferenceCountForObject(const PP_Var& plugin_object); + protected: + // VarTracker protected overrides. + virtual int32 AddVarInternal(::ppapi::Var* var, AddVarRefMode mode) OVERRIDE; + virtual void TrackedObjectGettingOneRef(VarMap::const_iterator iter) OVERRIDE; + virtual void ObjectGettingZeroRef(VarMap::iterator iter) OVERRIDE; + virtual bool DeleteObjectInfoIfNecessary(VarMap::iterator iter) OVERRIDE; + private: friend struct DefaultSingletonTraits<PluginVarTracker>; friend class PluginProxyTestHarness; - class RefCountedString : public base::RefCounted<RefCountedString> { - public: - RefCountedString() { - } - RefCountedString(const std::string& str) : value_(str) { - } - RefCountedString(const char* data, size_t len) - : value_(data, len) { - } - - const std::string& value() const { return value_; } - - private: - std::string value_; - - // Ensure only base::RefCounted can delete a RefCountedString. - friend void base::RefCounted<RefCountedString>::Release() const; - virtual ~RefCountedString() {} - }; - typedef scoped_refptr<RefCountedString> RefCountedStringPtr; - // Represents a var as received from the host. struct HostVar { - HostVar(PluginDispatcher* d, int64_t i); + HostVar(PluginDispatcher* d, int32 i); bool operator<(const HostVar& other) const; @@ -124,70 +88,32 @@ class PluginVarTracker { // The object ID that the host generated to identify the object. This is // unique only within that host: different hosts could give us different // objects with the same ID. - VarID host_object_id; - }; - - // The information associated with a var object in the plugin. - struct PluginVarInfo { - PluginVarInfo(const HostVar& host_var); - - // Maps back to the original var in the host. - HostVar host_var; - - // Explicit reference count. This value is affected by the renderer calling - // AddRef and Release. A nonzero value here is represented by a single - // reference in the host on our behalf (this reduces IPC traffic). - int32_t ref_count; - - // Tracked object count (see class comment above). - // - // "TrackObjectWithNoReference" might be called recursively in rare cases. - // For example, say the host calls a plugin function with an object as an - // argument, and in response, the plugin calls a host function that then - // calls another (or the same) plugin function with the same object. - // - // This value tracks the number of calls to TrackObjectWithNoReference so - // we know when we can stop tracking this object. - int32_t track_with_no_reference_count; + int32 host_object_id; }; - typedef std::map<int64_t, PluginVarInfo> PluginVarInfoMap; - - PluginVarTracker(); - ~PluginVarTracker(); + // Returns the existing var ID for the given object var, creating and + // assigning an ID to it if necessary. This does not affect the reference + // count, so in the creation case the refcount will be 0. It's assumed in + // this case the caller will either adjust the refcount or the + // track_with_no_reference_count. + PP_Var GetOrCreateObjectVarID(ppapi::ProxyObjectVar* object); // Sends an addref or release message to the browser for the given object ID. - void SendAddRefObjectMsg(const HostVar& host_var); - void SendReleaseObjectMsg(const HostVar& host_var); + void SendAddRefObjectMsg(const ppapi::ProxyObjectVar& proxy_object); + void SendReleaseObjectMsg(const ppapi::ProxyObjectVar& proxy_object); - PluginVarInfoMap::iterator FindOrMakePluginVarFromHostVar( + // Looks up the given host var. If we already know about it, returns a + // reference to the already-tracked object. If it doesn't creates a new one + // and returns it. If it's created, it's not added to the map. + scoped_refptr<ppapi::ProxyObjectVar> FindOrMakePluginVarFromHostVar( const PP_Var& var, PluginDispatcher* dispatcher); - // Checks the reference counds of the given plugin var info and removes the - // tracking information if necessary. We're done with the object when its - // explicit reference count and its "tracked with no reference" count both - // reach zero. - void DeletePluginVarInfoIfNecessary(PluginVarInfoMap::iterator iter); - - // Tracks all information about plugin vars. - PluginVarInfoMap plugin_var_info_; - - // Maps host vars to plugin vars. This allows us to know if we've previously - // seen a host var and re-use the information. - typedef std::map<HostVar, VarID> HostVarToPluginVarMap; + // Maps host vars in the host to IDs in the plugin process. + typedef std::map<HostVar, int32> HostVarToPluginVarMap; HostVarToPluginVarMap host_var_to_plugin_var_; - // Maps plugin var IDs to ref counted strings. - typedef std::map<VarID, RefCountedStringPtr> VarIDStringMap; - VarIDStringMap var_id_to_string_; - - // The last plugin PP_Var ID we've handed out. This must be unique for the - // process. - VarID last_plugin_var_id_; - - // Get a new Var ID and increment last_plugin_var_id_. - VarID GetNewVarID(); + DISALLOW_COPY_AND_ASSIGN(PluginVarTracker); }; } // namespace proxy diff --git a/ppapi/proxy/plugin_var_tracker_unittest.cc b/ppapi/proxy/plugin_var_tracker_unittest.cc index 08fba9b..01954db 100644 --- a/ppapi/proxy/plugin_var_tracker_unittest.cc +++ b/ppapi/proxy/plugin_var_tracker_unittest.cc @@ -12,21 +12,13 @@ namespace proxy { namespace { -PP_Var MakeObject(PluginVarTracker::VarID object_id) { +PP_Var MakeObject(int32 object_id) { PP_Var ret; ret.type = PP_VARTYPE_OBJECT; ret.value.as_id = object_id; return ret; } -// Creates a PP_Var from the given string ID. -PP_Var MakeString(PluginVarTracker::VarID string_id) { - PP_Var ret; - ret.type = PP_VARTYPE_STRING; - ret.value.as_id = string_id; - return ret; -} - } // namespace class PluginVarTrackerTest : public PluginProxyTest { @@ -36,7 +28,7 @@ class PluginVarTrackerTest : public PluginProxyTest { protected: // Asserts that there is a unique "release object" IPC message in the test // sink. This will return the var ID from the message or -1 if none found. - PluginVarTracker::VarID GetObjectIDForUniqueReleaseObject() { + int32 GetObjectIDForUniqueReleaseObject() { const IPC::Message* release_msg = sink().GetUniqueMessageMatching( PpapiHostMsg_PPBVar_ReleaseObject::ID); if (!release_msg) @@ -48,23 +40,6 @@ class PluginVarTrackerTest : public PluginProxyTest { } }; -TEST_F(PluginVarTrackerTest, Strings) { - std::string str("Hello"); - PluginVarTracker::VarID str_id1 = var_tracker().MakeString(str); - EXPECT_NE(0, str_id1); - - PluginVarTracker::VarID str_id2 = var_tracker().MakeString( - str.c_str(), static_cast<uint32_t>(str.size())); - EXPECT_NE(0, str_id2); - - // Make sure the strings come out the other end. - const std::string* result = - var_tracker().GetExistingString(MakeString(str_id1)); - EXPECT_EQ(str, *result); - result = var_tracker().GetExistingString(MakeString(str_id2)); - EXPECT_EQ(str, *result); -} - TEST_F(PluginVarTrackerTest, GetHostObject) { PP_Var host_object = MakeObject(12345); @@ -76,7 +51,7 @@ TEST_F(PluginVarTrackerTest, GetHostObject) { EXPECT_EQ(PP_VARTYPE_OBJECT, host_object2.type); EXPECT_EQ(host_object.value.as_id, host_object2.value.as_id); - var_tracker().Release(plugin_object); + var_tracker().ReleaseVar(plugin_object); } TEST_F(PluginVarTrackerTest, ReceiveObjectPassRef) { @@ -106,9 +81,9 @@ TEST_F(PluginVarTrackerTest, ReceiveObjectPassRef) { // Release the object, one ref at a time. The second release should free // the tracking data and send a release message to the browser. - var_tracker().Release(plugin_object); + var_tracker().ReleaseVar(plugin_object); EXPECT_EQ(1, var_tracker().GetRefCountForObject(plugin_object)); - var_tracker().Release(plugin_object); + var_tracker().ReleaseVar(plugin_object); EXPECT_EQ(-1, var_tracker().GetRefCountForObject(plugin_object)); EXPECT_EQ(host_object.value.as_id, GetObjectIDForUniqueReleaseObject()); } @@ -129,7 +104,7 @@ TEST_F(PluginVarTrackerTest, FreeTrackedAndReferencedObject) { // Free via the refcount, this should release the object to the browser but // maintain the tracked object. - var_tracker().Release(plugin_var); + var_tracker().ReleaseVar(plugin_var); EXPECT_EQ(0, var_tracker().GetRefCountForObject(plugin_var)); EXPECT_EQ(1u, sink().message_count()); EXPECT_EQ(host_object.value.as_id, GetObjectIDForUniqueReleaseObject()); @@ -156,7 +131,7 @@ TEST_F(PluginVarTrackerTest, FreeTrackedAndReferencedObject) { EXPECT_EQ(0u, sink().message_count()); // Now free via the refcount, this should delete it. - var_tracker().Release(plugin_var); + var_tracker().ReleaseVar(plugin_var); EXPECT_EQ(-1, var_tracker().GetRefCountForObject(plugin_var)); EXPECT_EQ(host_object.value.as_id, GetObjectIDForUniqueReleaseObject()); } diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc index c8bb7eb..eb7b440 100644 --- a/ppapi/proxy/ppapi_proxy_test.cc +++ b/ppapi/proxy/ppapi_proxy_test.cc @@ -148,7 +148,7 @@ Dispatcher* PluginProxyTestHarness::GetDispatcher() { void PluginProxyTestHarness::SetUpHarness() { // These must be first since the dispatcher set-up uses them. PluginResourceTracker::SetInstanceForTest(&resource_tracker_); - PluginVarTracker::SetInstanceForTest(&var_tracker_); + resource_tracker_.set_var_tracker_test_override(&var_tracker_); plugin_dispatcher_.reset(new PluginDispatcher( base::Process::Current().handle(), @@ -164,7 +164,7 @@ void PluginProxyTestHarness::SetUpHarnessWithChannel( bool is_client) { // These must be first since the dispatcher set-up uses them. PluginResourceTracker::SetInstanceForTest(&resource_tracker_); - PluginVarTracker::SetInstanceForTest(&var_tracker_); + resource_tracker_.set_var_tracker_test_override(&var_tracker_); plugin_delegate_mock_.Init(ipc_message_loop, shutdown_event); plugin_dispatcher_.reset(new PluginDispatcher( @@ -180,7 +180,6 @@ void PluginProxyTestHarness::TearDownHarness() { plugin_dispatcher_->DidDestroyInstance(pp_instance()); plugin_dispatcher_.reset(); - PluginVarTracker::SetInstanceForTest(NULL); PluginResourceTracker::SetInstanceForTest(NULL); } diff --git a/ppapi/proxy/ppb_file_ref_proxy.cc b/ppapi/proxy/ppb_file_ref_proxy.cc index de91996..5e4b706 100644 --- a/ppapi/proxy/ppb_file_ref_proxy.cc +++ b/ppapi/proxy/ppb_file_ref_proxy.cc @@ -75,8 +75,10 @@ FileRef::FileRef(const PPBFileRef_CreateInfo& info) } FileRef::~FileRef() { - PluginVarTracker::GetInstance()->Release(path_); - PluginVarTracker::GetInstance()->Release(name_); + PluginVarTracker& var_tracker = + PluginResourceTracker::GetInstance()->var_tracker(); + var_tracker.ReleaseVar(path_); + var_tracker.ReleaseVar(name_); } PPB_FileRef_API* FileRef::AsPPB_FileRef_API() { @@ -88,12 +90,12 @@ PP_FileSystemType FileRef::GetFileSystemType() const { } PP_Var FileRef::GetName() const { - PluginVarTracker::GetInstance()->AddRef(name_); + PluginResourceTracker::GetInstance()->var_tracker().AddRefVar(name_); return name_; } PP_Var FileRef::GetPath() const { - PluginVarTracker::GetInstance()->AddRef(path_); + PluginResourceTracker::GetInstance()->var_tracker().AddRefVar(path_); return path_; } diff --git a/ppapi/proxy/ppb_font_proxy.cc b/ppapi/proxy/ppb_font_proxy.cc index 38b00e2..4ce6543 100644 --- a/ppapi/proxy/ppb_font_proxy.cc +++ b/ppapi/proxy/ppb_font_proxy.cc @@ -13,10 +13,12 @@ #include "ppapi/proxy/serialized_var.h" #include "ppapi/shared_impl/ppapi_preferences.h" #include "ppapi/shared_impl/resource_object_base.h" +#include "ppapi/shared_impl/var.h" #include "ppapi/thunk/enter.h" #include "ppapi/thunk/ppb_image_data_api.h" #include "ppapi/thunk/thunk.h" +using ppapi::StringVar; using ppapi::thunk::EnterResourceNoLock; using ppapi::thunk::PPB_ImageData_API; using ppapi::WebKitForwarding; @@ -28,12 +30,11 @@ namespace { bool PPTextRunToTextRun(const PP_TextRun_Dev* run, WebKitForwarding::Font::TextRun* output) { - const std::string* str = PluginVarTracker::GetInstance()->GetExistingString( - run->text); + scoped_refptr<StringVar> str(StringVar::FromPPVar(run->text)); if (!str) return false; - output->text = *str; + output->text = str->value(); output->rtl = PP_ToBool(run->rtl); output->override_direction = PP_ToBool(run->override_direction); return true; @@ -82,10 +83,7 @@ PP_Var PPB_Font_Proxy::GetFontFamilies(PP_Instance instance) { new PpapiHostMsg_PPBFont_GetFontFamilies(&families)); } - PP_Var result; - result.type = PP_VARTYPE_STRING; - result.value.as_id = PluginVarTracker::GetInstance()->MakeString(families); - return result; + return StringVar::StringToPPVar(0, families); } bool PPB_Font_Proxy::OnMessageReceived(const IPC::Message& msg) { @@ -99,8 +97,7 @@ Font::Font(const HostResource& resource, : PluginResource(resource), webkit_event_(false, false) { TRACE_EVENT0("ppapi proxy", "Font::Font"); - const std::string* face = PluginVarTracker::GetInstance()->GetExistingString( - desc.face); + scoped_refptr<StringVar> face(StringVar::FromPPVar(desc.face)); WebKitForwarding* forwarding = GetDispatcher()->GetWebKitForwarding(); @@ -108,7 +105,7 @@ Font::Font(const HostResource& resource, RunOnWebKitThread(base::Bind(&WebKitForwarding::CreateFontForwarding, base::Unretained(forwarding), &webkit_event_, desc, - face ? *face : std::string(), + face.get() ? face->value() : std::string(), GetDispatcher()->preferences(), &result)); font_forwarding_.reset(result); @@ -135,13 +132,10 @@ PP_Bool Font::Describe(PP_FontDescription_Dev* description, &webkit_event_, description, &face, metrics, &result)); - if (result == PP_TRUE) { - description->face.type = PP_VARTYPE_STRING; - description->face.value.as_id = - PluginVarTracker::GetInstance()->MakeString(face); - } else { - description->face.type = PP_VARTYPE_UNDEFINED; - } + if (PP_ToBool(result)) + description->face = StringVar::StringToPPVar(0, face); + else + description->face = PP_MakeUndefined(); return result; } diff --git a/ppapi/proxy/ppb_input_event_proxy.cc b/ppapi/proxy/ppb_input_event_proxy.cc index 383cf1a..4c265de 100644 --- a/ppapi/proxy/ppb_input_event_proxy.cc +++ b/ppapi/proxy/ppb_input_event_proxy.cc @@ -10,6 +10,7 @@ #include "ppapi/proxy/plugin_var_tracker.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/shared_impl/input_event_impl.h" +#include "ppapi/shared_impl/var.h" #include "ppapi/thunk/thunk.h" using ppapi::InputEventData; @@ -48,10 +49,7 @@ PPB_InputEvent_API* InputEvent::AsPPB_InputEvent_API() { } PP_Var InputEvent::StringToPPVar(const std::string& str) { - PP_Var ret; - ret.type = PP_VARTYPE_STRING; - ret.value.as_id = PluginVarTracker::GetInstance()->MakeString(str); - return ret; + return ppapi::StringVar::StringToPPVar(0, str); } namespace { diff --git a/ppapi/proxy/ppb_url_util_proxy.cc b/ppapi/proxy/ppb_url_util_proxy.cc index b59ef3c..57e55be 100644 --- a/ppapi/proxy/ppb_url_util_proxy.cc +++ b/ppapi/proxy/ppb_url_util_proxy.cc @@ -9,32 +9,22 @@ #include "ppapi/c/dev/ppb_var_deprecated.h" #include "ppapi/c/ppb_core.h" #include "ppapi/proxy/plugin_dispatcher.h" +#include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/shared_impl/url_util_impl.h" +#include "ppapi/shared_impl/var.h" + +using ppapi::StringVar; +using ppapi::URLUtilImpl; namespace pp { namespace proxy { -using ppapi::URLUtilImpl; - namespace { -URLUtilImpl::VarFromUtf8 GetVarFromUtf8() { - const PPB_Var_Deprecated* var_deprecated = - static_cast<const PPB_Var_Deprecated*>( - PluginDispatcher::GetInterfaceFromDispatcher( - PPB_VAR_DEPRECATED_INTERFACE)); - return var_deprecated->VarFromUtf8; -} - -const std::string* GetStringFromVar(PP_Var var) { - return PluginVarTracker::GetInstance()->GetExistingString(var); -} - PP_Var Canonicalize(PP_Var url, PP_URLComponents_Dev* components) { - return URLUtilImpl::Canonicalize(&GetStringFromVar, GetVarFromUtf8(), - 0, url, components); + return URLUtilImpl::Canonicalize(0, url, components); } // Helper function for the functions below that optionally take a components @@ -49,20 +39,19 @@ PP_Var ConvertComponentsAndReturnURL(PP_Var url, if (!components) return url; // Common case - plugin doesn't care about parsing. - const std::string* url_string = GetStringFromVar(url); + scoped_refptr<StringVar> url_string(StringVar::FromPPVar(url)); if (!url_string) return url; PP_Var result = Canonicalize(url, components); - PluginVarTracker::GetInstance()->Release(url); + PluginResourceTracker::GetInstance()->var_tracker().ReleaseVar(url); return result; } PP_Var ResolveRelativeToURL(PP_Var base_url, PP_Var relative, PP_URLComponents_Dev* components) { - return URLUtilImpl::ResolveRelativeToURL(&GetStringFromVar, GetVarFromUtf8(), - 0, base_url, relative, components); + return URLUtilImpl::ResolveRelativeToURL(0, base_url, relative, components); } PP_Var ResolveRelativeToDocument(PP_Instance instance, @@ -81,7 +70,7 @@ PP_Var ResolveRelativeToDocument(PP_Instance instance, } PP_Bool IsSameSecurityOrigin(PP_Var url_a, PP_Var url_b) { - return URLUtilImpl::IsSameSecurityOrigin(&GetStringFromVar, url_a, url_b); + return URLUtilImpl::IsSameSecurityOrigin(url_a, url_b); } PP_Bool DocumentCanRequest(PP_Instance instance, PP_Var url) { diff --git a/ppapi/proxy/ppb_var_deprecated_proxy.cc b/ppapi/proxy/ppb_var_deprecated_proxy.cc index 652bdc8..709178f 100644 --- a/ppapi/proxy/ppb_var_deprecated_proxy.cc +++ b/ppapi/proxy/ppb_var_deprecated_proxy.cc @@ -14,10 +14,14 @@ #include "ppapi/c/ppb_core.h" #include "ppapi/proxy/host_dispatcher.h" #include "ppapi/proxy/plugin_dispatcher.h" +#include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/plugin_var_tracker.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/ppp_class_proxy.h" #include "ppapi/proxy/serialized_var.h" +#include "ppapi/shared_impl/var.h" + +using ppapi::StringVar; namespace pp { namespace proxy { @@ -35,17 +39,20 @@ PluginDispatcher* CheckExceptionAndGetDispatcher(const PP_Var& object, if (exception && exception->type != PP_VARTYPE_UNDEFINED) return NULL; - PluginVarTracker* tracker = PluginVarTracker::GetInstance(); - PluginDispatcher* dispatcher = tracker->DispatcherForPluginObject(object); - if (dispatcher) - return dispatcher; + + if (object.type == PP_VARTYPE_OBJECT) { + // Get the dispatcher for the object. + PluginDispatcher* dispatcher = PluginResourceTracker::GetInstance()-> + var_tracker().DispatcherForPluginObject(object); + if (dispatcher) + return dispatcher; + } // The object is invalid. This means we can't figure out which dispatcher // to use, which is OK because the call will fail anyway. Set the exception. if (exception) { - exception->type = PP_VARTYPE_STRING; - exception->value.as_id = - tracker->MakeString("Attempting to use an invalid object"); + *exception = StringVar::StringToPPVar(0, + std::string("Attempting to use an invalid object")); } return NULL; } @@ -53,27 +60,22 @@ PluginDispatcher* CheckExceptionAndGetDispatcher(const PP_Var& object, // PPP_Var_Deprecated plugin --------------------------------------------------- void AddRefVar(PP_Var var) { - PluginVarTracker::GetInstance()->AddRef(var); + PluginResourceTracker::GetInstance()->var_tracker().AddRefVar(var); } void ReleaseVar(PP_Var var) { - PluginVarTracker::GetInstance()->Release(var); + PluginResourceTracker::GetInstance()->var_tracker().ReleaseVar(var); } PP_Var VarFromUtf8(PP_Module module, const char* data, uint32_t len) { - PP_Var ret = {}; - ret.type = PP_VARTYPE_STRING; - ret.value.as_id = PluginVarTracker::GetInstance()->MakeString( - data, len); - return ret; + return StringVar::StringToPPVar(module, data, len); } const char* VarToUtf8(PP_Var var, uint32_t* len) { - const std::string* str = - PluginVarTracker::GetInstance()->GetExistingString(var); + scoped_refptr<StringVar> str(StringVar::FromPPVar(var)); if (str) { - *len = static_cast<uint32_t>(str->size()); - return str->c_str(); + *len = static_cast<uint32_t>(str->value().size()); + return str->value().c_str(); } *len = 0; return NULL; diff --git a/ppapi/proxy/ppb_var_proxy.cc b/ppapi/proxy/ppb_var_proxy.cc index 063d4d3..82ca00b 100644 --- a/ppapi/proxy/ppb_var_proxy.cc +++ b/ppapi/proxy/ppb_var_proxy.cc @@ -6,7 +6,11 @@ #include "ppapi/c/pp_var.h" #include "ppapi/c/ppb_var.h" +#include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/plugin_var_tracker.h" +#include "ppapi/shared_impl/var.h" + +using ppapi::StringVar; namespace pp { namespace proxy { @@ -16,26 +20,22 @@ namespace { // PPP_Var plugin -------------------------------------------------------------- void AddRefVar(PP_Var var) { - PluginVarTracker::GetInstance()->AddRef(var); + PluginResourceTracker::GetInstance()->var_tracker().AddRefVar(var); } void ReleaseVar(PP_Var var) { - PluginVarTracker::GetInstance()->Release(var); + PluginResourceTracker::GetInstance()->var_tracker().ReleaseVar(var); } PP_Var VarFromUtf8(PP_Module module, const char* data, uint32_t len) { - PP_Var ret = {}; - ret.type = PP_VARTYPE_STRING; - ret.value.as_id = PluginVarTracker::GetInstance()->MakeString(data, len); - return ret; + return StringVar::StringToPPVar(module, data, len); } const char* VarToUtf8(PP_Var var, uint32_t* len) { - const std::string* str = - PluginVarTracker::GetInstance()->GetExistingString(var); + scoped_refptr<StringVar> str(StringVar::FromPPVar(var)); if (str) { - *len = static_cast<uint32_t>(str->size()); - return str->c_str(); + *len = static_cast<uint32_t>(str->value().size()); + return str->value().c_str(); } *len = 0; return NULL; diff --git a/ppapi/proxy/ppp_messaging_proxy.cc b/ppapi/proxy/ppp_messaging_proxy.cc index 39ce89b..a0f467f 100644 --- a/ppapi/proxy/ppp_messaging_proxy.cc +++ b/ppapi/proxy/ppp_messaging_proxy.cc @@ -8,6 +8,7 @@ #include "ppapi/c/ppp_messaging.h" #include "ppapi/proxy/host_dispatcher.h" +#include "ppapi/proxy/plugin_resource_tracker.h" #include "ppapi/proxy/plugin_var_tracker.h" #include "ppapi/proxy/ppapi_messages.h" #include "ppapi/proxy/serialized_var.h" @@ -78,7 +79,7 @@ void PPP_Messaging_Proxy::OnMsgHandleMessage( PP_Var received_var(message_data.Get(dispatcher())); // SerializedVarReceiveInput will decrement the reference count, but we want // to give the recipient a reference. - PluginVarTracker::GetInstance()->AddRef(received_var); + PluginResourceTracker::GetInstance()->var_tracker().AddRefVar(received_var); ppp_messaging_target()->HandleMessage(instance, received_var); } diff --git a/ppapi/proxy/ppp_messaging_proxy_test.cc b/ppapi/proxy/ppp_messaging_proxy_test.cc index ef4143b..ac433a8 100644 --- a/ppapi/proxy/ppp_messaging_proxy_test.cc +++ b/ppapi/proxy/ppp_messaging_proxy_test.cc @@ -9,6 +9,9 @@ #include "ppapi/c/ppb_var.h" #include "ppapi/c/ppp_messaging.h" #include "ppapi/proxy/ppapi_proxy_test.h" +#include "ppapi/shared_impl/var.h" + +using ::ppapi::StringVar; namespace pp { namespace proxy { @@ -126,14 +129,14 @@ TEST_F(PPP_Messaging_ProxyTest, SendMessages) { handle_message_called.Wait(); EXPECT_EQ(expected_instance, received_instance); EXPECT_EQ(expected_var.type, received_var.type); - const std::string* received_string = - plugin().var_tracker().GetExistingString(received_var); - ASSERT_TRUE(received_string); - EXPECT_EQ(kTestString, *received_string); + + scoped_refptr<StringVar> received_string(StringVar::FromPPVar(received_var)); + ASSERT_TRUE(received_string.get()); + EXPECT_EQ(kTestString, received_string->value()); // Now release the var, and the string should go away (because the ref // count should be one). - plugin().var_tracker().Release(received_var); - EXPECT_FALSE(plugin().var_tracker().GetExistingString(received_var)); + plugin().var_tracker().ReleaseVar(received_var); + EXPECT_FALSE(StringVar::FromPPVar(received_var).get()); } } // namespace proxy diff --git a/ppapi/proxy/proxy_object_var.cc b/ppapi/proxy/proxy_object_var.cc new file mode 100644 index 0000000..7d53e33 --- /dev/null +++ b/ppapi/proxy/proxy_object_var.cc @@ -0,0 +1,50 @@ +// 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 "ppapi/proxy/proxy_object_var.h" + +#include "base/logging.h" +#include "ppapi/c/pp_var.h" + +using pp::proxy::PluginDispatcher; + +namespace ppapi { + +ProxyObjectVar::ProxyObjectVar(PluginDispatcher* dispatcher, + int32 host_var_id) + : Var(0), + dispatcher_(dispatcher), + host_var_id_(host_var_id) { + // Should be given valid objects or we'll crash later. + DCHECK(dispatcher_); + DCHECK(host_var_id_); +} + +ProxyObjectVar::~ProxyObjectVar() { +} + +ProxyObjectVar* ProxyObjectVar::AsProxyObjectVar() { + return this; +} + +PP_Var ProxyObjectVar::GetPPVar() { + int32 id = GetOrCreateVarID(); + if (!id) + return PP_MakeNull(); + + PP_Var result; + result.type = PP_VARTYPE_OBJECT; + result.value.as_id = id; + return result; +} + +PP_VarType ProxyObjectVar::GetType() const { + return PP_VARTYPE_OBJECT; +} + +void ProxyObjectVar::AssignVarID(int32 id) { + return Var::AssignVarID(id); +} + +} // namespace ppapi diff --git a/ppapi/proxy/proxy_object_var.h b/ppapi/proxy/proxy_object_var.h new file mode 100644 index 0000000..c07177e --- /dev/null +++ b/ppapi/proxy/proxy_object_var.h @@ -0,0 +1,50 @@ +// 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. + +#ifndef PPAPI_PROXY_PROXY_OBJECT_VAR_H_ +#define PPAPI_PROXY_PROXY_OBJECT_VAR_H_ + +#include "base/compiler_specific.h" +#include "ppapi/shared_impl/var.h" + +namespace pp { +namespace proxy { +class PluginDispatcher; +} // namespace proxy +} // namespace pp + +namespace ppapi { + +// Tracks a reference to an object var in the plugin side of the proxy. This +// just stores the dispatcher and host var ID, and provides the interface for +// integrating this with PP_Var creation. +class ProxyObjectVar : public Var { + public: + ProxyObjectVar(pp::proxy::PluginDispatcher* dispatcher, + int32 host_var_id); + + virtual ~ProxyObjectVar(); + + // Var overrides. + virtual ProxyObjectVar* AsProxyObjectVar() OVERRIDE; + virtual PP_Var GetPPVar() OVERRIDE; + virtual PP_VarType GetType() const OVERRIDE; + + pp::proxy::PluginDispatcher* dispatcher() const { return dispatcher_; } + int32 host_var_id() const { return host_var_id_; } + + // Expose AssignVarID on Var so the PluginResourceTracker can call us when + // it's creating IDs. + void AssignVarID(int32 id); + + private: + pp::proxy::PluginDispatcher* dispatcher_; + int32 host_var_id_; + + DISALLOW_COPY_AND_ASSIGN(ProxyObjectVar); +}; + +} // namespace ppapi + +#endif // PPAPI_PROXY_PROXY_OBJECT_VAR_H_ diff --git a/ppapi/proxy/resource_creation_proxy.cc b/ppapi/proxy/resource_creation_proxy.cc index be65bcb..18f4819 100644 --- a/ppapi/proxy/resource_creation_proxy.cc +++ b/ppapi/proxy/resource_creation_proxy.cc @@ -36,10 +36,12 @@ #include "ppapi/shared_impl/font_impl.h" #include "ppapi/shared_impl/function_group_base.h" #include "ppapi/shared_impl/input_event_impl.h" +#include "ppapi/shared_impl/var.h" #include "ppapi/thunk/enter.h" #include "ppapi/thunk/ppb_image_data_api.h" using ppapi::InputEventData; +using ppapi::StringVar; using ppapi::thunk::ResourceCreationAPI; namespace pp { @@ -206,15 +208,17 @@ PP_Resource ResourceCreationProxy::CreateKeyboardInputEvent( type != PP_INPUTEVENT_TYPE_KEYUP && type != PP_INPUTEVENT_TYPE_CHAR) return 0; - PluginVarTracker* tracker = PluginVarTracker::GetInstance(); - ppapi::InputEventData data; data.event_type = type; data.event_time_stamp = time_stamp; data.event_modifiers = modifiers; data.key_code = key_code; - if (character_text.type == PP_VARTYPE_STRING) - data.character_text = *tracker->GetExistingString(character_text); + if (character_text.type == PP_VARTYPE_STRING) { + scoped_refptr<StringVar> text_str(StringVar::FromPPVar(character_text)); + if (!text_str) + return 0; + data.character_text = text_str->value(); + } return PPB_InputEvent_Proxy::CreateProxyResource(instance, data); } diff --git a/ppapi/proxy/serialized_var_unittest.cc b/ppapi/proxy/serialized_var_unittest.cc index e67617b..34842c5 100644 --- a/ppapi/proxy/serialized_var_unittest.cc +++ b/ppapi/proxy/serialized_var_unittest.cc @@ -99,9 +99,9 @@ TEST_F(SerializedVarTest, PluginReceiveInput) { // release it, we should still be tracking the object since the // ReceiveInputs keep the "track_with_no_reference_count" alive until // they're destroyed. - var_tracker().AddRef(plugin_object); + var_tracker().AddRefVar(plugin_object); EXPECT_EQ(1, var_tracker().GetRefCountForObject(plugin_object)); - var_tracker().Release(plugin_object); + var_tracker().ReleaseVar(plugin_object); EXPECT_EQ(0, var_tracker().GetRefCountForObject(plugin_object)); EXPECT_EQ(2u, sink().message_count()); } @@ -143,13 +143,13 @@ TEST_F(SerializedVarTest, PluginReceiveReturn) { EXPECT_EQ(1u, sink().message_count()); // Manually release one refcount, it shouldn't have sent any more messages. - var_tracker().Release(plugin_object); + var_tracker().ReleaseVar(plugin_object); EXPECT_EQ(1, var_tracker().GetRefCountForObject(plugin_object)); EXPECT_EQ(1u, sink().message_count()); // Manually release the last refcount, it should have freed it and sent a // release message to the browser. - var_tracker().Release(plugin_object); + var_tracker().ReleaseVar(plugin_object); EXPECT_EQ(-1, var_tracker().GetRefCountForObject(plugin_object)); EXPECT_EQ(2u, sink().message_count()); } diff --git a/ppapi/shared_impl/id_assignment.cc b/ppapi/shared_impl/id_assignment.cc new file mode 100644 index 0000000..5cbb1b1 --- /dev/null +++ b/ppapi/shared_impl/id_assignment.cc @@ -0,0 +1,18 @@ +// 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 "ppapi/shared_impl/id_assignment.h" + +#include "base/basictypes.h" + +namespace ppapi { + +const unsigned int kPPIdTypeBits = 2; + +const int32 kMaxPPId = std::numeric_limits<int32>::max() >> kPPIdTypeBits; + +COMPILE_ASSERT(PP_ID_TYPE_COUNT <= (1<<kPPIdTypeBits), + kPPIdTypeBits_is_too_small_for_all_id_types); + +} // namespace ppapi diff --git a/ppapi/shared_impl/id_assignment.h b/ppapi/shared_impl/id_assignment.h new file mode 100644 index 0000000..1adc2bd --- /dev/null +++ b/ppapi/shared_impl/id_assignment.h @@ -0,0 +1,45 @@ +// 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. + +#ifndef PPAPI_SHARED_IMPL_ID_ASSIGNMENT_H_ +#define PPAPI_SHARED_IMPL_ID_ASSIGNMENT_H_ + +#include <limits> + +#include "base/basictypes.h" + +namespace ppapi { + +enum PPIdType { + PP_ID_TYPE_MODULE, + PP_ID_TYPE_INSTANCE, + PP_ID_TYPE_RESOURCE, + PP_ID_TYPE_VAR, + + // Not a real type, must be last. + PP_ID_TYPE_COUNT +}; + +extern const unsigned int kPPIdTypeBits; + +extern const int32 kMaxPPId; + +// The most significant bits are the type, the rest are the value. +template <typename T> inline T MakeTypedId(T value, PPIdType type) { + return (value << kPPIdTypeBits) | static_cast<T>(type); +} + +template <typename T> inline bool CheckIdType(T id, PPIdType type) { + // Say a resource of 0 is always valid, since that means "no resource." + // You shouldn't be passing 0 var, instance, or module IDs around so those + // are still invalid. + if (type == PP_ID_TYPE_RESOURCE && !id) + return true; + const T mask = (static_cast<T>(1) << kPPIdTypeBits) - 1; + return (id & mask) == type; +} + +} // namespace ppapi + +#endif // PPAPI_SHARED_IMPL_ID_ASSIGNMENT_H_ diff --git a/ppapi/shared_impl/tracker_base.h b/ppapi/shared_impl/tracker_base.h index 7790ea3..4b45a79 100644 --- a/ppapi/shared_impl/tracker_base.h +++ b/ppapi/shared_impl/tracker_base.h @@ -15,7 +15,7 @@ namespace ppapi { class FunctionGroupBase; class ResourceObjectBase; -class Var; +class VarTracker; // Tracks resource and function APIs, providing a mapping between ID and // object. @@ -50,16 +50,7 @@ class TrackerBase { // resource is invalid. virtual PP_Instance GetInstanceForResource(PP_Resource resource) = 0; - // PP_Vars ------------------------------------------------------------------- - - // Adds a new var to the tracker, returning the new Var ID. - virtual int32 AddVar(Var* var) = 0; - - // Retrieves a var from the tracker, returning an empty scoped ptr on failure. - virtual scoped_refptr<Var> GetVar(int32 var_id) const = 0; - - virtual bool AddRefVar(int32 var_id) = 0; - virtual bool UnrefVar(int32 var_id) = 0; + virtual VarTracker* GetVarTracker() = 0; }; } // namespace ppapi diff --git a/ppapi/shared_impl/url_util_impl.cc b/ppapi/shared_impl/url_util_impl.cc index f45d4fef..66a6185 100644 --- a/ppapi/shared_impl/url_util_impl.cc +++ b/ppapi/shared_impl/url_util_impl.cc @@ -5,6 +5,7 @@ #include "ppapi/shared_impl/url_util_impl.h" #include "googleurl/src/gurl.h" +#include "ppapi/shared_impl/var.h" namespace ppapi { @@ -40,48 +41,42 @@ void ConvertComponents(const url_parse::Parsed& input, } // namespace // static -PP_Var URLUtilImpl::Canonicalize(StringFromVar string_from_var, - VarFromUtf8 var_from_utf8, - PP_Module pp_module, +PP_Var URLUtilImpl::Canonicalize(PP_Module pp_module, PP_Var url, PP_URLComponents_Dev* components) { - const std::string* url_string = string_from_var(url); + scoped_refptr<StringVar> url_string(StringVar::FromPPVar(url)); if (!url_string) return PP_MakeNull(); - return GenerateURLReturn(var_from_utf8, pp_module, - GURL(*url_string), components); + return GenerateURLReturn(pp_module, GURL(url_string->value()), components); } // static -PP_Var URLUtilImpl::ResolveRelativeToURL(StringFromVar string_from_var, - VarFromUtf8 var_from_utf8, - PP_Module pp_module, +PP_Var URLUtilImpl::ResolveRelativeToURL(PP_Module pp_module, PP_Var base_url, PP_Var relative, PP_URLComponents_Dev* components) { - const std::string* base_url_string = string_from_var(base_url); - const std::string* relative_string = string_from_var(relative); + scoped_refptr<StringVar> base_url_string(StringVar::FromPPVar(base_url)); + scoped_refptr<StringVar> relative_string(StringVar::FromPPVar(relative)); if (!base_url_string || !relative_string) return PP_MakeNull(); - GURL base_gurl(*base_url_string); + GURL base_gurl(base_url_string->value()); if (!base_gurl.is_valid()) return PP_MakeNull(); - return GenerateURLReturn(var_from_utf8, pp_module, - base_gurl.Resolve(*relative_string), + return GenerateURLReturn(pp_module, + base_gurl.Resolve(relative_string->value()), components); } // static -PP_Bool URLUtilImpl::IsSameSecurityOrigin(StringFromVar string_from_var, - PP_Var url_a, PP_Var url_b) { - const std::string* url_a_string = string_from_var(url_a); - const std::string* url_b_string = string_from_var(url_b); +PP_Bool URLUtilImpl::IsSameSecurityOrigin(PP_Var url_a, PP_Var url_b) { + scoped_refptr<StringVar> url_a_string(StringVar::FromPPVar(url_a)); + scoped_refptr<StringVar> url_b_string(StringVar::FromPPVar(url_b)); if (!url_a_string || !url_b_string) return PP_FALSE; - GURL gurl_a(*url_a_string); - GURL gurl_b(*url_b_string); + GURL gurl_a(url_a_string->value()); + GURL gurl_b(url_b_string->value()); if (!gurl_a.is_valid() || !gurl_b.is_valid()) return PP_FALSE; @@ -90,15 +85,13 @@ PP_Bool URLUtilImpl::IsSameSecurityOrigin(StringFromVar string_from_var, // Used for returning the given GURL from a PPAPI function, with an optional // out param indicating the components. -PP_Var URLUtilImpl::GenerateURLReturn(VarFromUtf8 var_from_utf8, - PP_Module module, +PP_Var URLUtilImpl::GenerateURLReturn(PP_Module module, const GURL& url, PP_URLComponents_Dev* components) { if (!url.is_valid()) return PP_MakeNull(); ConvertComponents(url.parsed_for_possibly_invalid_spec(), components); - return var_from_utf8(module, url.possibly_invalid_spec().c_str(), - static_cast<uint32_t>(url.possibly_invalid_spec().size())); + return StringVar::StringToPPVar(module, url.possibly_invalid_spec()); } } // namespace ppapi diff --git a/ppapi/shared_impl/url_util_impl.h b/ppapi/shared_impl/url_util_impl.h index 008f317..55d6c63 100644 --- a/ppapi/shared_impl/url_util_impl.h +++ b/ppapi/shared_impl/url_util_impl.h @@ -21,41 +21,19 @@ namespace ppapi { // and the renderer. class URLUtilImpl { public: - // The functions here would normally take the var interface for constructing - // return strings. However, at the current time there's some mixup between - // using Var and VarDeprecated. To resolve this, we instead pass the pointer - // to the string creation function so can be used independently of this. - typedef PP_Var (*VarFromUtf8)(PP_Module, const char*, uint32_t); - - // Function that converts the given var to a std::string or NULL if the - // var is not a string or is invalid. - // - // We could use PPB_Var for this, but that interface requires an additional - // string conversion. Both the proxy and the host side maintain the strings - // in a std::string, and the form we want for passing to GURL is also a - // std::string. Parameterizing this separately saves this, and also solves - // the same problem that VarFromUtf8 does. - typedef const std::string* (*StringFromVar)(PP_Var var); - // PPB_URLUtil shared functions. - static PP_Var Canonicalize(StringFromVar string_from_var, - VarFromUtf8 var_from_utf8, - PP_Module pp_module, + static PP_Var Canonicalize(PP_Module pp_module, PP_Var url, PP_URLComponents_Dev* components); - static PP_Var ResolveRelativeToURL(StringFromVar string_from_var, - VarFromUtf8 var_from_utf8, - PP_Module pp_module, + static PP_Var ResolveRelativeToURL(PP_Module pp_module, PP_Var base_url, PP_Var relative, PP_URLComponents_Dev* components); - static PP_Bool IsSameSecurityOrigin(StringFromVar string_from_var, - PP_Var url_a, PP_Var url_b); + static PP_Bool IsSameSecurityOrigin(PP_Var url_a, PP_Var url_b); // Used for returning the given GURL from a PPAPI function, with an optional // out param indicating the components. - static PP_Var GenerateURLReturn(VarFromUtf8 var_from_utf8, - PP_Module pp_module, + static PP_Var GenerateURLReturn(PP_Module pp_module, const GURL& url, PP_URLComponents_Dev* components); }; diff --git a/ppapi/shared_impl/var.cc b/ppapi/shared_impl/var.cc index 4148afe..af63b0b 100644 --- a/ppapi/shared_impl/var.cc +++ b/ppapi/shared_impl/var.cc @@ -11,6 +11,7 @@ #include "base/string_util.h" #include "ppapi/c/pp_var.h" #include "ppapi/shared_impl/tracker_base.h" +#include "ppapi/shared_impl/var_tracker.h" namespace ppapi { @@ -60,22 +61,6 @@ std::string Var::PPVarToLogString(PP_Var var) { } } -// static -void Var::PluginAddRefPPVar(PP_Var var) { - if (var.type == PP_VARTYPE_STRING || var.type == PP_VARTYPE_OBJECT) { - if (!TrackerBase::Get()->AddRefVar(static_cast<int32>(var.value.as_id))) - DLOG(WARNING) << "AddRefVar()ing a nonexistent string/object var."; - } -} - -// static -void Var::PluginReleasePPVar(PP_Var var) { - if (var.type == PP_VARTYPE_STRING || var.type == PP_VARTYPE_OBJECT) { - if (!TrackerBase::Get()->UnrefVar(static_cast<int32>(var.value.as_id))) - DLOG(WARNING) << "ReleaseVar()ing a nonexistent string/object var."; - } -} - StringVar* Var::AsStringVar() { return NULL; } @@ -84,12 +69,16 @@ NPObjectVar* Var::AsNPObjectVar() { return NULL; } +ProxyObjectVar* Var::AsProxyObjectVar() { + return NULL; +} + int32 Var::GetExistingVarID() const { return var_id_; } int32 Var::GetOrCreateVarID() { - TrackerBase* tracker = TrackerBase::Get(); + VarTracker* tracker = TrackerBase::Get()->GetVarTracker(); if (var_id_) { if (!tracker->AddRefVar(var_id_)) return 0; @@ -101,6 +90,11 @@ int32 Var::GetOrCreateVarID() { return var_id_; } +void Var::AssignVarID(int32 id) { + DCHECK(!var_id_); // Must not have already been generated. + var_id_ = id; +} + // StringVar ------------------------------------------------------------------- StringVar::StringVar(PP_Module module, const char* str, uint32 len) @@ -126,6 +120,10 @@ PP_Var StringVar::GetPPVar() { return result; } +PP_VarType StringVar::GetType() const { + return PP_VARTYPE_STRING; +} + // static PP_Var StringVar::StringToPPVar(PP_Module module, const std::string& var) { return StringToPPVar(module, var.c_str(), var.size()); @@ -145,7 +143,7 @@ scoped_refptr<StringVar> StringVar::FromPPVar(PP_Var var) { if (var.type != PP_VARTYPE_STRING) return scoped_refptr<StringVar>(); scoped_refptr<Var> var_object( - TrackerBase::Get()->GetVar(static_cast<int32>(var.value.as_id))); + TrackerBase::Get()->GetVarTracker()->GetVar(var)); if (!var_object) return scoped_refptr<StringVar>(); return scoped_refptr<StringVar>(var_object->AsStringVar()); diff --git a/ppapi/shared_impl/var.h b/ppapi/shared_impl/var.h index e03e286..1ff42dd 100644 --- a/ppapi/shared_impl/var.h +++ b/ppapi/shared_impl/var.h @@ -10,12 +10,12 @@ #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "ppapi/c/pp_module.h" - -struct PP_Var; +#include "ppapi/c/pp_var.h" namespace ppapi { class NPObjectVar; +class ProxyObjectVar; class StringVar; // Var ------------------------------------------------------------------------- @@ -30,43 +30,17 @@ class Var : public base::RefCounted<Var> { // Returns a string representing the given var for logging purposes. static std::string PPVarToLogString(PP_Var var); - // Provides access to the manual refcounting of a PP_Var from the plugin's - // perspective. This is different than the AddRef/Release on this scoped - // object. This uses the ResourceTracker, which keeps a separate "plugin - // refcount" that prevents the plugin from messing up our refcounting or - // freeing something out from under us. - // - // You should not generally need to use these functions. However, if you - // call a plugin function that returns a var, it will transfer a ref to us - // (the caller) which in the case of a string or object var will need to - // be released. - // - // Example, assuming we're expecting the plugin to return a string: - // PP_Var rv = some_ppp_interface->DoSomething(a, b, c); - // - // // Get the string value. This will take a reference to the object which - // // will prevent it from being deleted out from under us when we call - // // PluginReleasePPVar(). - // scoped_refptr<StringVar> string(StringVar::FromPPVar(rv)); - // - // // Release the reference the plugin gave us when returning the value. - // // This is legal to do for all types of vars. - // Var::PluginReleasePPVar(rv); - // - // // Use the string. - // if (!string) - // return false; // It didn't return a proper string. - // UseTheString(string->value()); - static void PluginAddRefPPVar(PP_Var var); - static void PluginReleasePPVar(PP_Var var); - virtual StringVar* AsStringVar(); virtual NPObjectVar* AsNPObjectVar(); + virtual ProxyObjectVar* AsProxyObjectVar(); // Creates a PP_Var corresponding to this object. The return value will have // one reference addrefed on behalf of the caller. virtual PP_Var GetPPVar() = 0; + // Returns the type of this var. + virtual PP_VarType GetType() const = 0; + // Returns the ID corresponing to the string or object if it exists already, // or 0 if an ID hasn't been generated for this object (the plugin is holding // no refs). @@ -89,6 +63,10 @@ class Var : public base::RefCounted<Var> { // caller. int32 GetOrCreateVarID(); + // Sets the internal object ID. This assumes that the ID hasn't been set + // before. This is used in cases where the ID is generated externally. + void AssignVarID(int32 id); + private: PP_Module pp_module_; @@ -120,6 +98,7 @@ class StringVar : public Var { // Var override. virtual StringVar* AsStringVar() OVERRIDE; virtual PP_Var GetPPVar() OVERRIDE; + virtual PP_VarType GetType() const OVERRIDE; // Helper function to create a PP_Var of type string that contains a copy of // the given string. The input data must be valid UTF-8 encoded text, if it diff --git a/ppapi/shared_impl/var_tracker.cc b/ppapi/shared_impl/var_tracker.cc new file mode 100644 index 0000000..7f7157e --- /dev/null +++ b/ppapi/shared_impl/var_tracker.cc @@ -0,0 +1,161 @@ +// 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 "ppapi/shared_impl/var_tracker.h" + +#include <limits> + +#include "base/logging.h" +#include "ppapi/shared_impl/id_assignment.h" +#include "ppapi/shared_impl/var.h" + +namespace ppapi { + +VarTracker::VarInfo::VarInfo() + : var(), + ref_count(0), + track_with_no_reference_count(0) { +} + +VarTracker::VarInfo::VarInfo(Var* v, int input_ref_count) + : var(v), + ref_count(input_ref_count), + track_with_no_reference_count(0) { +} + +VarTracker::VarTracker() : last_var_id_(0) { +} + +VarTracker::~VarTracker() { +} + +int32 VarTracker::AddVar(Var* var) { + return AddVarInternal(var, ADD_VAR_TAKE_ONE_REFERENCE); +} + +Var* VarTracker::GetVar(int32 var_id) const { + VarMap::const_iterator result = live_vars_.find(var_id); + if (result == live_vars_.end()) + return NULL; + return result->second.var.get(); +} + +Var* VarTracker::GetVar(const PP_Var& var) const { + if (!IsVarTypeRefcounted(var.type)) + return NULL; + return GetVar(static_cast<int32>(var.value.as_id)); +} + +bool VarTracker::AddRefVar(int32 var_id) { + DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR)) + << var_id << " is not a PP_Var ID."; + VarMap::iterator found = live_vars_.find(var_id); + if (found == live_vars_.end()) { + NOTREACHED(); // Invalid var. + return false; + } + + VarInfo& info = found->second; + if (info.ref_count == 0) { + // All live vars with no refcount should be tracked objects. + DCHECK(info.track_with_no_reference_count > 0); + DCHECK(info.var->GetType() == PP_VARTYPE_OBJECT); + + TrackedObjectGettingOneRef(found); + } + + // Basic refcount increment. + info.ref_count++; + return true; +} + +bool VarTracker::AddRefVar(const PP_Var& var) { + if (!IsVarTypeRefcounted(var.type)) + return false; + return AddRefVar(static_cast<int32>(var.value.as_id)); +} + +bool VarTracker::ReleaseVar(int32 var_id) { + DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR)) + << var_id << " is not a PP_Var ID."; + VarMap::iterator found = live_vars_.find(var_id); + if (found == live_vars_.end()) { + NOTREACHED() << "Unref-ing an invalid var"; + return false; + } + + VarInfo& info = found->second; + if (info.ref_count == 0) { + NOTREACHED() << "Releasing an object with zero ref"; + return false; + } + info.ref_count--; + + if (info.ref_count == 0) { + if (info.var->GetType() == PP_VARTYPE_OBJECT) { + // Objects have special requirements and may not necessarily be released + // when the refcount goes to 0. + ObjectGettingZeroRef(found); + } else { + // All other var types can just be released. + DCHECK(info.track_with_no_reference_count == 0); + live_vars_.erase(found); + } + } + return true; +} + +bool VarTracker::ReleaseVar(const PP_Var& var) { + if (!IsVarTypeRefcounted(var.type)) + return false; + return ReleaseVar(static_cast<int32>(var.value.as_id)); +} + +int32 VarTracker::AddVarInternal(Var* var, AddVarRefMode mode) { + // If the plugin manages to create millions of strings. + if (last_var_id_ == std::numeric_limits<int32>::max() >> kPPIdTypeBits) + return 0; + + int32 new_id = MakeTypedId(++last_var_id_, PP_ID_TYPE_VAR); + live_vars_.insert(std::make_pair(new_id, + VarInfo(var, mode == ADD_VAR_TAKE_ONE_REFERENCE ? 1 : 0))); + + return new_id; +} + +VarTracker::VarMap::iterator VarTracker::GetLiveVar(int32 id) { + return live_vars_.find(id); +} + +VarTracker::VarMap::iterator VarTracker::GetLiveVar(const PP_Var& var) { + return live_vars_.find(static_cast<int32>(var.value.as_id)); +} + +VarTracker::VarMap::const_iterator VarTracker::GetLiveVar( + const PP_Var& var) const { + return live_vars_.find(static_cast<int32>(var.value.as_id)); +} + +bool VarTracker::IsVarTypeRefcounted(PP_VarType type) const { + return type == PP_VARTYPE_STRING || type == PP_VARTYPE_OBJECT; +} + +void VarTracker::TrackedObjectGettingOneRef(VarMap::const_iterator obj) { + // Anybody using tracked objects should override this. + NOTREACHED(); +} + +void VarTracker::ObjectGettingZeroRef(VarMap::iterator iter) { + DeleteObjectInfoIfNecessary(iter); +} + +bool VarTracker::DeleteObjectInfoIfNecessary(VarMap::iterator iter) { + if (iter->second.ref_count != 0 || + iter->second.track_with_no_reference_count != 0) + return false; // Object still alive. + live_vars_.erase(iter); + return true; +} + +} // namespace ppapi diff --git a/ppapi/shared_impl/var_tracker.h b/ppapi/shared_impl/var_tracker.h new file mode 100644 index 0000000..f05c5ac --- /dev/null +++ b/ppapi/shared_impl/var_tracker.h @@ -0,0 +1,133 @@ +// 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. + +#ifndef PPAPI_SHARED_IMPL_VAR_TRACKER_H_ +#define PPAPI_SHARED_IMPL_VAR_TRACKER_H_ + +#include "base/basictypes.h" +#include "base/hash_tables.h" +#include "base/memory/ref_counted.h" +#include "ppapi/c/pp_var.h" + +namespace ppapi { + +class Var; + +// Tracks non-POD (refcounted) var objects held by a plugin. +// +// The tricky part is the concept of a "tracked object". These are only +// necessary in the plugin side of the proxy when running out of process. A +// tracked object is one that the plugin is aware of, but doesn't hold a +// reference to. This will happen when the plugin is passed an object as an +// argument from the host (renderer) as an input argument to a sync function, +// but where ownership is not passed. +// +// This class maintains the "track_with_no_reference_count" but doesn't do +// anything with it other than call virtual functions. The interesting parts +// are added by the PluginObjectVar derived from this class. +class VarTracker { + public: + VarTracker(); + virtual ~VarTracker(); + + // Called by the Var object to add a new var to the tracker. + int32 AddVar(Var* var); + + // Looks up a given var and returns a reference to the Var if it exists. + // Returns NULL if the var type is not an object we track (POD) or is + // invalid. + Var* GetVar(int32 var_id) const; + Var* GetVar(const PP_Var& var) const; + + // Increases a previously-known Var ID's refcount, returning true on success, + // false if the ID is invalid. The PP_Var version returns true and does + // nothing for non-refcounted type vars. + bool AddRefVar(int32 var_id); + bool AddRefVar(const PP_Var& var); + + // Decreases the given Var ID's refcount, returning true on success, false if + // the ID is invalid or if the refcount was already 0. The PP_Var version + // returns true and does nothing for non-refcounted type vars. The var will + // be deleted if there are no more refs to it. + bool ReleaseVar(int32 var_id); + bool ReleaseVar(const PP_Var& var); + + protected: + struct VarInfo { + VarInfo(); + VarInfo(Var* v, int input_ref_count); + + scoped_refptr<Var> var; + + // Explicit reference count. This value is affected by the renderer calling + // AddRef and Release. A nonzero value here is represented by a single + // reference in the host on our behalf (this reduces IPC traffic). + int ref_count; + + // Tracked object count (see class comment above). + // + // "TrackObjectWithNoReference" might be called recursively in rare cases. + // For example, say the host calls a plugin function with an object as an + // argument, and in response, the plugin calls a host function that then + // calls another (or the same) plugin function with the same object. + // + // This value tracks the number of calls to TrackObjectWithNoReference so + // we know when we can stop tracking this object. + int track_with_no_reference_count; + }; + typedef base::hash_map<int32, VarInfo> VarMap; + + // Specifies what should happen with the refcount when calling AddVarInternal. + enum AddVarRefMode { + ADD_VAR_TAKE_ONE_REFERENCE, + ADD_VAR_CREATE_WITH_NO_REFERENCE + }; + + // Implementation of AddVar that allows the caller to specify whether the + // initial refcount of the added object will be 0 or 1. + // + // Overridden in the plugin proxy to do additional object tracking. + virtual int32 AddVarInternal(Var* var, AddVarRefMode mode); + + // Convenience functions for doing lookups into the live_vars_ map. + VarMap::iterator GetLiveVar(int32 id); + VarMap::iterator GetLiveVar(const PP_Var& var); + VarMap::const_iterator GetLiveVar(const PP_Var& var) const; + + // Returns true if the given vartype is refcounted and has associated objects + // (it's not POD). + bool IsVarTypeRefcounted(PP_VarType type) const; + + // Called when AddRefVar increases a "tracked" ProxyObject's refcount from + // zero to one. In the plugin side of the proxy, we need to send some + // messages to the host. In the host side, this should never be called since + // there are no proxy objects. + virtual void TrackedObjectGettingOneRef(VarMap::const_iterator iter); + + // Called when ReleaseVar decreases a object's refcount from one to zero. It + // may still be "tracked" (has a "track_with_no_reference_count") value. In + // the plugin side of the proxy, we need to tell the host that we no longer + // have a reference. In the host side, this should never be called since + // there are no proxy objects. + virtual void ObjectGettingZeroRef(VarMap::iterator iter); + + // Called when an object may have had its refcount or + // track_with_no_reference_count value decreased. If the object has neither + // refs anymore, this will remove it and return true. Returns false if it's + // still alive. + // + // Overridden by the PluginVarTracker to also clean up the host info map. + virtual bool DeleteObjectInfoIfNecessary(VarMap::iterator iter); + + VarMap live_vars_; + + // Last assigned var ID. + int32 last_var_id_; + + DISALLOW_COPY_AND_ASSIGN(VarTracker); +}; + +} // namespace ppapi + +#endif // PPAPI_SHARED_IMPL_VAR_TRACKER_H_ diff --git a/ppapi/tests/test_var_deprecated.cc b/ppapi/tests/test_var_deprecated.cc index 07ccf91..253058d 100644 --- a/ppapi/tests/test_var_deprecated.cc +++ b/ppapi/tests/test_var_deprecated.cc @@ -294,78 +294,70 @@ std::string TestVarDeprecated::TestVarToUtf8ForWrongType() { } std::string TestVarDeprecated::TestHasPropertyAndMethod() { - uint32_t before_objects = testing_interface_->GetLiveObjectsForInstance( - instance_->pp_instance()); - { - pp::VarPrivate window = instance_->GetWindowObject(); - ASSERT_TRUE(window.is_object()); - - // Regular property. - pp::Var exception; - ASSERT_TRUE(window.HasProperty("scrollX", &exception)); - ASSERT_TRUE(exception.is_undefined()); - ASSERT_FALSE(window.HasMethod("scrollX", &exception)); - ASSERT_TRUE(exception.is_undefined()); - - // Regular method (also counts as HasProperty). - ASSERT_TRUE(window.HasProperty("find", &exception)); - ASSERT_TRUE(exception.is_undefined()); - ASSERT_TRUE(window.HasMethod("find", &exception)); - ASSERT_TRUE(exception.is_undefined()); - - // Nonexistant ones should return false and not set the exception. - ASSERT_FALSE(window.HasProperty("superEvilBit", &exception)); - ASSERT_TRUE(exception.is_undefined()); - ASSERT_FALSE(window.HasMethod("superEvilBit", &exception)); - ASSERT_TRUE(exception.is_undefined()); - - // Check exception and return false on invalid property name. - ASSERT_FALSE(window.HasProperty(3.14159, &exception)); - ASSERT_FALSE(exception.is_undefined()); - exception = pp::Var(); - - exception = pp::Var(); - ASSERT_FALSE(window.HasMethod(3.14159, &exception)); - ASSERT_FALSE(exception.is_undefined()); - - // Try to use something not an object. - exception = pp::Var(); - pp::VarPrivate string_object("asdf"); - ASSERT_FALSE(string_object.HasProperty("find", &exception)); - ASSERT_FALSE(exception.is_undefined()); - exception = pp::Var(); - ASSERT_FALSE(string_object.HasMethod("find", &exception)); - ASSERT_FALSE(exception.is_undefined()); - - // Try to use an invalid object (need to use the C API). - PP_Var invalid_object; - invalid_object.type = PP_VARTYPE_OBJECT; - invalid_object.value.as_id = static_cast<int64_t>(-1234567); - PP_Var exception2 = PP_MakeUndefined(); - ASSERT_FALSE(var_interface_->HasProperty(invalid_object, - pp::Var("find").pp_var(), - &exception2)); - ASSERT_NE(PP_VARTYPE_UNDEFINED, exception2.type); - var_interface_->Release(exception2); - - exception2 = PP_MakeUndefined(); - ASSERT_FALSE(var_interface_->HasMethod(invalid_object, + pp::VarPrivate window = instance_->GetWindowObject(); + ASSERT_TRUE(window.is_object()); + + // Regular property. + pp::Var exception; + ASSERT_TRUE(window.HasProperty("scrollX", &exception)); + ASSERT_TRUE(exception.is_undefined()); + ASSERT_FALSE(window.HasMethod("scrollX", &exception)); + ASSERT_TRUE(exception.is_undefined()); + + // Regular method (also counts as HasProperty). + ASSERT_TRUE(window.HasProperty("find", &exception)); + ASSERT_TRUE(exception.is_undefined()); + ASSERT_TRUE(window.HasMethod("find", &exception)); + ASSERT_TRUE(exception.is_undefined()); + + // Nonexistant ones should return false and not set the exception. + ASSERT_FALSE(window.HasProperty("superEvilBit", &exception)); + ASSERT_TRUE(exception.is_undefined()); + ASSERT_FALSE(window.HasMethod("superEvilBit", &exception)); + ASSERT_TRUE(exception.is_undefined()); + + // Check exception and return false on invalid property name. + ASSERT_FALSE(window.HasProperty(3.14159, &exception)); + ASSERT_FALSE(exception.is_undefined()); + exception = pp::Var(); + + exception = pp::Var(); + ASSERT_FALSE(window.HasMethod(3.14159, &exception)); + ASSERT_FALSE(exception.is_undefined()); + + // Try to use something not an object. + exception = pp::Var(); + pp::VarPrivate string_object("asdf"); + ASSERT_FALSE(string_object.HasProperty("find", &exception)); + ASSERT_FALSE(exception.is_undefined()); + exception = pp::Var(); + ASSERT_FALSE(string_object.HasMethod("find", &exception)); + ASSERT_FALSE(exception.is_undefined()); + + // Try to use an invalid object (need to use the C API). + PP_Var invalid_object; + invalid_object.type = PP_VARTYPE_OBJECT; + invalid_object.value.as_id = static_cast<int64_t>(-1234567); + PP_Var exception2 = PP_MakeUndefined(); + ASSERT_FALSE(var_interface_->HasProperty(invalid_object, pp::Var("find").pp_var(), &exception2)); - ASSERT_NE(PP_VARTYPE_UNDEFINED, exception2.type); - var_interface_->Release(exception2); - - // Get a valid property/method when the exception is set returns false. - exception = pp::Var("Bad something-or-other exception"); - ASSERT_FALSE(window.HasProperty("find", &exception)); - ASSERT_FALSE(exception.is_undefined()); - ASSERT_FALSE(window.HasMethod("find", &exception)); - ASSERT_FALSE(exception.is_undefined()); - } - - // Make sure nothing leaked. - ASSERT_TRUE(testing_interface_->GetLiveObjectsForInstance( - instance_->pp_instance()) == before_objects); + ASSERT_NE(PP_VARTYPE_UNDEFINED, exception2.type); + var_interface_->Release(exception2); + + exception2 = PP_MakeUndefined(); + ASSERT_FALSE(var_interface_->HasMethod(invalid_object, + pp::Var("find").pp_var(), + &exception2)); + ASSERT_NE(PP_VARTYPE_UNDEFINED, exception2.type); + var_interface_->Release(exception2); + + // Getting a valid property/method when the exception is set returns false. + exception = pp::Var("Bad something-or-other exception"); + ASSERT_FALSE(window.HasProperty("find", &exception)); + ASSERT_FALSE(exception.is_undefined()); + ASSERT_FALSE(window.HasMethod("find", &exception)); + ASSERT_FALSE(exception.is_undefined()); PASS(); } diff --git a/webkit/plugins/ppapi/npapi_glue.cc b/webkit/plugins/ppapi/npapi_glue.cc index 93c57ed..7e90a1e 100644 --- a/webkit/plugins/ppapi/npapi_glue.cc +++ b/webkit/plugins/ppapi/npapi_glue.cc @@ -159,7 +159,7 @@ PPResultAndExceptionToNPResult::~PPResultAndExceptionToNPResult() { // been lost. DCHECK(checked_exception_); - NPObjectVar::PluginReleasePPVar(exception_); + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(exception_); } // Call this with the return value of the PPAPI function. It will convert @@ -184,7 +184,7 @@ bool PPResultAndExceptionToNPResult::SetResult(PP_Var result) { // No matter what happened, we need to release the reference to the // value passed in. On success, a reference to this value will be in // the np_result_. - ::ppapi::Var::PluginReleasePPVar(result); + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(result); return success_; } @@ -237,8 +237,9 @@ PPVarArrayFromNPVariantArray::PPVarArrayFromNPVariantArray( } PPVarArrayFromNPVariantArray::~PPVarArrayFromNPVariantArray() { + ::ppapi::VarTracker* var_tracker = ResourceTracker::Get()->GetVarTracker(); for (size_t i = 0; i < size_; i++) - ::ppapi::Var::PluginReleasePPVar(array_[i]); + var_tracker->ReleaseVar(array_[i]); } // PPVarFromNPObject ----------------------------------------------------------- @@ -248,7 +249,7 @@ PPVarFromNPObject::PPVarFromNPObject(PluginInstance* instance, NPObject* object) } PPVarFromNPObject::~PPVarFromNPObject() { - ::ppapi::Var::PluginReleasePPVar(var_); + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(var_); } // NPObjectAccessorWithIdentifier ---------------------------------------------- @@ -268,7 +269,7 @@ NPObjectAccessorWithIdentifier::NPObjectAccessorWithIdentifier( } NPObjectAccessorWithIdentifier::~NPObjectAccessorWithIdentifier() { - ::ppapi::Var::PluginReleasePPVar(identifier_); + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(identifier_); } // TryCatch -------------------------------------------------------------------- diff --git a/webkit/plugins/ppapi/npobject_var.cc b/webkit/plugins/ppapi/npobject_var.cc index b951cde..e430dd2 100644 --- a/webkit/plugins/ppapi/npobject_var.cc +++ b/webkit/plugins/ppapi/npobject_var.cc @@ -46,6 +46,10 @@ PP_Var NPObjectVar::GetPPVar() { return result; } +PP_VarType NPObjectVar::GetType() const { + return PP_VARTYPE_OBJECT; +} + void NPObjectVar::InstanceDeleted() { DCHECK(pp_instance_); pp_instance_ = 0; @@ -55,8 +59,8 @@ void NPObjectVar::InstanceDeleted() { scoped_refptr<NPObjectVar> NPObjectVar::FromPPVar(PP_Var var) { if (var.type != PP_VARTYPE_OBJECT) return scoped_refptr<NPObjectVar>(NULL); - scoped_refptr<Var> var_object(webkit::ppapi::ResourceTracker::Get()->GetVar( - static_cast<int32>(var.value.as_id))); + scoped_refptr<Var> var_object( + webkit::ppapi::ResourceTracker::Get()->GetVarTracker()->GetVar(var)); if (!var_object) return scoped_refptr<NPObjectVar>(); return scoped_refptr<NPObjectVar>(var_object->AsNPObjectVar()); diff --git a/webkit/plugins/ppapi/npobject_var.h b/webkit/plugins/ppapi/npobject_var.h index d0ba122..f82861a 100644 --- a/webkit/plugins/ppapi/npobject_var.h +++ b/webkit/plugins/ppapi/npobject_var.h @@ -39,6 +39,7 @@ class NPObjectVar : public Var { // Var overrides. virtual NPObjectVar* AsNPObjectVar() OVERRIDE; virtual PP_Var GetPPVar() OVERRIDE; + virtual PP_VarType GetType() const OVERRIDE; // Returns the underlying NPObject corresponding to this NPObjectVar. // Guaranteed non-NULL. diff --git a/webkit/plugins/ppapi/plugin_object.cc b/webkit/plugins/ppapi/plugin_object.cc index 95d8fba..5362fa4 100644 --- a/webkit/plugins/ppapi/plugin_object.cc +++ b/webkit/plugins/ppapi/plugin_object.cc @@ -21,6 +21,7 @@ #include "webkit/plugins/ppapi/plugin_module.h" #include "webkit/plugins/ppapi/ppapi_plugin_instance.h" #include "webkit/plugins/ppapi/resource.h" +#include "webkit/plugins/ppapi/resource_tracker.h" #include "webkit/plugins/ppapi/string.h" using ppapi::StringVar; @@ -136,7 +137,7 @@ bool WrapperClass_SetProperty(NPObject* object, NPIdentifier property_name, accessor.object()->ppp_class()->SetProperty( accessor.object()->ppp_class_data(), accessor.identifier(), value_var, result_converter.exception()); - Var::PluginReleasePPVar(value_var); + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(value_var); return result_converter.CheckExceptionForNoResult(); } @@ -201,8 +202,9 @@ bool WrapperClass_Enumerate(NPObject* object, NPIdentifier** values, // Release the PP_Var that the plugin allocated. On success, they will all // be converted to NPVariants, and on failure, we want them to just go away. + ::ppapi::VarTracker* var_tracker = ResourceTracker::Get()->GetVarTracker(); for (uint32_t i = 0; i < property_count; ++i) - Var::PluginReleasePPVar(properties[i]); + var_tracker->ReleaseVar(properties[i]); free(properties); return result_converter.success(); } diff --git a/webkit/plugins/ppapi/ppapi_plugin_instance.cc b/webkit/plugins/ppapi/ppapi_plugin_instance.cc index 5770ca8..242c246 100644 --- a/webkit/plugins/ppapi/ppapi_plugin_instance.cc +++ b/webkit/plugins/ppapi/ppapi_plugin_instance.cc @@ -609,7 +609,8 @@ string16 PluginInstance::GetSelectedText(bool html) { PP_Var rv = plugin_selection_interface_->GetSelectedText(pp_instance(), PP_FromBool(html)); scoped_refptr<StringVar> string(StringVar::FromPPVar(rv)); - Var::PluginReleasePPVar(rv); // Release the ref the plugin transfered to us. + // Release the ref the plugin transfered to us. + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(rv); if (!string) return string16(); return UTF8ToUTF16(string->value()); @@ -626,7 +627,8 @@ string16 PluginInstance::GetLinkAtPosition(const gfx::Point& point) { p.y = point.y(); PP_Var rv = plugin_pdf_interface_->GetLinkAtPosition(pp_instance(), p); scoped_refptr<StringVar> string(StringVar::FromPPVar(rv)); - Var::PluginReleasePPVar(rv); // Release the ref the plugin transfered to us. + // Release the ref the plugin transfered to us. + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(rv); if (!string) return string16(); return UTF8ToUTF16(string->value()); diff --git a/webkit/plugins/ppapi/ppb_url_util_impl.cc b/webkit/plugins/ppapi/ppb_url_util_impl.cc index ebd8921..1900731 100644 --- a/webkit/plugins/ppapi/ppb_url_util_impl.cc +++ b/webkit/plugins/ppapi/ppb_url_util_impl.cc @@ -39,13 +39,6 @@ PP_Module GetModuleFromVar(PP_Var string_var) { return str->pp_module(); } -const std::string* StringFromVar(PP_Var var) { - scoped_refptr<StringVar> string(StringVar::FromPPVar(var)); - if (!string) - return NULL; - return &string->value(); -} - // Sets |*security_origin| to be the WebKit security origin associated with the // document containing the given plugin instance. On success, returns true. If // the instance is invalid, returns false and |*security_origin| will be @@ -62,20 +55,14 @@ bool SecurityOriginForInstance(PP_Instance instance_id, } PP_Var Canonicalize(PP_Var url, PP_URLComponents_Dev* components) { - return URLUtilImpl::Canonicalize(&StringFromVar, - PPB_Var_Impl::GetVarInterface()->VarFromUtf8, - GetModuleFromVar(url), - url, components); + return URLUtilImpl::Canonicalize(GetModuleFromVar(url), url, components); } PP_Var ResolveRelativeToURL(PP_Var base_url, PP_Var relative, PP_URLComponents_Dev* components) { - return URLUtilImpl::ResolveRelativeToURL( - &StringFromVar, - PPB_Var_Impl::GetVarInterface()->VarFromUtf8, - GetModuleFromVar(base_url), - base_url, relative, components); + return URLUtilImpl::ResolveRelativeToURL(GetModuleFromVar(base_url), + base_url, relative, components); } PP_Var ResolveRelativeToDocument(PP_Instance instance_id, @@ -92,14 +79,13 @@ PP_Var ResolveRelativeToDocument(PP_Instance instance_id, WebKit::WebElement plugin_element = instance->container()->element(); GURL document_url = plugin_element.document().baseURL(); return URLUtilImpl::GenerateURLReturn( - PPB_Var_Impl::GetVarInterface()->VarFromUtf8, instance->module()->pp_module(), document_url.Resolve(relative_string->value()), components); } PP_Bool IsSameSecurityOrigin(PP_Var url_a, PP_Var url_b) { - return URLUtilImpl::IsSameSecurityOrigin(&StringFromVar, url_a, url_b); + return URLUtilImpl::IsSameSecurityOrigin(url_a, url_b); } PP_Bool DocumentCanRequest(PP_Instance instance, PP_Var url) { @@ -137,10 +123,8 @@ PP_Var GetDocumentURL(PP_Instance instance_id, return PP_MakeNull(); WebKit::WebDocument document = instance->container()->element().document(); - return URLUtilImpl::GenerateURLReturn( - PPB_Var_Impl::GetVarInterface()->VarFromUtf8, - instance->module()->pp_module(), - document.url(), components); + return URLUtilImpl::GenerateURLReturn(instance->module()->pp_module(), + document.url(), components); } PP_Var GetPluginInstanceURL(PP_Instance instance_id, @@ -150,10 +134,8 @@ PP_Var GetPluginInstanceURL(PP_Instance instance_id, return PP_MakeNull(); const GURL& url = instance->plugin_url(); - return URLUtilImpl::GenerateURLReturn( - PPB_Var_Impl::GetVarInterface()->VarFromUtf8, - instance->module()->pp_module(), - url, components); + return URLUtilImpl::GenerateURLReturn(instance->module()->pp_module(), + url, components); } const PPB_URLUtil_Dev ppb_url_util = { diff --git a/webkit/plugins/ppapi/ppb_var_impl.cc b/webkit/plugins/ppapi/ppb_var_impl.cc index 49576b3..160b972 100644 --- a/webkit/plugins/ppapi/ppb_var_impl.cc +++ b/webkit/plugins/ppapi/ppb_var_impl.cc @@ -169,6 +169,14 @@ class ObjectAccessorWithIdentifierTryCatch : public ObjectAccessorTryCatch { // PPB_Var methods ------------------------------------------------------------- +void AddRefVar(PP_Var var) { + ResourceTracker::Get()->GetVarTracker()->AddRefVar(var); +} + +void ReleaseVar(PP_Var var) { + ResourceTracker::Get()->GetVarTracker()->ReleaseVar(var); +} + PP_Var VarFromUtf8(PP_Module module, const char* data, uint32_t len) { return StringVar::StringToPPVar(module, data, len); } @@ -418,8 +426,8 @@ PP_Var CreateObjectWithModuleDeprecated(PP_Module module_id, } const PPB_Var_Deprecated var_deprecated_interface = { - &Var::PluginAddRefPPVar, - &Var::PluginReleasePPVar, + &AddRefVar, + &ReleaseVar, &VarFromUtf8, &VarToUtf8, &HasPropertyDeprecated, @@ -436,8 +444,8 @@ const PPB_Var_Deprecated var_deprecated_interface = { }; const PPB_Var var_interface = { - &Var::PluginAddRefPPVar, - &Var::PluginReleasePPVar, + &AddRefVar, + &ReleaseVar, &VarFromUtf8, &VarToUtf8 }; diff --git a/webkit/plugins/ppapi/resource_tracker.cc b/webkit/plugins/ppapi/resource_tracker.cc index 3d050a5..5e15103 100644 --- a/webkit/plugins/ppapi/resource_tracker.cc +++ b/webkit/plugins/ppapi/resource_tracker.cc @@ -12,6 +12,7 @@ #include "ppapi/c/pp_resource.h" #include "ppapi/c/pp_var.h" #include "ppapi/shared_impl/function_group_base.h" +#include "ppapi/shared_impl/id_assignment.h" #include "ppapi/shared_impl/tracker_base.h" #include "webkit/plugins/ppapi/npobject_var.h" #include "webkit/plugins/ppapi/plugin_module.h" @@ -23,36 +24,12 @@ #include "webkit/plugins/ppapi/resource.h" #include "webkit/plugins/ppapi/resource_creation_impl.h" +using ppapi::CheckIdType; +using ppapi::MakeTypedId; using ppapi::NPObjectVar; +using ppapi::PPIdType; using ppapi::Var; -enum PPIdType { - PP_ID_TYPE_MODULE, - PP_ID_TYPE_INSTANCE, - PP_ID_TYPE_RESOURCE, - PP_ID_TYPE_VAR, - PP_ID_TYPE_COUNT -}; - -static const unsigned int kPPIdTypeBits = 2; -COMPILE_ASSERT(PP_ID_TYPE_COUNT <= (1<<kPPIdTypeBits), - kPPIdTypeBits_is_too_small_for_all_id_types); - -static const int32 kMaxPPIdType = - std::numeric_limits<int32>::max() >> kPPIdTypeBits; - -template <typename T> static inline T MakeTypedId(T value, PPIdType type) { - return (value << kPPIdTypeBits) | static_cast<T>(type); -} - -template <typename T> static inline bool CheckIdType(T id, PPIdType type) { - // 0 is a valid resource. - if (!id) - return true; - const T mask = (static_cast<T>(1) << kPPIdTypeBits) - 1; - return (id & mask) == type; -} - namespace webkit { namespace ppapi { @@ -89,7 +66,7 @@ struct ResourceTracker::InstanceData { }; scoped_refptr<Resource> ResourceTracker::GetResource(PP_Resource res) const { - DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE)) + DLOG_IF(ERROR, !CheckIdType(res, ::ppapi::PP_ID_TYPE_RESOURCE)) << res << " is not a PP_Resource."; ResourceMap::const_iterator result = live_resources_.find(res); if (result == live_resources_.end()) { @@ -103,8 +80,7 @@ ResourceTracker* ResourceTracker::global_tracker_ = NULL; ResourceTracker* ResourceTracker::singleton_override_ = NULL; ResourceTracker::ResourceTracker() - : last_resource_id_(0), - last_var_id_(0) { + : last_resource_id_(0) { // Wire up the new shared resource tracker base to use our implementation. ::ppapi::TrackerBase::Init(&GetTrackerBase); } @@ -145,11 +121,12 @@ void ResourceTracker::ResourceDestroyed(Resource* resource) { PP_Resource ResourceTracker::AddResource(Resource* resource) { // If the plugin manages to create 1 billion resources, don't do crazy stuff. if (last_resource_id_ == - (std::numeric_limits<PP_Resource>::max() >> kPPIdTypeBits)) + (std::numeric_limits<PP_Resource>::max() >> ::ppapi::kPPIdTypeBits)) return 0; // Add the resource with plugin use-count 1. - PP_Resource new_id = MakeTypedId(++last_resource_id_, PP_ID_TYPE_RESOURCE); + PP_Resource new_id = MakeTypedId(++last_resource_id_, + ::ppapi::PP_ID_TYPE_RESOURCE); live_resources_.insert(std::make_pair(new_id, std::make_pair(resource, 1))); // Track associated with the instance. @@ -160,7 +137,7 @@ PP_Resource ResourceTracker::AddResource(Resource* resource) { } bool ResourceTracker::AddRefResource(PP_Resource res) { - DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE)) + DLOG_IF(ERROR, !CheckIdType(res, ::ppapi::PP_ID_TYPE_RESOURCE)) << res << " is not a PP_Resource."; ResourceMap::iterator i = live_resources_.find(res); if (i != live_resources_.end()) { @@ -175,7 +152,7 @@ bool ResourceTracker::AddRefResource(PP_Resource res) { } bool ResourceTracker::UnrefResource(PP_Resource res) { - DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE)) + DLOG_IF(ERROR, !CheckIdType(res, ::ppapi::PP_ID_TYPE_RESOURCE)) << res << " is not a PP_Resource."; ResourceMap::iterator i = live_resources_.find(res); if (i != live_resources_.end()) { @@ -197,7 +174,7 @@ bool ResourceTracker::UnrefResource(PP_Resource res) { void ResourceTracker::CleanupInstanceData(PP_Instance instance, bool delete_instance) { - DLOG_IF(ERROR, !CheckIdType(instance, PP_ID_TYPE_INSTANCE)) + DLOG_IF(ERROR, !CheckIdType(instance, ::ppapi::PP_ID_TYPE_INSTANCE)) << instance << " is not a PP_Instance."; InstanceMap::iterator found = instance_map_.find(instance); if (found == instance_map_.end()) { @@ -270,7 +247,7 @@ uint32 ResourceTracker::GetLiveObjectsForInstance( ::ppapi::ResourceObjectBase* ResourceTracker::GetResourceAPI( PP_Resource res) { - DLOG_IF(ERROR, !CheckIdType(res, PP_ID_TYPE_RESOURCE)) + DLOG_IF(ERROR, !CheckIdType(res, ::ppapi::PP_ID_TYPE_RESOURCE)) << res << " is not a PP_Resource."; ResourceMap::const_iterator result = live_resources_.find(res); if (result == live_resources_.end()) @@ -327,55 +304,8 @@ PP_Instance ResourceTracker::GetInstanceForResource(PP_Resource pp_resource) { return resource->instance()->pp_instance(); } -int32 ResourceTracker::AddVar(Var* var) { - // If the plugin manages to create 1B strings... - if (last_var_id_ == kMaxPPIdType) - return 0; - - // Validate the module. - if (!GetModule(var->pp_module())) - 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))); - - return new_id; -} - -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()) - return scoped_refptr<Var>(); - return result->second.first; -} - -bool ResourceTracker::AddRefVar(int32 var_id) { - DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR)) - << var_id << " is not a PP_Var ID."; - VarMap::iterator i = live_vars_.find(var_id); - if (i != live_vars_.end()) { - // We don't protect against overflow, since a plugin as malicious as to ref - // once per every byte in the address space could have just as well unrefed - // one time too many. - ++i->second.second; - return true; - } - return false; -} - -bool ResourceTracker::UnrefVar(int32 var_id) { - DLOG_IF(ERROR, !CheckIdType(var_id, PP_ID_TYPE_VAR)) - << 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) - live_vars_.erase(i); - return true; - } - return false; +::ppapi::VarTracker* ResourceTracker::GetVarTracker() { + return &var_tracker_; } void ResourceTracker::AddNPObjectVar(NPObjectVar* object_var) { @@ -426,7 +356,7 @@ PP_Instance ResourceTracker::AddInstance(PluginInstance* instance) { PP_Instance new_instance; do { new_instance = MakeTypedId(static_cast<PP_Instance>(base::RandUint64()), - PP_ID_TYPE_INSTANCE); + ::ppapi::PP_ID_TYPE_INSTANCE); } while (!new_instance || instance_map_.find(new_instance) != instance_map_.end() || !instance->module()->ReserveInstanceID(new_instance)); @@ -445,7 +375,7 @@ void ResourceTracker::InstanceCrashed(PP_Instance instance) { } PluginInstance* ResourceTracker::GetInstance(PP_Instance instance) { - DLOG_IF(ERROR, !CheckIdType(instance, PP_ID_TYPE_INSTANCE)) + DLOG_IF(ERROR, !CheckIdType(instance, ::ppapi::PP_ID_TYPE_INSTANCE)) << instance << " is not a PP_Instance."; InstanceMap::iterator found = instance_map_.find(instance); if (found == instance_map_.end()) @@ -465,7 +395,7 @@ PP_Module ResourceTracker::AddModule(PluginModule* module) { PP_Module new_module; do { new_module = MakeTypedId(static_cast<PP_Module>(base::RandUint64()), - PP_ID_TYPE_MODULE); + ::ppapi::PP_ID_TYPE_MODULE); } while (!new_module || module_map_.find(new_module) != module_map_.end()); module_map_[new_module] = module; @@ -473,7 +403,7 @@ PP_Module ResourceTracker::AddModule(PluginModule* module) { } void ResourceTracker::ModuleDeleted(PP_Module module) { - DLOG_IF(ERROR, !CheckIdType(module, PP_ID_TYPE_MODULE)) + DLOG_IF(ERROR, !CheckIdType(module, ::ppapi::PP_ID_TYPE_MODULE)) << module << " is not a PP_Module."; ModuleMap::iterator found = module_map_.find(module); if (found == module_map_.end()) { @@ -484,7 +414,7 @@ void ResourceTracker::ModuleDeleted(PP_Module module) { } PluginModule* ResourceTracker::GetModule(PP_Module module) { - DLOG_IF(ERROR, !CheckIdType(module, PP_ID_TYPE_MODULE)) + DLOG_IF(ERROR, !CheckIdType(module, ::ppapi::PP_ID_TYPE_MODULE)) << module << " is not a PP_Module."; ModuleMap::iterator found = module_map_.find(module); if (found == module_map_.end()) diff --git a/webkit/plugins/ppapi/resource_tracker.h b/webkit/plugins/ppapi/resource_tracker.h index faf5aec..7fd445b 100644 --- a/webkit/plugins/ppapi/resource_tracker.h +++ b/webkit/plugins/ppapi/resource_tracker.h @@ -21,6 +21,7 @@ #include "ppapi/proxy/interface_id.h" #include "ppapi/shared_impl/function_group_base.h" #include "ppapi/shared_impl/tracker_base.h" +#include "ppapi/shared_impl/var_tracker.h" typedef struct NPObject NPObject; @@ -63,22 +64,17 @@ class ResourceTracker : public ::ppapi::TrackerBase { // Returns the number of resources associated with this module. uint32 GetLiveObjectsForInstance(PP_Instance instance) const; - // ResourceTrackerBase. + // TrackerBase. virtual ::ppapi::ResourceObjectBase* GetResourceAPI( PP_Resource res) OVERRIDE; virtual ::ppapi::FunctionGroupBase* GetFunctionAPI( PP_Instance pp_instance, pp::proxy::InterfaceID id) OVERRIDE; virtual PP_Instance GetInstanceForResource(PP_Resource resource) OVERRIDE; + virtual ::ppapi::VarTracker* GetVarTracker() OVERRIDE; // PP_Vars ------------------------------------------------------------------- - // TrackerBase implementation. - virtual int32 AddVar(::ppapi::Var* var) OVERRIDE; - virtual scoped_refptr< ::ppapi::Var > GetVar(int32 var_id) const OVERRIDE; - virtual bool AddRefVar(int32 var_id) OVERRIDE; - virtual bool UnrefVar(int32 var_id) OVERRIDE; - // Tracks all live NPObjectVar. This is so we can map between instance + // NPObject and get the NPObjectVar corresponding to it. This Add/Remove // function is called by the NPObjectVar when it is created and @@ -182,9 +178,10 @@ class ResourceTracker : public ::ppapi::TrackerBase { // See SetSingletonOverride above. static ResourceTracker* singleton_override_; - // Last assigned resource & var ID. + // Last assigned resource ID. PP_Resource last_resource_id_; - int32 last_var_id_; + + ::ppapi::VarTracker var_tracker_; // For each PP_Resource, keep the Resource* (as refptr) and plugin use count. // This use count is different then Resource's RefCount, and is manipulated diff --git a/webkit/plugins/ppapi/resource_tracker_unittest.cc b/webkit/plugins/ppapi/resource_tracker_unittest.cc index 89590e8..1f2720c 100644 --- a/webkit/plugins/ppapi/resource_tracker_unittest.cc +++ b/webkit/plugins/ppapi/resource_tracker_unittest.cc @@ -216,14 +216,15 @@ TEST_F(ResourceTrackerTest, ReuseVar) { } // 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)); + ::ppapi::VarTracker* var_tracker = tracker().GetVarTracker(); + var_tracker->ReleaseVar(static_cast<int32_t>(pp_object2.value.as_id)); + var_tracker->ReleaseVar(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 = 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)); + var_tracker->ReleaseVar(static_cast<int32_t>(pp_object3.value.as_id)); } } // namespace ppapi |