Bug 653083 - Deal with NPRuntime identifier which may be non-interned being passed across the process boundary, and the current cache which isn't aware of that problem. r=bent,cdleary

--HG--
extra : rebase_source : e21fac12c028e15f7d1a36988a6b66859c9addd8

dom/plugins/base/nsJSNPRuntime.cpp

@@ -334,7 +334,11 @@ struct AutoCXPusher
   }
 };
 
-static JSContext *
+namespace mozilla {
+namespace plugins {
+namespace parent {
+
+JSContext *
 GetJSContext(NPP npp)
 {
   NS_ENSURE_TRUE(npp, nsnull);
@@ -360,6 +364,9 @@ GetJSContext(NPP npp)
   return (JSContext *)scx->GetNativeContext();
 }
 
+}
+}
+}
 
 static NPP
 LookupNPP(NPObject *npobj);

dom/plugins/base/nsNPAPIPlugin.h

@@ -117,7 +117,7 @@ namespace parent {
 
 JS_STATIC_ASSERT(sizeof(NPIdentifier) == sizeof(jsid));
 
-static inline jsid
+inline jsid
 NPIdentifierToJSId(NPIdentifier id)
 {
     jsid tmp;
@@ -125,52 +125,59 @@ NPIdentifierToJSId(NPIdentifier id)
     return tmp;
 }
 
-static inline NPIdentifier
+inline NPIdentifier
 JSIdToNPIdentifier(jsid id)
 {
     return (NPIdentifier)JSID_BITS(id);
 }
 
-static inline bool
+inline bool
 NPIdentifierIsString(NPIdentifier id)
 {
     return JSID_IS_STRING(NPIdentifierToJSId(id));
 }
 
-static inline JSString *
+inline JSString *
 NPIdentifierToString(NPIdentifier id)
 {
     return JSID_TO_STRING(NPIdentifierToJSId(id));
 }
 
-static inline NPIdentifier
+inline NPIdentifier
 StringToNPIdentifier(JSContext *cx, JSString *str)
 {
     return JSIdToNPIdentifier(INTERNED_STRING_TO_JSID(cx, str));
 }
 
-static inline bool
+inline bool
 NPIdentifierIsInt(NPIdentifier id)
 {
     return JSID_IS_INT(NPIdentifierToJSId(id));
 }
 
-static inline jsint
+inline jsint
 NPIdentifierToInt(NPIdentifier id)
 {
     return JSID_TO_INT(NPIdentifierToJSId(id));
 }
 
-static inline NPIdentifier
+inline NPIdentifier
 IntToNPIdentifier(jsint i)
 {
     return JSIdToNPIdentifier(INT_TO_JSID(i));
 }
 
-static inline bool
-NPIdentifierIsVoid(NPIdentifier id)
+JSContext* GetJSContext(NPP npp);
+
+inline bool
+NPStringIdentifierIsPermanent(NPP npp, NPIdentifier id)
 {
-    return JSID_IS_VOID(NPIdentifierToJSId(id));
+  JSContext* cx = GetJSContext(npp);
+  if (!cx) // OOM?
+    return false;
+
+  JSAutoRequest ar(cx);
+  return JS_StringHasBeenInterned(cx, NPIdentifierToString(id));
 }
 
 #define NPIdentifier_VOID (JSIdToNPIdentifier(JSID_VOID))

dom/plugins/ipc/Makefile.in

@@ -109,6 +109,8 @@ CPPSRCS = \
   ChildAsyncCall.cpp \
   ChildTimer.cpp \
   PluginMessageUtils.cpp \
+  PluginIdentifierChild.cpp \
+  PluginIdentifierParent.cpp \
   PluginInstanceChild.cpp \
   PluginInstanceParent.cpp \
   PluginModuleChild.cpp \

dom/plugins/ipc/PPluginIdentifier.ipdl

@@ -48,6 +48,13 @@ async protocol PPluginIdentifier
 {
   manager PPluginModule;
 
+parent:
+  /**
+   * If this is a temporary identifier, inform the parent that the plugin
+   * has made the identifier permanent by calling NPN_GetStringIdentifier.
+   */
+  async Retain();
+
 child:
   async __delete__();
 };

dom/plugins/ipc/PPluginModule.ipdl

@@ -65,9 +65,13 @@ both:
    * constructor with the same string or int argument then we create two actors
    * and detect the second instance in the child. We prevent the parent's actor
    * from leaking out to plugin code and only allow the child's to be used.
+   *
+   * When calling into the plugin, the parent may create a "temporary"
+   * identifier which is only valid for the lifetime of the current RPC frame.
    */
   async PPluginIdentifier(nsCString aString,
-                          int32_t aInt);
+                          int32_t aInt,
+                          bool temporary);
 
   // Window-specific message which instructs the RPC mechanism to enter
   // a nested event loop for the current RPC call.

