Bug 641685 - Make plugin object map a singleton in the plugin process (r=bsmedberg)
authorBill McCloskey <wmccloskey@mozilla.com>
Wed, 29 Oct 2014 08:04:51 -0700
changeset 212882 5e6215461462eeaed579a3742de2d225306e00f7
parent 212881 703cf7b92df4c2e68770cfba9ef8610e3b5b77e3
child 212883 55310850248c649cbfc0c101d998b2c3a0837e7d
push id51079
push userwmccloskey@mozilla.com
push dateWed, 29 Oct 2014 15:05:53 +0000
treeherdermozilla-inbound@0f7fd7a22708 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs641685
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 641685 - Make plugin object map a singleton in the plugin process (r=bsmedberg) This patches moves the object map (which tracks the PluginScriptableObjectChild and PluginInstanceChild for a given NPObject) from the PluginModuleChild to a global variable. This change prepares the way for having multiple PluginModuleChild instances in a given plugin process.
dom/plugins/ipc/PluginInstanceChild.cpp
dom/plugins/ipc/PluginInstanceChild.h
dom/plugins/ipc/PluginModuleChild.cpp
dom/plugins/ipc/PluginModuleChild.h
dom/plugins/ipc/PluginScriptableObjectChild.cpp
dom/plugins/ipc/PluginScriptableObjectChild.h
dom/plugins/test/testplugin/nptest.cpp
--- a/dom/plugins/ipc/PluginInstanceChild.cpp
+++ b/dom/plugins/ipc/PluginInstanceChild.cpp
@@ -2388,17 +2388,17 @@ PluginInstanceChild::GetActorForNPObject
     if (aObject->_class == PluginScriptableObjectChild::GetClass()) {
         // One of ours! It's a browser-provided object.
         ChildNPObject* object = static_cast<ChildNPObject*>(aObject);
         NS_ASSERTION(object->parent, "Null actor!");
         return object->parent;
     }
 
     PluginScriptableObjectChild* actor =
-        PluginModuleChild::current()->GetActorForNPObject(aObject);
+        PluginScriptableObjectChild::GetActorForNPObject(aObject);
     if (actor) {
         // Plugin-provided object that we've previously wrapped.
         return actor;
     }
 
     actor = new PluginScriptableObjectChild(LocalObject);
     if (!SendPPluginScriptableObjectConstructor(actor)) {
         NS_ERROR("Failed to send constructor message!");
@@ -3724,17 +3724,17 @@ PluginInstanceChild::AnswerNPP_Destroy(N
             mAsyncInvalidateTask->Cancel();
             mAsyncInvalidateTask = nullptr;
         }
     }
 
     ClearAllSurfaces();
 
     mDeletingHash = new nsTHashtable<DeletingObjectEntry>;
-    PluginModuleChild::current()->FindNPObjectsForInstance(this);
+    PluginScriptableObjectChild::NotifyOfInstanceShutdown(this);
 
     mDeletingHash->EnumerateEntries(InvalidateObject, nullptr);
     mDeletingHash->EnumerateEntries(DeleteObject, nullptr);
 
     // Null out our cached actors as they should have been killed in the
     // PluginInstanceDestroyed call above.
     mCachedWindowActor = nullptr;
     mCachedElementActor = nullptr;
