Back out bug 653083 because of crashtest crashes (bug698862.html) in PluginInstanceParent::GetNPP for a crashed plugin

dom/plugins/base/nsJSNPRuntime.cpp

@@ -334,11 +334,7 @@ struct AutoCXPusher
   }
 };
 
-namespace mozilla {
+static JSContext *
-namespace plugins {
-namespace parent {
-
-JSContext *
 GetJSContext(NPP npp)
 {
   NS_ENSURE_TRUE(npp, nsnull);
@@ -364,9 +360,6 @@ 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));
 
-inline jsid
+static inline jsid
 NPIdentifierToJSId(NPIdentifier id)
 {
     jsid tmp;
@@ -125,59 +125,52 @@ NPIdentifierToJSId(NPIdentifier id)
     return tmp;
 }
 
-inline NPIdentifier
+static inline NPIdentifier
 JSIdToNPIdentifier(jsid id)
 {
     return (NPIdentifier)JSID_BITS(id);
 }
 
-inline bool
+static inline bool
 NPIdentifierIsString(NPIdentifier id)
 {
     return JSID_IS_STRING(NPIdentifierToJSId(id));
 }
 
-inline JSString *
+static inline JSString *
 NPIdentifierToString(NPIdentifier id)
 {
     return JSID_TO_STRING(NPIdentifierToJSId(id));
 }
 
-inline NPIdentifier
+static inline NPIdentifier
 StringToNPIdentifier(JSContext *cx, JSString *str)
 {
     return JSIdToNPIdentifier(INTERNED_STRING_TO_JSID(cx, str));
 }
 
-inline bool
+static inline bool
 NPIdentifierIsInt(NPIdentifier id)
 {
     return JSID_IS_INT(NPIdentifierToJSId(id));
 }
 
-inline jsint
+static inline jsint
 NPIdentifierToInt(NPIdentifier id)
 {
     return JSID_TO_INT(NPIdentifierToJSId(id));
 }
 
-inline NPIdentifier
+static inline NPIdentifier
 IntToNPIdentifier(jsint i)
 {
     return JSIdToNPIdentifier(INT_TO_JSID(i));
 }
 
-JSContext* GetJSContext(NPP npp);
+static inline bool
-
+NPIdentifierIsVoid(NPIdentifier id)
-inline bool
-NPStringIdentifierIsPermanent(NPP npp, NPIdentifier id)
 {
-  JSContext* cx = GetJSContext(npp);
+    return JSID_IS_VOID(NPIdentifierToJSId(id));
-  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,8 +109,6 @@ CPPSRCS = \
   ChildAsyncCall.cpp \
   ChildTimer.cpp \
   PluginMessageUtils.cpp \
-  PluginIdentifierChild.cpp \
-  PluginIdentifierParent.cpp \
   PluginInstanceChild.cpp \
   PluginInstanceParent.cpp \
   PluginModuleChild.cpp \

dom/plugins/ipc/PPluginIdentifier.ipdl

@@ -48,13 +48,6 @@ 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,13 +65,9 @@ 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

@@ -1,174 +0,0 @@
-/* -*- 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,72 +41,36 @@
 #define dom_plugins_PluginIdentifierChild_h
 
 #include "mozilla/plugins/PPluginIdentifierChild.h"
-#include "npapi.h"
+#include "mozilla/plugins/PluginModuleChild.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 mIsString;
+    return reinterpret_cast<intptr_t>(mCanonicalIdentifier) & 1;
   }
 
   NPIdentifier ToNPIdentifier()
   {
-    if (mCanonicalIdentifier) {
+    return reinterpret_cast<PluginIdentifierChild*>(
-      return mCanonicalIdentifier;
+      reinterpret_cast<intptr_t>(mCanonicalIdentifier) & ~1);
   }
 
-    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)
-    : mCanonicalIdentifier(NULL)
+    : ALLOW_THIS_IN_INITIALIZER_LIST(mCanonicalIdentifier(this))
-    , mHashed(false)
-    , mTemporaryRefs(0)
-    , mIsString(aIsString)
   {
     MOZ_COUNT_CTOR(PluginIdentifierChild);
+    if (aIsString) {
+      SetIsString();
+    }
   }
 
   virtual ~PluginIdentifierChild()
@@ -114,37 +78,24 @@ protected:
     MOZ_COUNT_DTOR(PluginIdentifierChild);
   }
 
-  // The following functions are implemented by the subclasses for their
+  void SetCanonicalIdentifier(PluginIdentifierChild* aIdentifier)
-  // identifier maps.
+  {
-  virtual PluginIdentifierChild* GetCanonical() = 0;
+    NS_ASSERTION(ToNPIdentifier() == this, "Already got one!");
-  virtual void Hash() = 0;
+    bool isString = IsString();
-  virtual void Unhash() = 0;
+    mCanonicalIdentifier = aIdentifier;
+    if (isString) {
+      SetIsString();
+    }
+  }
 
 private:
-  void StartTemporary();
+  void SetIsString()
-  void FinishTemporary();
+  {
-
+    mCanonicalIdentifier = reinterpret_cast<PluginIdentifierChild*>(
-  // There's a possibility that we already have an actor that wraps the same
+      reinterpret_cast<intptr_t>(mCanonicalIdentifier) | 1);
-  // 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
@@ -162,10 +113,6 @@ protected:
       mString(aString)
   { }
 
-  virtual PluginIdentifierChild* GetCanonical();
-  virtual void Hash();
-  virtual void Unhash();
-
   nsCString mString;
 };
 
@@ -184,10 +131,6 @@ protected:
       mInt(aInt)
   { }
 
-  virtual PluginIdentifierChild* GetCanonical();
-  virtual void Hash();
-  virtual void Unhash();
-
   int32_t mInt;
 };
 

dom/plugins/ipc/PluginIdentifierParent.cpp

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

dom/plugins/ipc/PluginModuleChild.h

@@ -117,14 +117,7 @@ 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);
@@ -397,11 +390,8 @@ private:
      */
     nsTHashtable<NPObjectData> mObjectMap;
 
-    friend class PluginIdentifierChild;
+    nsDataHashtable<nsCStringHashKey, PluginIdentifierChild*> mStringIdentifiers;
-    friend class PluginIdentifierChildString;
+    nsDataHashtable<nsUint32HashKey, PluginIdentifierChild*> mIntIdentifiers;
-    friend class PluginIdentifierChildInt;
-    nsDataHashtable<nsCStringHashKey, PluginIdentifierChildString*> mStringIdentifiers;
-    nsDataHashtable<nsUint32HashKey, PluginIdentifierChildInt*> mIntIdentifiers;
 
 public: // called by PluginInstanceChild
     /**

dom/plugins/ipc/PluginModuleParent.cpp

@@ -73,7 +73,6 @@ 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";
@@ -342,14 +341,8 @@ 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());
@@ -359,7 +352,7 @@ PluginModuleParent::AllocPPluginIdentifier(const nsCString& aString,
         return nsnull;
     }
 
-    PluginIdentifierParent* ident = new PluginIdentifierParent(npident, false);
+    PluginIdentifierParent* ident = new PluginIdentifierParent(npident);
     mIdentifiers.Put(npident, ident);
     return ident;
 }
@@ -606,20 +599,13 @@ PluginModuleParent::AnswerNPN_UserAgent(nsCString* userAgent)
     return true;
 }
 
-PluginIdentifierParent*
+PPluginIdentifierParent*
-PluginModuleParent::GetIdentifierForNPIdentifier(NPP npp, NPIdentifier aIdentifier)
+PluginModuleParent::GetIdentifierForNPIdentifier(NPIdentifier aIdentifier)
 {
     PluginIdentifierParent* ident;
-    if (mIdentifiers.Get(aIdentifier, &ident)) {
+    if (!mIdentifiers.Get(aIdentifier, &ident)) {
-        if (ident->IsTemporary()) {
-            ident->AddTemporaryRef();
-        }
-        return ident;
-    }
-
         nsCString string;
         int32_t intval = -1;
-    bool temporary = false;
         if (mozilla::plugins::parent::_identifierisstring(aIdentifier)) {
             NPUTF8* chars =
                 mozilla::plugins::parent::_utf8fromidentifier(aIdentifier);
@@ -627,18 +613,15 @@ PluginModuleParent::GetIdentifierForNPIdentifier(NPP npp, NPIdentifier aIdentifi
                 return nsnull;
             }
             string.Adopt(chars);
-        temporary = !NPStringIdentifierIsPermanent(npp, aIdentifier);
         }
         else {
             intval = mozilla::plugins::parent::_intfromidentifier(aIdentifier);
             string.SetIsVoid(PR_TRUE);
         }
-
+        ident = new PluginIdentifierParent(aIdentifier);
-    ident = new PluginIdentifierParent(aIdentifier, temporary);
+        if (!SendPPluginIdentifierConstructor(ident, string, intval))
-    if (!SendPPluginIdentifierConstructor(ident, string, intval, temporary))
             return nsnull;
 
-    if (!temporary) {
         mIdentifiers.Put(aIdentifier, ident);
     }
     return ident;

dom/plugins/ipc/PluginModuleParent.h

@@ -90,8 +90,7 @@ protected:
 
     virtual PPluginIdentifierParent*
     AllocPPluginIdentifier(const nsCString& aString,
-                           const int32_t& aInt,
+                           const int32_t& aInt);
-                           const bool& aTemporary);
 
     virtual bool
     DeallocPPluginIdentifier(PPluginIdentifierParent* aActor);
@@ -137,14 +136,8 @@ public:
         return !IsOnCxxStack();
     }
 
-    /**
+    PPluginIdentifierParent*
-     * Get an identifier actor for this NPIdentifier. If this is a temporary
+    GetIdentifierForNPIdentifier(NPIdentifier aIdentifier);
-     * 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,10 +42,6 @@
 
 using namespace mozilla::plugins;
 
-namespace {
-typedef PluginIdentifierChild::StackIdentifier StackIdentifier;
-}
-
 // static
 NPObject*
 PluginScriptableObjectChild::ScriptableAllocate(NPP aInstance,
@@ -664,7 +660,7 @@ PluginScriptableObjectChild::AnswerHasMethod(PPluginIdentifierChild* aId,
     return true;
   }
 
-  StackIdentifier id(aId);
+  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
   *aHasMethod = mObject->_class->hasMethod(mObject, id->ToNPIdentifier());
   return true;
 }
@@ -708,7 +704,7 @@ PluginScriptableObjectChild::AnswerInvoke(PPluginIdentifierChild* aId,
 
   NPVariant result;
   VOID_TO_NPVARIANT(result);
-  StackIdentifier id(aId);
+  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
   bool success = mObject->_class->invoke(mObject, id->ToNPIdentifier(),
                                          convertedArgs.Elements(), argCount,
                                          &result);
@@ -829,7 +825,7 @@ PluginScriptableObjectChild::AnswerHasProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  StackIdentifier id(aId);
+  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
   *aHasProperty = mObject->_class->hasProperty(mObject, id->ToNPIdentifier());
   return true;
 }
@@ -859,8 +855,7 @@ PluginScriptableObjectChild::AnswerGetChildProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  StackIdentifier stackID(aId);
+  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
-  NPIdentifier id = stackID->ToNPIdentifier();
 
   *aHasProperty = mObject->_class->hasProperty(mObject, id);
   *aHasMethod = mObject->_class->hasMethod(mObject, id);
@@ -906,8 +901,7 @@ PluginScriptableObjectChild::AnswerSetProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  StackIdentifier stackID(aId);
+  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
-  NPIdentifier id = stackID->ToNPIdentifier();
 
   if (!mObject->_class->hasProperty(mObject, id)) {
     *aSuccess = false;
@@ -944,8 +938,7 @@ PluginScriptableObjectChild::AnswerRemoveProperty(PPluginIdentifierChild* aId,
     return true;
   }
 
-  StackIdentifier stackID(aId);
+  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
-  NPIdentifier id = stackID->ToNPIdentifier();
   *aSuccess = mObject->_class->hasProperty(mObject, id) ?
               mObject->_class->removeProperty(mObject, id) :
               true;

dom/plugins/ipc/PluginScriptableObjectParent.cpp

@@ -39,16 +39,12 @@
 #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)
@@ -59,6 +55,32 @@ 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
@@ -132,7 +154,7 @@ PluginScriptableObjectParent::ScriptableHasMethod(NPObject* aObject,
     return false;
   }
 
-  StackIdentifier identifier(aObject, aName);
+  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -172,7 +194,7 @@ PluginScriptableObjectParent::ScriptableInvoke(NPObject* aObject,
     return false;
   }
 
-  StackIdentifier identifier(aObject, aName);
+  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -274,7 +296,7 @@ PluginScriptableObjectParent::ScriptableHasProperty(NPObject* aObject,
     return false;
   }
 
-  StackIdentifier identifier(aObject, aName);
+  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -323,7 +345,7 @@ PluginScriptableObjectParent::ScriptableSetProperty(NPObject* aObject,
     return false;
   }
 
-  StackIdentifier identifier(aObject, aName);
+  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -366,7 +388,7 @@ PluginScriptableObjectParent::ScriptableRemoveProperty(NPObject* aObject,
     return false;
   }
 
-  StackIdentifier identifier(aObject, aName);
+  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
   if (!identifier) {
     return false;
   }
@@ -1074,23 +1096,8 @@ PluginScriptableObjectParent::AnswerEnumerate(InfallibleTArray<PPluginIdentifier
     return true;
   }
 
-  JSContext* cx = GetJSContext(instance->GetNPP());
-  JSAutoRequest ar(cx);
-
   for (uint32_t index = 0; index < idCount; index++) {
-    // Because of GC hazards, all identifiers returned from enumerate
+    aProperties->AppendElement(GetIdentifier(instance, ids[index]));
-    // 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);
@@ -1261,7 +1268,7 @@ PluginScriptableObjectParent::GetPropertyHelper(NPIdentifier aName,
     return JS_FALSE;
   }
 
-  StackIdentifier identifier(GetInstance(), aName);
+  PPluginIdentifierParent* identifier = GetIdentifier(GetInstance(), aName);
   if (!identifier) {
     return JS_FALSE;
   }

dom/plugins/test/mochitest/Makefile.in

@@ -52,7 +52,6 @@ _MOCHITEST_FILES = \
   test_npruntime_npninvoke.html \
   test_npruntime_npninvokedefault.html \
   test_npruntime_identifiers.html \
-  npruntime_identifiers_subpage.html \
   loremipsum.txt \
   loremipsum_file.txt \
   loremipsum_nocache.txt \

dom/plugins/test/mochitest/npruntime_identifiers_subpage.html

@@ -1,4 +0,0 @@
-<html>
-  <body>
-  <embed id="plugin1" type="application/x-test" width="400" height="100">
-  </embed>

dom/plugins/test/mochitest/test_npruntime_identifiers.html

@@ -8,10 +8,11 @@
   <link rel="stylesheet" type="text/css" 
         href="/tests/SimpleTest/test.css" />
 </head>
-<body>
+<body onload="runTests()">
   <p id="display"></p>
 
-  <iframe id="subframe" src="npruntime_identifiers_subpage.html"></iframe>
+  <embed id="plugin1" type="application/x-test" width="400" height="100">
+  </embed>
 
   <script class="testbody" type="application/javascript">
   ////
@@ -20,43 +21,17 @@
   
   SimpleTest.waitForExplicitFinish();
 
-  var testsRun = 0;
+  function runTests() {
+    var reflector = document.getElementById("plugin1").getReflector();
 
-  document.getElementById('subframe').addEventListener('load', doTest, false);
+    for (var i = -10; i < 10; ++i)
-
-  function doTest() {
-    SpecialPowers.gc();
-
-    var reflector = document.getElementById("subframe").contentDocument.getElementById("plugin1").getReflector();
-
-    var i, prop, randomnumber;
-
-    for (i = 0; i < 20; ++i) {
-      randomnumber=Math.floor(Math.random()*1001);
-      prop = "prop" + randomnumber;
-      is(reflector[prop], prop, "Property " + prop);
-    }
-
-    for (i = -10; i < 10; ++i) {
       is(reflector[i], i, "Property " + i);
-      prop = "prop" + i;
-      is(reflector[prop], prop, "Property " + prop);
-    }
 
     is(reflector.a, 'a', "Property .a");
     is(reflector['a'], 'a', "Property ['a']");
-    reflector = null;
             
-    SpecialPowers.gc();
-
-    ++testsRun;
-    if (testsRun == 3) {
     SimpleTest.finish();
   }
-    else {
-      document.getElementById('subframe').contentWindow.location.reload(true);
-    }
-  }
   </script>
 </body>
 </html>