dom/plugins/ipc/PluginIdentifierChild.cpp

@@ -0,0 +1,174 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * vim: sw=2 ts=2 et :
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Mozilla Plugins.
+ *
+ * The Initial Developer of the Original Code is
+ *   Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Ben Turner <bent.mozilla@gmail.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "PluginIdentifierChild.h"
+#include "PluginModuleChild.h"
+
+namespace mozilla {
+namespace plugins {
+
+void
+PluginIdentifierChild::MakePermanent()
+{
+  if (mCanonicalIdentifier) {
+    NS_ASSERTION(mCanonicalIdentifier->mHashed && mCanonicalIdentifier->mTemporaryRefs == 0,
+                 "Canonical identifiers should always be permanent.");
+    return; // nothing to do
+  }
+
+  if (!mHashed) {
+    NS_ASSERTION(mTemporaryRefs == 0, "Not hashed, but temporary refs?");
+
+    PluginIdentifierChild* c = GetCanonical();
+    if (c) {
+      NS_ASSERTION(c != this, "How did I get in the hash?");
+      mCanonicalIdentifier = c;
+      NS_ASSERTION(mCanonicalIdentifier->mHashed && mCanonicalIdentifier->mTemporaryRefs == 0,
+                   "Canonical identifiers should always be permanent.");
+      return;
+    }
+
+    Hash();
+    mHashed = true;
+    return;
+  }
+
+  if (mTemporaryRefs) {
+    SendRetain();
+    mTemporaryRefs = 0;
+  }
+}
+
+void
+PluginIdentifierChild::StartTemporary()
+{
+  if (mCanonicalIdentifier) {
+    NS_ASSERTION(mCanonicalIdentifier->mHashed && mCanonicalIdentifier->mTemporaryRefs == 0,
+                 "Canonical identifiers should always be permanent.");
+    return; // nothing to do
+  }
+
+  if (!mHashed) {
+    NS_ASSERTION(mTemporaryRefs == 0, "Not hashed, but temporary refs?");
+
+    PluginIdentifierChild* c = GetCanonical();
+    if (c) {
+      NS_ASSERTION(c != this, "How did I get in the hash?");
+      mCanonicalIdentifier = c;
+      NS_ASSERTION(mCanonicalIdentifier->mHashed && mCanonicalIdentifier->mTemporaryRefs == 0,
+                   "Canonical identifiers should always be permanent.");
+      return;
+    }
+
+    Hash();
+    mHashed = true;
+    mTemporaryRefs = 1;
+    return;
+  }
+
+  if (mTemporaryRefs)
+    ++mTemporaryRefs;
+}
+
+void
+PluginIdentifierChild::FinishTemporary()
+{
+  if (mCanonicalIdentifier)
+    return;
+
+  NS_ASSERTION(mHashed, "Finishing unhashed identifier?");
+  if (!mTemporaryRefs)
+    return;
+
+  --mTemporaryRefs;
+  if (mTemporaryRefs)
+    return;
+
+  Unhash();
+  mHashed = false;
+}
+
+PluginIdentifierChild*
+PluginIdentifierChildString::GetCanonical()
+{
+  PluginModuleChild* module = static_cast<PluginModuleChild*>(Manager());
+  return module->mStringIdentifiers.Get(mString);
+}
+
+void
+PluginIdentifierChildString::Hash()
+{
+  PluginModuleChild* module = static_cast<PluginModuleChild*>(Manager());
+  NS_ASSERTION(module->mStringIdentifiers.Get(mString) == NULL, "Replacing Hash?");
+  module->mStringIdentifiers.Put(mString, this);
+}
+
+void
+PluginIdentifierChildString::Unhash()
+{
+  PluginModuleChild* module = static_cast<PluginModuleChild*>(Manager());
+  NS_ASSERTION(module->mStringIdentifiers.Get(mString) == this, "Incorrect identifier hash?");
+  module->mStringIdentifiers.Remove(mString);
+}
+
+PluginIdentifierChild*
+PluginIdentifierChildInt::GetCanonical()
+{
+  PluginModuleChild* module = static_cast<PluginModuleChild*>(Manager());
+  return module->mIntIdentifiers.Get(mInt);
+}
+
+void
+PluginIdentifierChildInt::Hash()
+{
+  PluginModuleChild* module = static_cast<PluginModuleChild*>(Manager());
+  NS_ASSERTION(module->mIntIdentifiers.Get(mInt) == NULL, "Replacing Hash?");
+  module->mIntIdentifiers.Put(mInt, this);
+}
+
+void
+PluginIdentifierChildInt::Unhash()
+{
+  PluginModuleChild* module = static_cast<PluginModuleChild*>(Manager());
+  NS_ASSERTION(module->mIntIdentifiers.Get(mInt) == this, "Incorrect identifier hash?");
+  module->mIntIdentifiers.Remove(mInt);
+}
+
+} // namespace mozilla::plugins
+} // namespace mozilla