--- a/dom/plugins/ipc/PluginInstanceChild.h
+++ b/dom/plugins/ipc/PluginInstanceChild.h
@@ -50,17 +50,18 @@ namespace plugins {
 class PBrowserStreamChild;
 class BrowserStreamChild;
 class StreamNotifyChild;
 
 class PluginInstanceChild : public PPluginInstanceChild
 {
     friend class BrowserStreamChild;
     friend class PluginStreamChild;
-    friend class StreamNotifyChild; 
+    friend class StreamNotifyChild;
+    friend class PluginScriptableObjectChild;
 
 #ifdef OS_WIN
     friend LRESULT CALLBACK PluginWindowProc(HWND hWnd,
                                              UINT message,
                                              WPARAM wParam,
                                              LPARAM lParam);
     static LRESULT CALLBACK PluginWindowProcInternal(HWND hWnd,
                                                      UINT message,
--- a/dom/plugins/ipc/PluginModuleChild.cpp
+++ b/dom/plugins/ipc/PluginModuleChild.cpp
@@ -727,69 +727,16 @@ const char*
 PluginModuleChild::GetUserAgent()
 {
     if (mUserAgent.IsVoid() && !CallNPN_UserAgent(&mUserAgent))
         return nullptr;
 
     return NullableStringGet(mUserAgent);
 }
 
-bool
-PluginModuleChild::RegisterActorForNPObject(NPObject* aObject,
-                                            PluginScriptableObjectChild* aActor)
-{
-    AssertPluginThread();
-    NS_ASSERTION(aObject && aActor, "Null pointer!");
-
-    NPObjectData* d = mObjectMap.GetEntry(aObject);
-    if (!d) {
-        NS_ERROR("NPObject not in object table");
-        return false;
-    }
-
-    d->actor = aActor;
-    return true;
-}
-
-void
-PluginModuleChild::UnregisterActorForNPObject(NPObject* aObject)
-{
-    AssertPluginThread();
-    NS_ASSERTION(aObject, "Null pointer!");
-
-    NPObjectData* d = mObjectMap.GetEntry(aObject);
-    NS_ASSERTION(d, "NPObject not in object table");
-    if (d) {
-        d->actor = nullptr;
-    }
-}
-
-PluginScriptableObjectChild*
-PluginModuleChild::GetActorForNPObject(NPObject* aObject)
-{
-    AssertPluginThread();
-    NS_ASSERTION(aObject, "Null pointer!");
-
-    NPObjectData* d = mObjectMap.GetEntry(aObject);
-    if (!d) {
-        NS_ERROR("Plugin using object not created with NPN_CreateObject?");
-        return nullptr;
-    }
-
-    return d->actor;
-}
-
-#ifdef DEBUG
-bool
-PluginModuleChild::NPObjectIsRegistered(NPObject* aObject)
-{
-    return !!mObjectMap.GetEntry(aObject);
-}
-#endif
-
 //-----------------------------------------------------------------------------
 // FIXME/cjones: just getting this out of the way for the moment ...
 
 namespace mozilla {
 namespace plugins {
 namespace child {
 
 static NPError
@@ -1544,17 +1491,17 @@ void
               const NPUTF8* aMessage)
 {
     PLUGIN_LOG_DEBUG_FUNCTION;
     ENSURE_PLUGIN_THREAD_VOID();
 
     PluginModuleChild* self = PluginModuleChild::current();
     PluginScriptableObjectChild* actor = nullptr;
     if (aNPObj) {
-        actor = self->GetActorForNPObject(aNPObj);
+        actor = PluginScriptableObjectChild::GetActorForNPObject(aNPObj);
         if (!actor) {
             NS_ERROR("Failed to get actor!");
             return;
         }
     }
 
     self->SendNPN_SetException(static_cast<PPluginScriptableObjectChild*>(actor),
                                NullableString(aMessage));
@@ -2045,20 +1992,17 @@ PluginModuleChild::NPN_CreateObject(NPP 
     }
 
     if (newObject) {
         newObject->_class = aClass;
         newObject->referenceCount = 1;
         NS_LOG_ADDREF(newObject, 1, "NPObject", sizeof(NPObject));
     }
 
-    NPObjectData* d = static_cast<PluginModuleChild*>(i->Manager())
-        ->mObjectMap.PutEntry(newObject);
-    NS_ASSERTION(!d->instance, "New NPObject already mapped?");
-    d->instance = i;
+    PluginScriptableObjectChild::RegisterObject(newObject, i);
 
     return newObject;
 }
 
 NPObject*
 PluginModuleChild::NPN_RetainObject(NPObject* aNPObj)
 {
     AssertPluginThread();
@@ -2072,25 +2016,25 @@ PluginModuleChild::NPN_RetainObject(NPOb
     return aNPObj;
 }
 
 void
 PluginModuleChild::NPN_ReleaseObject(NPObject* aNPObj)
 {
     AssertPluginThread();
 
-    NPObjectData* d = current()->mObjectMap.GetEntry(aNPObj);
-    if (!d) {
+    PluginInstanceChild* instance = PluginScriptableObjectChild::GetInstanceForNPObject(aNPObj);
+    if (!instance) {
         NS_ERROR("Releasing object not in mObjectMap?");
         return;
     }
 
     DeletingObjectEntry* doe = nullptr;
-    if (d->instance->mDeletingHash) {
-        doe = d->instance->mDeletingHash->GetEntry(aNPObj);
+    if (instance->mDeletingHash) {
+        doe = instance->mDeletingHash->GetEntry(aNPObj);
         if (!doe) {
             NS_ERROR("An object for a destroyed instance isn't in the instance deletion hash");
             return;
         }
         if (doe->mDeleted)
             return;
     }
 
@@ -2109,39 +2053,21 @@ void
 PluginModuleChild::DeallocNPObject(NPObject* aNPObj)
 {
     if (aNPObj->_class && aNPObj->_class->deallocate) {
         aNPObj->_class->deallocate(aNPObj);
     } else {
         child::_memfree(aNPObj);
     }
 
-    NPObjectData* d = current()->mObjectMap.GetEntry(aNPObj);
-    if (d->actor)
-        d->actor->NPObjectDestroyed();
-
-    current()->mObjectMap.RemoveEntry(aNPObj);
-}
+    PluginScriptableObjectChild* actor = PluginScriptableObjectChild::GetActorForNPObject(aNPObj);
+    if (actor)
+        actor->NPObjectDestroyed();
 
-void
-PluginModuleChild::FindNPObjectsForInstance(PluginInstanceChild* instance)
-{
-    NS_ASSERTION(instance->mDeletingHash, "filling null mDeletingHash?");
-    mObjectMap.EnumerateEntries(CollectForInstance, instance);
-}
-
-PLDHashOperator
-PluginModuleChild::CollectForInstance(NPObjectData* d, void* userArg)
-{
-    PluginInstanceChild* instance = static_cast<PluginInstanceChild*>(userArg);
-    if (d->instance == instance) {
-        NPObject* o = d->GetKey();
-        instance->mDeletingHash->PutEntry(o);
-    }
-    return PL_DHASH_NEXT;
+    PluginScriptableObjectChild::UnregisterObject(aNPObj);
 }
 
 NPIdentifier
 PluginModuleChild::NPN_GetStringIdentifier(const NPUTF8* aName)
 {
     PLUGIN_LOG_DEBUG_FUNCTION;
     AssertPluginThread();
 
--- a/dom/plugins/ipc/PluginModuleChild.h
+++ b/dom/plugins/ipc/PluginModuleChild.h
@@ -152,27 +152,16 @@ public:
     void CleanUp();
 
     const char* GetUserAgent();
 
     static const NPNetscapeFuncs sBrowserFuncs;
 
     static PluginModuleChild* current();
 
-    bool RegisterActorForNPObject(NPObject* aObject,
-                                  PluginScriptableObjectChild* aActor);
-
-    void UnregisterActorForNPObject(NPObject* aObject);
-
-    PluginScriptableObjectChild* GetActorForNPObject(NPObject* aObject);
-
-#ifdef DEBUG
-    bool NPObjectIsRegistered(NPObject* aObject);
-#endif
-
     /**
      * The child implementation of NPN_CreateObject.
      */
     static NPObject* NPN_CreateObject(NPP aNPP, NPClass* aClass);
     /**
      * The child implementation of NPN_RetainObject.
      */
     static NPObject* NPN_RetainObject(NPObject* aNPObj);
@@ -348,56 +337,28 @@ private:
     // g_main_context_iteration, or 0 when dispatched directly from
     // MessagePumpForUI.
     int mTopLoopDepth;
 #  endif
 #elif defined (MOZ_WIDGET_QT)
     NestedLoopTimer *mNestedLoopTimerObject;
 #endif
 
-    struct NPObjectData : public nsPtrHashKey<NPObject>
-    {
-        explicit NPObjectData(const NPObject* key)
-            : nsPtrHashKey<NPObject>(key)
-            , instance(nullptr)
-            , actor(nullptr)
-        { }
-
-        // never nullptr
-        PluginInstanceChild* instance;
-
-        // sometimes nullptr (no actor associated with an NPObject)
-        PluginScriptableObjectChild* actor;
-    };
-    /**
-     * mObjectMap contains all the currently active NPObjects (from NPN_CreateObject until the
-     * final release/dealloc, whether or not an actor is currently associated with the object.
-     */
-    nsTHashtable<NPObjectData> mObjectMap;
-
 public: // called by PluginInstanceChild
     /**
      * Dealloc an NPObject after last-release or when the associated instance
      * is destroyed. This function will remove the object from mObjectMap.
      */
     static void DeallocNPObject(NPObject* o);
 
     NPError NPP_Destroy(PluginInstanceChild* instance) {
         return mFunctions.destroy(instance->GetNPP(), 0);
     }
 
-    /**
-     * Fill PluginInstanceChild.mDeletingHash with all the remaining NPObjects
-     * associated with that instance.
-     */
-    void FindNPObjectsForInstance(PluginInstanceChild* instance);
-
 private:
-    static PLDHashOperator CollectForInstance(NPObjectData* d, void* userArg);
-
 #if defined(OS_WIN)
     virtual void EnteredCall() MOZ_OVERRIDE;
     virtual void ExitedCall() MOZ_OVERRIDE;
 
     // Entered/ExitedCall notifications keep track of whether the plugin has
     // entered a nested event loop within this interrupt call.
     struct IncallFrame
     {
--- a/dom/plugins/ipc/PluginScriptableObjectChild.cpp
+++ b/dom/plugins/ipc/PluginScriptableObjectChild.cpp
@@ -534,17 +534,17 @@ PluginScriptableObjectChild::PluginScrip
   AssertPluginThread();
 }
 
 PluginScriptableObjectChild::~PluginScriptableObjectChild()
 {
   AssertPluginThread();
 
   if (mObject) {
-    PluginModuleChild::current()->UnregisterActorForNPObject(mObject);
+    UnregisterActor(mObject);
 
     if (mObject->_class == GetClass()) {
       NS_ASSERTION(mType == Proxy, "Wrong type!");
       static_cast<ChildNPObject*>(mObject)->parent = nullptr;
     }
     else {
       NS_ASSERTION(mType == LocalObject, "Wrong type!");
       PluginModuleChild::sBrowserFuncs.releaseobject(mObject);
@@ -564,18 +564,18 @@ PluginScriptableObjectChild::InitializeP
   NS_ASSERTION(mInstance, "Null manager?!");
 
   NPObject* object = CreateProxyObject();
   if (!object) {
     NS_ERROR("Failed to create object!");
     return false;
   }
 
-  if (!PluginModuleChild::current()->RegisterActorForNPObject(object, this)) {
-    NS_ERROR("RegisterActorForNPObject failed");
+  if (!RegisterActor(object)) {
+    NS_ERROR("RegisterActor failed");
     return false;
   }
 
   mObject = object;
   return true;
 }
 
 void
@@ -589,18 +589,18 @@ PluginScriptableObjectChild::InitializeL
   mInstance = static_cast<PluginInstanceChild*>(Manager());
   NS_ASSERTION(mInstance, "Null manager?!");
 
   PluginModuleChild::sBrowserFuncs.retainobject(aObject);
 
   NS_ASSERTION(!mProtectCount, "Should be zero!");
   mProtectCount++;
 
-  if (!PluginModuleChild::current()->RegisterActorForNPObject(aObject, this)) {
-    NS_ERROR("RegisterActorForNPObject failed");
+  if (!RegisterActor(aObject)) {
+    NS_ERROR("RegisterActor failed");
   }
 
   mObject = aObject;
 }
 
 NPObject*
 PluginScriptableObjectChild::CreateProxyObject()
 {
@@ -683,17 +683,17 @@ void
 PluginScriptableObjectChild::DropNPObject()
 {
   NS_ASSERTION(mObject, "Invalidated object!");
   NS_ASSERTION(mObject->_class == GetClass(), "Wrong type of object!");
   NS_ASSERTION(mType == Proxy, "Shouldn't call this for non-proxy object!");
 
   // We think we're about to be deleted, but we could be racing with the other
   // process.
-  PluginModuleChild::current()->UnregisterActorForNPObject(mObject);
+  UnregisterActor(mObject);
   mObject = nullptr;
 
   SendUnprotect();
 }
 
 void
 PluginScriptableObjectChild::NPObjectDestroyed()
 {
@@ -1176,8 +1176,110 @@ PluginScriptableObjectChild::Evaluate(NP
 
   if (!success) {
     return false;
   }
 
   ConvertToVariant(result, *aResult);
   return true;
 }
+
+nsTHashtable<PluginScriptableObjectChild::NPObjectData>* PluginScriptableObjectChild::sObjectMap;
+
+bool
+PluginScriptableObjectChild::RegisterActor(NPObject* aObject)
+{
+  AssertPluginThread();
+  MOZ_ASSERT(aObject, "Null pointer!");
+
+  NPObjectData* d = sObjectMap->GetEntry(aObject);
+  if (!d) {
+    NS_ERROR("NPObject not in object table");
+    return false;
+  }
+
+  d->actor = this;
+  return true;
+}
+
+void
+PluginScriptableObjectChild::UnregisterActor(NPObject* aObject)
+{
+  AssertPluginThread();
+  MOZ_ASSERT(aObject, "Null pointer!");
+
+  NPObjectData* d = sObjectMap->GetEntry(aObject);
+  MOZ_ASSERT(d, "NPObject not in object table");
+  if (d) {
+    d->actor = nullptr;
+  }
+}
+
+/* static */ PluginScriptableObjectChild*
+PluginScriptableObjectChild::GetActorForNPObject(NPObject* aObject)
+{
+  AssertPluginThread();
+  MOZ_ASSERT(aObject, "Null pointer!");
+
+  NPObjectData* d = sObjectMap->GetEntry(aObject);
+  if (!d) {
+    NS_ERROR("Plugin using object not created with NPN_CreateObject?");
+    return nullptr;
+  }
+
+  return d->actor;
+}
+
+/* static */ void
+PluginScriptableObjectChild::RegisterObject(NPObject* aObject, PluginInstanceChild* aInstance)
+{
+  AssertPluginThread();
+
+  if (!sObjectMap) {
+    sObjectMap = new nsTHashtable<PluginScriptableObjectChild::NPObjectData>();
+  }
+
+  NPObjectData* d = sObjectMap->PutEntry(aObject);
+  MOZ_ASSERT(!d->instance, "New NPObject already mapped?");
+  d->instance = aInstance;
+}
+
+/* static */ void
+PluginScriptableObjectChild::UnregisterObject(NPObject* aObject)
+{
+  AssertPluginThread();
+
+  sObjectMap->RemoveEntry(aObject);
+
+  if (!sObjectMap->Count()) {
+    delete sObjectMap;
+    sObjectMap = nullptr;
+  }
+}
+
+/* static */ PluginInstanceChild*
+PluginScriptableObjectChild::GetInstanceForNPObject(NPObject* aObject)
+{
+  AssertPluginThread();
+  NPObjectData* d = sObjectMap->GetEntry(aObject);
+  if (!d) {
+    return nullptr;
+  }
+  return d->instance;
+}
+
+/* static */ PLDHashOperator
+PluginScriptableObjectChild::CollectForInstance(NPObjectData* d, void* userArg)
+{
+    PluginInstanceChild* instance = static_cast<PluginInstanceChild*>(userArg);
+    if (d->instance == instance) {
+        NPObject* o = d->GetKey();
+        instance->mDeletingHash->PutEntry(o);
+    }
+    return PL_DHASH_NEXT;
+}
+
+/* static */ void
+PluginScriptableObjectChild::NotifyOfInstanceShutdown(PluginInstanceChild* aInstance)
+{
+  AssertPluginThread();
+  sObjectMap->EnumerateEntries(CollectForInstance, aInstance);
+}
--- a/dom/plugins/ipc/PluginScriptableObjectChild.h
+++ b/dom/plugins/ipc/PluginScriptableObjectChild.h
@@ -212,16 +212,32 @@ public:
     DISALLOW_COPY_AND_ASSIGN(StackIdentifier);
 
     PluginIdentifier mIdentifier;
     nsRefPtr<StoredIdentifier> mStored;
   };
 
   static void ClearIdentifiers();
 
+  bool RegisterActor(NPObject* aObject);
+  void UnregisterActor(NPObject* aObject);
+
+  static PluginScriptableObjectChild* GetActorForNPObject(NPObject* aObject);
+
+  static void RegisterObject(NPObject* aObject, PluginInstanceChild* aInstance);
+  static void UnregisterObject(NPObject* aObject);
+
+  static PluginInstanceChild* GetInstanceForNPObject(NPObject* aObject);
+
+  /**
+   * Fill PluginInstanceChild.mDeletingHash with all the remaining NPObjects
+   * associated with that instance.
+   */
+  static void NotifyOfInstanceShutdown(PluginInstanceChild* aInstance);
+
 private:
   static NPObject*
   ScriptableAllocate(NPP aInstance,
                      NPClass* aClass);
 
   static void
   ScriptableInvalidate(NPObject* aObject);
 
@@ -292,14 +308,37 @@ private:
 
   static const NPClass sNPClass;
 
   static StoredIdentifier* HashIdentifier(const nsCString& aIdentifier);
   static void UnhashIdentifier(StoredIdentifier* aIdentifier);
 
   typedef nsDataHashtable<nsCStringHashKey, nsRefPtr<StoredIdentifier>> IdentifierTable;
   static IdentifierTable sIdentifiers;
+
+  struct NPObjectData : public nsPtrHashKey<NPObject>
+  {
+    explicit NPObjectData(const NPObject* key)
+    : nsPtrHashKey<NPObject>(key),
+      instance(nullptr),
+      actor(nullptr)
+    { }
+
+    // never nullptr
+    PluginInstanceChild* instance;
+
+    // sometimes nullptr (no actor associated with an NPObject)
+    PluginScriptableObjectChild* actor;
+  };
+
+  static PLDHashOperator CollectForInstance(NPObjectData* d, void* userArg);
+
+  /**
+   * mObjectMap contains all the currently active NPObjects (from NPN_CreateObject until the
+   * final release/dealloc, whether or not an actor is currently associated with the object.
+   */
+  static nsTHashtable<NPObjectData>* sObjectMap;
 };
 
 } /* namespace plugins */
 } /* namespace mozilla */
 
 #endif /* dom_plugins_PluginScriptableObjectChild_h */
--- a/dom/plugins/test/testplugin/nptest.cpp
+++ b/dom/plugins/test/testplugin/nptest.cpp
@@ -1980,16 +1980,20 @@ scriptableSetProperty(NPObject* npobj, N
 }
 
 bool
 scriptableRemoveProperty(NPObject* npobj, NPIdentifier name)
 {
   for (int i = 0; i < int(ARRAY_LENGTH(sPluginPropertyIdentifiers)); i++) {
     if (name == sPluginPropertyIdentifiers[i]) {
       NPN_ReleaseVariantValue(&sPluginPropertyValues[i]);
+
+      // Avoid double frees (see test_propertyAndMethod.html, which deletes a
+      // property that doesn't exist).
+      VOID_TO_NPVARIANT(sPluginPropertyValues[i]);
       return true;
     }
   }
   return false;
 }
 
 bool
 scriptableEnumerate(NPObject* npobj, NPIdentifier** identifier, uint32_t* count)