dom/plugins/ipc/PluginIdentifierChild.h

@@ -41,36 +41,72 @@
 #define dom_plugins_PluginIdentifierChild_h
 
 #include "mozilla/plugins/PPluginIdentifierChild.h"
-#include "mozilla/plugins/PluginModuleChild.h"
+#include "npapi.h"
+#include "npruntime.h"
 
 #include "nsStringGlue.h"
 
 namespace mozilla {
 namespace plugins {
 
+class PluginModuleChild;
+
+/**
+ * Plugin identifiers may be "temporary", see the comment on the
+ * PPluginIdentifier constructor for details. This means that any IPDL method
+ * which receives a PPluginIdentifierChild* parameter must use StackIdentifier
+ * to track it.
+ */
 class PluginIdentifierChild : public PPluginIdentifierChild
 {
   friend class PluginModuleChild;
 public:
   bool IsString()
   {
-    return reinterpret_cast<intptr_t>(mCanonicalIdentifier) & 1;
+    return mIsString;
   }
 
   NPIdentifier ToNPIdentifier()
   {
-    return reinterpret_cast<PluginIdentifierChild*>(
-      reinterpret_cast<intptr_t>(mCanonicalIdentifier) & ~1);
+    if (mCanonicalIdentifier) {
+      return mCanonicalIdentifier;
+    }
+
+    NS_ASSERTION(mHashed, "Handing out an unhashed identifier?");
+    return this;
   }
 
+  void MakePermanent();
+
+  class NS_STACK_CLASS StackIdentifier
+  {
+  public:
+    StackIdentifier(PPluginIdentifierChild* actor)
+      : mIdentifier(static_cast<PluginIdentifierChild*>(actor))
+    {
+      if (mIdentifier)
+        mIdentifier->StartTemporary();
+    }
+
+    ~StackIdentifier() {
+      if (mIdentifier)
+        mIdentifier->FinishTemporary();
+    }
+
+    PluginIdentifierChild* operator->() { return mIdentifier; }
+
+  private:
+    PluginIdentifierChild* mIdentifier;
+  };
+
 protected:
   PluginIdentifierChild(bool aIsString)
-    : ALLOW_THIS_IN_INITIALIZER_LIST(mCanonicalIdentifier(this))
+    : mCanonicalIdentifier(NULL)
+    , mHashed(false)
+    , mTemporaryRefs(0)
+    , mIsString(aIsString)
   {
     MOZ_COUNT_CTOR(PluginIdentifierChild);
-    if (aIsString) {
-      SetIsString();
-    }
   }
 
   virtual ~PluginIdentifierChild()
@@ -78,24 +114,37 @@ protected:
     MOZ_COUNT_DTOR(PluginIdentifierChild);
   }
 
-  void SetCanonicalIdentifier(PluginIdentifierChild* aIdentifier)
-  {
-    NS_ASSERTION(ToNPIdentifier() == this, "Already got one!");
-    bool isString = IsString();
-    mCanonicalIdentifier = aIdentifier;
-    if (isString) {
-      SetIsString();
-    }
-  }
+  // The following functions are implemented by the subclasses for their
+  // identifier maps.
+  virtual PluginIdentifierChild* GetCanonical() = 0;
+  virtual void Hash() = 0;
+  virtual void Unhash() = 0;
 
 private:
-  void SetIsString()
-  {
-    mCanonicalIdentifier = reinterpret_cast<PluginIdentifierChild*>(
-      reinterpret_cast<intptr_t>(mCanonicalIdentifier) | 1);
-  }
+  void StartTemporary();
+  void FinishTemporary();
+
+  // There's a possibility that we already have an actor that wraps the same
+  // string or int because we do all this identifier construction
+  // asynchronously. In this case we need to hand out the canonical version
+  // created by the child side.
+  //
+  // In order to deal with temporary identifiers which appear on the stack,
+  // identifiers use the following state invariants:
+  //
+  // * mCanonicalIdentifier is non-NULL: this is a duplicate identifier, no
+  //   further information is necessary.
+  // * mHashed is false: this identifier is a newborn, non-permanent identifier
+  // * mHashed is true, mTemporaryRefs is 0: this identifier is permanent
+  // * mHashed is true, mTemporaryRefs is non-0: this identifier is temporary;
+  //   if NPN_GetFooIdentifier is called for it, we need to retain it. If
+  //   all stack references are lost, unhash it because it will soon be 
+  //   deleted.
 
   PluginIdentifierChild* mCanonicalIdentifier;
+  bool mHashed;
+  unsigned int mTemporaryRefs;
+  bool mIsString;
 };
 
 class PluginIdentifierChildString : public PluginIdentifierChild
@@ -113,6 +162,10 @@ protected:
       mString(aString)
   { }
 
+  virtual PluginIdentifierChild* GetCanonical();
+  virtual void Hash();
+  virtual void Unhash();
+
   nsCString mString;
 };
 
@@ -131,6 +184,10 @@ protected:
       mInt(aInt)
   { }
 
+  virtual PluginIdentifierChild* GetCanonical();
+  virtual void Hash();
+  virtual void Unhash();
+
   int32_t mInt;
 };
 

dom/plugins/ipc/PluginIdentifierParent.cpp

@@ -0,0 +1,120 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * vim: sw=2 ts=2 et :
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Mozilla Plugins.
+ *
+ * The Initial Developer of the Original Code is
+ *   Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2010
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Ben Turner <bent.mozilla@gmail.com>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "PluginIdentifierParent.h"
+
+#include "nsServiceManagerUtils.h"
+#include "nsNPAPIPlugin.h"
+#include "nsIJSContextStack.h"
+#include "PluginScriptableObjectUtils.h"
+#include "mozilla/unused.h"
+
+using namespace mozilla::plugins::parent;
+
+namespace mozilla {
+namespace plugins {
+
+bool
+PluginIdentifierParent::RecvRetain()
+{
+  mTemporaryRefs = 0;
+
+  // Intern the jsid if necessary.
+  jsid id = NPIdentifierToJSId(mIdentifier);
+  if (JSID_IS_INT(id)) {
+    return true;
+  }
+
+  // The following is what nsNPAPIPlugin.cpp does. Gross, but the API doesn't
+  // give you a NPP to play with.
+  nsCOMPtr<nsIThreadJSContextStack> stack =
+    do_GetService("@mozilla.org/js/xpc/ContextStack;1");
+  if (!stack) {
+    return false;
+  }
+
+  JSContext *cx = nsnull;
+  stack->GetSafeJSContext(&cx);
+  if (!cx) {
+    return false;
+  }
+
+  JSAutoRequest ar(cx);
+  JSString* str = JSID_TO_STRING(id);
+  JSString* str2 = JS_InternJSString(cx, str);
+  if (!str2) {
+    return false;
+  }
+
+  NS_ASSERTION(str == str2, "Interning a string in a JSID should always return the same string.");
+
+  return true;
+}
+
+PluginIdentifierParent::StackIdentifier::StackIdentifier
+    (PluginInstanceParent* inst, NPIdentifier aIdentifier)
+  : mIdentifier(inst->Module()->GetIdentifierForNPIdentifier(inst->GetNPP(), aIdentifier))
+{
+}
+
+PluginIdentifierParent::StackIdentifier::StackIdentifier
+    (NPObject* aObject, NPIdentifier aIdentifier)
+  : mIdentifier(NULL)
+{
+  PluginInstanceParent* inst = GetInstance(aObject);
+  mIdentifier = inst->Module()->GetIdentifierForNPIdentifier(inst->GetNPP(), aIdentifier);
+}
+
+PluginIdentifierParent::StackIdentifier::~StackIdentifier()
+{
+  if (!mIdentifier) {
+    return;
+  }
+
+  if (!mIdentifier->IsTemporary()) {
+    return;
+  }
+
+  if (mIdentifier->RemoveTemporaryRef()) {
+    unused << PPluginIdentifierParent::Send__delete__(mIdentifier);
+  }
+}
+
+} // namespace mozilla::plugins
+} // namespace mozilla

dom/plugins/ipc/PluginIdentifierParent.h

@@ -48,6 +48,8 @@
 namespace mozilla {
 namespace plugins {
 
+class PluginInstanceParent;
+
 class PluginIdentifierParent : public PPluginIdentifierParent
 {
   friend class PluginModuleParent;
@@ -58,9 +60,34 @@ public:
     return mIdentifier;
   }
 
+  bool IsTemporary() {
+    return !!mTemporaryRefs;
+  }
+
+  /**
+   * Holds a perhaps-temporary identifier for the current stack frame.
+   */
+  class NS_STACK_CLASS StackIdentifier
+  {
+  public:
+    StackIdentifier(PluginInstanceParent* inst, NPIdentifier aIdentifier);
+    StackIdentifier(NPObject* aObject, NPIdentifier aIdentifier);
+    ~StackIdentifier();
+
+    operator PluginIdentifierParent*() {
+      return mIdentifier;
+    }
+
+  private:
+    DISALLOW_COPY_AND_ASSIGN(StackIdentifier);
+
+    PluginIdentifierParent* mIdentifier;
+  };
+
 protected:
-  PluginIdentifierParent(NPIdentifier aIdentifier)
+  PluginIdentifierParent(NPIdentifier aIdentifier, bool aTemporary)
     : mIdentifier(aIdentifier)
+    , mTemporaryRefs(aTemporary ? 1 : 0)
   {
     MOZ_COUNT_CTOR(PluginIdentifierParent);
   }
@@ -70,8 +97,23 @@ protected:
     MOZ_COUNT_DTOR(PluginIdentifierParent);
   }
 
+  virtual bool RecvRetain();
+
+  void AddTemporaryRef() {
+    mTemporaryRefs++;
+  }
+
+  /**
+   * @returns true if the last temporary reference was removed.
+   */
+  bool RemoveTemporaryRef() {
+    --mTemporaryRefs;
+    return !mTemporaryRefs;
+  }
+
 private:
   NPIdentifier mIdentifier;
+  unsigned int mTemporaryRefs;
 };
 
 } // namespace plugins

dom/plugins/ipc/PluginModuleChild.cpp

@@ -1775,30 +1775,27 @@ PluginModuleChild::AnswerNP_Initialize(NativeThreadId* tid, NPError* _retval)
 
 PPluginIdentifierChild*
 PluginModuleChild::AllocPPluginIdentifier(const nsCString& aString,
-                                          const int32_t& aInt)
+                                          const int32_t& aInt,
+                                          const bool& aTemporary)
 {
-    // There's a possibility that we already have an actor that wraps the same
-    // string or int because we do all this identifier construction
-    // asynchronously. Check to see if we've already wrapped here, and then set
-    // canonical actor of the new one to the actor already in our hash.
-    PluginIdentifierChild* newActor;
-    PluginIdentifierChild* existingActor;
-
+    // We cannot call SetPermanent within this function because Manager() isn't
+    // set up yet.
     if (aString.IsVoid()) {
-        newActor = new PluginIdentifierChildInt(aInt);
-        if (mIntIdentifiers.Get(aInt, &existingActor))
-            newActor->SetCanonicalIdentifier(existingActor);
-        else
-            mIntIdentifiers.Put(aInt, newActor);
+        return new PluginIdentifierChildInt(aInt);
     }
-    else {
-        newActor = new PluginIdentifierChildString(aString);
-        if (mStringIdentifiers.Get(aString, &existingActor))
-            newActor->SetCanonicalIdentifier(existingActor);
-        else
-            mStringIdentifiers.Put(aString, newActor);
+    return new PluginIdentifierChildString(aString);
+}
+
+bool
+PluginModuleChild::RecvPPluginIdentifierConstructor(PPluginIdentifierChild* actor,
+                                                    const nsCString& aString,
+                                                    const int32_t& aInt,
+                                                    const bool& aTemporary)
+{
+    if (!aTemporary) {
+        static_cast<PluginIdentifierChild*>(actor)->MakePermanent();
     }
-    return newActor;
+    return true;
 }
 
 bool
@@ -2101,15 +2098,14 @@ PluginModuleChild::NPN_GetStringIdentifier(const NPUTF8* aName)
     PluginModuleChild* self = PluginModuleChild::current();
     nsDependentCString name(aName);
 
-    PluginIdentifierChild* ident;
-    if (!self->mStringIdentifiers.Get(name, &ident)) {
+    PluginIdentifierChildString* ident = self->mStringIdentifiers.Get(name);
+    if (!ident) {
         nsCString nameCopy(name);
 
         ident = new PluginIdentifierChildString(nameCopy);
-        self->SendPPluginIdentifierConstructor(ident, nameCopy, -1);
-        self->mStringIdentifiers.Put(nameCopy, ident);
+        self->SendPPluginIdentifierConstructor(ident, nameCopy, -1, false);
     }
-
+    ident->MakePermanent();
     return ident;
 }
 
@@ -2133,14 +2129,14 @@ PluginModuleChild::NPN_GetStringIdentifiers(const NPUTF8** aNames,
             continue;
         }
         nsDependentCString name(aNames[index]);
-        PluginIdentifierChild* ident;
-        if (!self->mStringIdentifiers.Get(name, &ident)) {
+        PluginIdentifierChildString* ident = self->mStringIdentifiers.Get(name);
+        if (!ident) {
             nsCString nameCopy(name);
 
             ident = new PluginIdentifierChildString(nameCopy);
-            self->SendPPluginIdentifierConstructor(ident, nameCopy, -1);
-            self->mStringIdentifiers.Put(nameCopy, ident);
+            self->SendPPluginIdentifierConstructor(ident, nameCopy, -1, false);
         }
+        ident->MakePermanent();
         aIdentifiers[index] = ident;
     }
 }
@@ -2163,15 +2159,15 @@ PluginModuleChild::NPN_GetIntIdentifier(int32_t aIntId)
 
     PluginModuleChild* self = PluginModuleChild::current();
 
-    PluginIdentifierChild* ident;
-    if (!self->mIntIdentifiers.Get(aIntId, &ident)) {
+    PluginIdentifierChildInt* ident = self->mIntIdentifiers.Get(aIntId);
+    if (!ident) {
         nsCString voidString;
         voidString.SetIsVoid(PR_TRUE);
 
         ident = new PluginIdentifierChildInt(aIntId);
-        self->SendPPluginIdentifierConstructor(ident, voidString, aIntId);
-        self->mIntIdentifiers.Put(aIntId, ident);
+        self->SendPPluginIdentifierConstructor(ident, voidString, aIntId, false);
     }
+    ident->MakePermanent();
     return ident;
 }
 

dom/plugins/ipc/PluginModuleChild.h

@@ -117,7 +117,14 @@ protected:
 
     virtual PPluginIdentifierChild*
     AllocPPluginIdentifier(const nsCString& aString,
-                           const int32_t& aInt);
+                           const int32_t& aInt,
+                           const bool& aTemporary);
+
+    virtual bool
+    RecvPPluginIdentifierConstructor(PPluginIdentifierChild* actor,
+                                     const nsCString& aString,
+                                     const int32_t& aInt,
+                                     const bool& aTemporary);
 
     virtual bool
     DeallocPPluginIdentifier(PPluginIdentifierChild* aActor);
@@ -390,8 +397,11 @@ private:
      */
     nsTHashtable<NPObjectData> mObjectMap;
 
-    nsDataHashtable<nsCStringHashKey, PluginIdentifierChild*> mStringIdentifiers;
-    nsDataHashtable<nsUint32HashKey, PluginIdentifierChild*> mIntIdentifiers;
+    friend class PluginIdentifierChild;
+    friend class PluginIdentifierChildString;
+    friend class PluginIdentifierChildInt;
+    nsDataHashtable<nsCStringHashKey, PluginIdentifierChildString*> mStringIdentifiers;
+    nsDataHashtable<nsUint32HashKey, PluginIdentifierChildInt*> mIntIdentifiers;
 
 public: // called by PluginInstanceChild
     /**

dom/plugins/ipc/PluginModuleParent.cpp

@@ -73,6 +73,7 @@ using mozilla::ipc::SyncChannel;
 
 using namespace mozilla;
 using namespace mozilla::plugins;
+using namespace mozilla::plugins::parent;
 
 static const char kTimeoutPref[] = "dom.ipc.plugins.timeoutSecs";
 static const char kLaunchTimeoutPref[] = "dom.ipc.plugins.processLaunchTimeoutSecs";
@@ -341,8 +342,14 @@ PluginModuleParent::NotifyPluginCrashed()
 
 PPluginIdentifierParent*
 PluginModuleParent::AllocPPluginIdentifier(const nsCString& aString,
-                                           const int32_t& aInt)
+                                           const int32_t& aInt,
+                                           const bool& aTemporary)
 {
+    if (aTemporary) {
+        NS_ERROR("Plugins don't create temporary identifiers.");
+        return NULL; // should abort the plugin
+    }
+
     NPIdentifier npident = aString.IsVoid() ?
         mozilla::plugins::parent::_getintidentifier(aInt) :
         mozilla::plugins::parent::_getstringidentifier(aString.get());
@@ -352,7 +359,7 @@ PluginModuleParent::AllocPPluginIdentifier(const nsCString& aString,
         return nsnull;
     }
 
-    PluginIdentifierParent* ident = new PluginIdentifierParent(npident);
+    PluginIdentifierParent* ident = new PluginIdentifierParent(npident, false);
     mIdentifiers.Put(npident, ident);
     return ident;
 }
@@ -599,29 +606,39 @@ PluginModuleParent::AnswerNPN_UserAgent(nsCString* userAgent)
     return true;
 }
 
-PPluginIdentifierParent*
-PluginModuleParent::GetIdentifierForNPIdentifier(NPIdentifier aIdentifier)
+PluginIdentifierParent*
+PluginModuleParent::GetIdentifierForNPIdentifier(NPP npp, NPIdentifier aIdentifier)
 {
     PluginIdentifierParent* ident;
-    if (!mIdentifiers.Get(aIdentifier, &ident)) {
-        nsCString string;
-        int32_t intval = -1;
-        if (mozilla::plugins::parent::_identifierisstring(aIdentifier)) {
-            NPUTF8* chars =
-                mozilla::plugins::parent::_utf8fromidentifier(aIdentifier);
-            if (!chars) {
-                return nsnull;
-            }
-            string.Adopt(chars);
+    if (mIdentifiers.Get(aIdentifier, &ident)) {
+        if (ident->IsTemporary()) {
+            ident->AddTemporaryRef();
         }
-        else {
-            intval = mozilla::plugins::parent::_intfromidentifier(aIdentifier);
-            string.SetIsVoid(PR_TRUE);
-        }
-        ident = new PluginIdentifierParent(aIdentifier);
-        if (!SendPPluginIdentifierConstructor(ident, string, intval))
-            return nsnull;
+        return ident;
+    }
 
+    nsCString string;
+    int32_t intval = -1;
+    bool temporary = false;
+    if (mozilla::plugins::parent::_identifierisstring(aIdentifier)) {
+        NPUTF8* chars =
+            mozilla::plugins::parent::_utf8fromidentifier(aIdentifier);
+        if (!chars) {
+            return nsnull;
+        }
+        string.Adopt(chars);
+        temporary = !NPStringIdentifierIsPermanent(npp, aIdentifier);
+    }
+    else {
+        intval = mozilla::plugins::parent::_intfromidentifier(aIdentifier);
+        string.SetIsVoid(PR_TRUE);
+    }
+
+    ident = new PluginIdentifierParent(aIdentifier, temporary);
+    if (!SendPPluginIdentifierConstructor(ident, string, intval, temporary))
+        return nsnull;
+
+    if (!temporary) {
         mIdentifiers.Put(aIdentifier, ident);
     }
     return ident;

dom/plugins/ipc/PluginModuleParent.h

@@ -90,7 +90,8 @@ protected:
 
     virtual PPluginIdentifierParent*
     AllocPPluginIdentifier(const nsCString& aString,
-                           const int32_t& aInt);
+                           const int32_t& aInt,
+                           const bool& aTemporary);
 
     virtual bool
     DeallocPPluginIdentifier(PPluginIdentifierParent* aActor);
@@ -136,8 +137,14 @@ public:
         return !IsOnCxxStack();
     }
 
-    PPluginIdentifierParent*
-    GetIdentifierForNPIdentifier(NPIdentifier aIdentifier);
+    /**
+     * Get an identifier actor for this NPIdentifier. If this is a temporary
+     * identifier, the temporary refcount is increased by one. This method
+     * is intended only for use by StackIdentifier and the scriptable
+     * Enumerate hook.
+     */
+    PluginIdentifierParent*
+    GetIdentifierForNPIdentifier(NPP npp, NPIdentifier aIdentifier);
 
     void ProcessRemoteNativeEventsInRPCCall();
 

dom/plugins/ipc/PluginScriptableObjectChild.cpp

@@ -42,6 +42,10 @@
 
 using namespace mozilla::plugins;
 
+namespace {
+typedef PluginIdentifierChild::StackIdentifier StackIdentifier;
+}
+
 // static
 NPObject*
 PluginScriptableObjectChild::ScriptableAllocate(NPP aInstance,
@@ -660,7 +664,7 @@ PluginScriptableObjectChild::AnswerHasMethod(PPluginIdentifierChild* aId,
     return true;
   }
 
-  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
+  StackIdentifier id(aId);
   *aHasMethod = mObject->_class->hasMethod(mObject, id->ToNPIdentifier());
   return true;
 }
@@ -704,7 +708,7 @@ PluginScriptableObjectChild::AnswerInvoke(PPluginIdentifierChild* aId,
 
   NPVariant result;
   VOID_TO_NPVARIANT(result);
-  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
+  StackIdentifier id(aId);
   bool success = mObject->_class->invoke(mObject, id->ToNPIdentifier(),
                                          convertedArgs.Elements(), argCount,
                                          &result);
@@ -825,7 +829,7 @@ PluginScriptableObjectChild::AnswerHasProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
+  StackIdentifier id(aId);
   *aHasProperty = mObject->_class->hasProperty(mObject, id->ToNPIdentifier());
   return true;
 }
@@ -855,7 +859,8 @@ PluginScriptableObjectChild::AnswerGetChildProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
+  StackIdentifier stackID(aId);
+  NPIdentifier id = stackID->ToNPIdentifier();
 
   *aHasProperty = mObject->_class->hasProperty(mObject, id);
   *aHasMethod = mObject->_class->hasMethod(mObject, id);
@@ -901,7 +906,8 @@ PluginScriptableObjectChild::AnswerSetProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
+  StackIdentifier stackID(aId);
+  NPIdentifier id = stackID->ToNPIdentifier();
 
   if (!mObject->_class->hasProperty(mObject, id)) {
     *aSuccess = false;
@@ -938,7 +944,8 @@ PluginScriptableObjectChild::AnswerRemoveProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
+  StackIdentifier stackID(aId);
+  NPIdentifier id = stackID->ToNPIdentifier();
   *aSuccess = mObject->_class->hasProperty(mObject, id) ?
               mObject->_class->removeProperty(mObject, id) :
               true;

dom/plugins/ipc/PluginScriptableObjectParent.cpp

@@ -39,12 +39,16 @@
 #include "PluginScriptableObjectParent.h"
 #include "PluginScriptableObjectUtils.h"
 
+#include "nsNPAPIPlugin.h"
 #include "mozilla/unused.h"
 
 using namespace mozilla::plugins;
+using namespace mozilla::plugins::parent;
 
 namespace {
 
+typedef PluginIdentifierParent::StackIdentifier StackIdentifier;
+
 inline void
 ReleaseVariant(NPVariant& aVariant,
                PluginInstanceParent* aInstance)
@@ -55,32 +59,6 @@ ReleaseVariant(NPVariant& aVariant,
   }
 }
 
-inline PPluginIdentifierParent*
-GetIdentifier(PluginInstanceParent* aInstance,
-              NPIdentifier aIdentifier)
-{
-  PluginModuleParent* module = aInstance->Module();
-  if (!module) {
-    NS_WARNING("Huh?!");
-    return false;
-  }
-
-  return module->GetIdentifierForNPIdentifier(aIdentifier);
-}
-
-inline PPluginIdentifierParent*
-GetIdentifier(NPObject* aObject,
-              NPIdentifier aIdentifier)
-{
-  PluginInstanceParent* instance = GetInstance(aObject);
-  if (!instance) {
-    NS_WARNING("Huh?!");
-    return false;
-  }
-
-  return GetIdentifier(instance, aIdentifier);
-}
-
 } // anonymous namespace
 
 // static
@@ -154,7 +132,7 @@ PluginScriptableObjectParent::ScriptableHasMethod(NPObject* aObject,
     return false;
   }
 
-  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
+  StackIdentifier identifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -194,7 +172,7 @@ PluginScriptableObjectParent::ScriptableInvoke(NPObject* aObject,
     return false;
   }
 
-  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
+  StackIdentifier identifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -296,7 +274,7 @@ PluginScriptableObjectParent::ScriptableHasProperty(NPObject* aObject,
     return false;
   }
 
-  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
+  StackIdentifier identifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -345,7 +323,7 @@ PluginScriptableObjectParent::ScriptableSetProperty(NPObject* aObject,
     return false;
   }
 
-  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
+  StackIdentifier identifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -388,7 +366,7 @@ PluginScriptableObjectParent::ScriptableRemoveProperty(NPObject* aObject,
     return false;
   }
 
-  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
+  StackIdentifier identifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -1096,8 +1074,23 @@ PluginScriptableObjectParent::AnswerEnumerate(InfallibleTArray<PPluginIdentifier
     return true;
   }
 
+  JSContext* cx = GetJSContext(instance->GetNPP());
+  JSAutoRequest ar(cx);
+
   for (uint32_t index = 0; index < idCount; index++) {
-    aProperties->AppendElement(GetIdentifier(instance, ids[index]));
+    // Because of GC hazards, all identifiers returned from enumerate
+    // must be made permanent.
+    if (_identifierisstring(ids[index])) {
+      JSString* str = NPIdentifierToString(ids[index]);
+      if (!JS_StringHasBeenInterned(cx, str)) {
+        JSString* str2 = JS_InternJSString(cx, str);
+        NS_ASSERTION(str2 == str, "Interning a JS string which is currently an ID should return itself.");
+      }
+    }
+    PluginIdentifierParent* id =
+      instance->Module()->GetIdentifierForNPIdentifier(instance->GetNPP(), ids[index]);
+    aProperties->AppendElement(id);
+    NS_ASSERTION(!id->IsTemporary(), "Should only have permanent identifiers!");
   }
 
   npn->memfree(ids);
@@ -1268,7 +1261,7 @@ PluginScriptableObjectParent::GetPropertyHelper(NPIdentifier aName,
     return JS_FALSE;
   }
 
-  PPluginIdentifierParent* identifier = GetIdentifier(GetInstance(), aName);
+  StackIdentifier identifier(GetInstance(), aName);
   if (!identifier) {
     return JS_FALSE;
   }