Bug 556849 - '[OOPP] Reduce unnecessary HasProperty calls for plugin scriptable objects'. r=jst+josh+bsmedberg.
☠☠ backed out by 91a74fb57fcb ☠ ☠
authorBen Turner <bent.mozilla@gmail.com>
Wed, 07 Apr 2010 13:55:10 -0700
changeset 40554 0ed67564770063b0a5b83236e019b9efe933ecaa
parent 40553 10d2046d2b641734e5d94da84ffc61698460b5e3
child 40555 70d96e0639a90036419a1022a574986b3375a466
child 40562 91a74fb57fcb5c5577fd92bd3781d33a8f108a4c
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjst
bugs556849
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 556849 - '[OOPP] Reduce unnecessary HasProperty calls for plugin scriptable objects'. r=jst+josh+bsmedberg.
dom/plugins/PPluginScriptableObject.ipdl
dom/plugins/PluginScriptableObjectChild.cpp
dom/plugins/PluginScriptableObjectChild.h
dom/plugins/PluginScriptableObjectParent.cpp
dom/plugins/PluginScriptableObjectParent.h
modules/plugin/base/src/nsJSNPRuntime.cpp
modules/plugin/test/mochitest/Makefile.in
modules/plugin/test/mochitest/test_propertyAndMethod.html
modules/plugin/test/testplugin/nptest.cpp
--- a/dom/plugins/PPluginScriptableObject.ipdl
+++ b/dom/plugins/PPluginScriptableObject.ipdl
@@ -87,20 +87,16 @@ both:
 
   rpc InvokeDefault(Variant[] aArgs)
     returns (Variant aResult,
              bool aSuccess);
 
   rpc HasProperty(PPluginIdentifier aId)
     returns (bool aHasProperty);
 
-  rpc GetProperty(PPluginIdentifier aId)
-    returns (Variant aResult,
-             bool aSuccess);
-
   rpc SetProperty(PPluginIdentifier aId,
                   Variant aValue)
     returns (bool aSuccess);
 
   rpc RemoveProperty(PPluginIdentifier aId)
     returns (bool aSuccess);
 
   rpc Enumerate()
@@ -115,12 +111,30 @@ both:
   // only affect protocol objects that represent NPObjects created in the same
   // process (rather than protocol objects that are a proxy for an NPObject
   // created in another process). Protocol objects representing local NPObjects
   // are protected after an NPObject has been associated with the protocol
   // object. Sending the protocol object as an argument to the other process
   // temporarily protects the protocol object again for the duration of the call.
   async Protect();
   async Unprotect();
+
+  /**
+   * GetProperty is slightly wonky due to the way we support NPObjects that have
+   * methods and properties with the same name. When child calls parent we
+   * simply return a property. When parent calls child, however, we need to do
+   * several checks at once and return all the results simultaneously.
+   */
+parent:
+  rpc GetParentProperty(PPluginIdentifier aId)
+    returns (Variant aResult,
+             bool aSuccess);
+
+child:
+  rpc GetChildProperty(PPluginIdentifier aId)
+    returns (bool aHasProperty,
+             bool aHasMethod,
+             Variant aResult,
+             bool aSuccess);
 };
 
 } // namespace plugins
 } // namespace mozilla
--- a/dom/plugins/PluginScriptableObjectChild.cpp
+++ b/dom/plugins/PluginScriptableObjectChild.cpp
@@ -252,18 +252,18 @@ PluginScriptableObjectChild::ScriptableG
   }
 
   ProtectedActor<PluginScriptableObjectChild> actor(object->parent);
   NS_ASSERTION(actor, "This shouldn't ever be null!");
   NS_ASSERTION(actor->Type() == Proxy, "Bad type!");
 
   Variant result;
   bool success;
-  actor->CallGetProperty(static_cast<PPluginIdentifierChild*>(aName), &result,
-                         &success);
+  actor->CallGetParentProperty(static_cast<PPluginIdentifierChild*>(aName),
+                               &result, &success);
 
   if (!success) {
     return false;
   }
 
   ConvertToVariant(result, *aResult);
   return true;
 }
@@ -826,55 +826,59 @@ PluginScriptableObjectChild::AnswerHasPr
   }
 
   PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
   *aHasProperty = mObject->_class->hasProperty(mObject, id->ToNPIdentifier());
   return true;
 }
 
 bool
-PluginScriptableObjectChild::AnswerGetProperty(PPluginIdentifierChild* aId,
-                                               Variant* aResult,
-                                               bool* aSuccess)
+PluginScriptableObjectChild::AnswerGetChildProperty(PPluginIdentifierChild* aId,
+                                                    bool* aHasProperty,
+                                                    bool* aHasMethod,
+                                                    Variant* aResult,
+                                                    bool* aSuccess)
 {
   AssertPluginThread();
 
+  *aHasProperty = *aHasMethod = *aSuccess = false;
+  *aResult = void_t();
+
   if (mInvalidated) {
     NS_WARNING("Calling AnswerGetProperty with an invalidated object!");
-    *aResult = void_t();
-    *aSuccess = false;
     return true;
   }
 
   NS_ASSERTION(mObject->_class != GetClass(), "Bad object type!");
   NS_ASSERTION(mType == LocalObject, "Bad type!");
 
-  if (!(mObject->_class && mObject->_class->getProperty)) {
-    *aResult = void_t();
-    *aSuccess = false;
+  if (!(mObject->_class && mObject->_class->hasProperty &&
+        mObject->_class->hasMethod && mObject->_class->getProperty)) {
     return true;
   }
 
-  NPVariant result;
-  VOID_TO_NPVARIANT(result);
-  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
-  if (!mObject->_class->getProperty(mObject, id->ToNPIdentifier(), &result)) {
-    *aResult = void_t();
-    *aSuccess = false;
-    return true;
-  }
+  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
+
+  *aHasProperty = mObject->_class->hasProperty(mObject, id);
+  *aHasMethod = mObject->_class->hasMethod(mObject, id);
+
+  if (*aHasProperty) {
+    NPVariant result;
+    VOID_TO_NPVARIANT(result);
 
-  Variant converted;
-  if ((*aSuccess = ConvertToRemoteVariant(result, converted, GetInstance(),
-                                          false))) {
-    DeferNPVariantLastRelease(&PluginModuleChild::sBrowserFuncs, &result);
-    *aResult = converted;
-  }
-  else {
-    *aResult = void_t();
+    if (!mObject->_class->getProperty(mObject, id, &result)) {
+      return true;
+    }
+
+    Variant converted;
+    if ((*aSuccess = ConvertToRemoteVariant(result, converted, GetInstance(),
+                                            false))) {
+      DeferNPVariantLastRelease(&PluginModuleChild::sBrowserFuncs, &result);
+      *aResult = converted;
+    }
   }
 
   return true;
 }
 
 bool
 PluginScriptableObjectChild::AnswerSetProperty(PPluginIdentifierChild* aId,
                                                const Variant& aValue,
@@ -886,27 +890,33 @@ PluginScriptableObjectChild::AnswerSetPr
     NS_WARNING("Calling AnswerSetProperty with an invalidated object!");
     *aSuccess = false;
     return true;
   }
 
   NS_ASSERTION(mObject->_class != GetClass(), "Bad object type!");
   NS_ASSERTION(mType == LocalObject, "Bad type!");
 
-  if (!(mObject->_class && mObject->_class->setProperty)) {
+  if (!(mObject->_class && mObject->_class->hasProperty &&
+        mObject->_class->setProperty)) {
+    *aSuccess = false;
+    return true;
+  }
+
+  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
+
+  if (!mObject->_class->hasProperty(mObject, id)) {
     *aSuccess = false;
     return true;
   }
 
   NPVariant converted;
   ConvertToVariant(aValue, converted);
 
-  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
-  if ((*aSuccess = mObject->_class->setProperty(mObject, id->ToNPIdentifier(),
-                                                &converted))) {
+  if ((*aSuccess = mObject->_class->setProperty(mObject, id, &converted))) {
     PluginModuleChild::sBrowserFuncs.releasevariantvalue(&converted);
   }
   return true;
 }
 
 bool
 PluginScriptableObjectChild::AnswerRemoveProperty(PPluginIdentifierChild* aId,
                                                   bool* aSuccess)
@@ -917,23 +927,27 @@ PluginScriptableObjectChild::AnswerRemov
     NS_WARNING("Calling AnswerRemoveProperty with an invalidated object!");
     *aSuccess = false;
     return true;
   }
 
   NS_ASSERTION(mObject->_class != GetClass(), "Bad object type!");
   NS_ASSERTION(mType == LocalObject, "Bad type!");
 
-  if (!(mObject->_class && mObject->_class->removeProperty)) {
+  if (!(mObject->_class && mObject->_class->hasProperty &&
+        mObject->_class->removeProperty)) {
     *aSuccess = false;
     return true;
   }
 
-  PluginIdentifierChild* id = static_cast<PluginIdentifierChild*>(aId);
-  *aSuccess = mObject->_class->removeProperty(mObject, id->ToNPIdentifier());
+  NPIdentifier id = static_cast<PluginIdentifierChild*>(aId)->ToNPIdentifier();
+  *aSuccess = mObject->_class->hasProperty(mObject, id) ?
+              mObject->_class->removeProperty(mObject, id) :
+              true;
+
   return true;
 }
 
 bool
 PluginScriptableObjectChild::AnswerEnumerate(nsTArray<PPluginIdentifierChild*>* aProperties,
                                              bool* aSuccess)
 {
   AssertPluginThread();
--- a/dom/plugins/PluginScriptableObjectChild.h
+++ b/dom/plugins/PluginScriptableObjectChild.h
@@ -102,19 +102,21 @@ public:
                       Variant* aResult,
                       bool* aSuccess);
 
   virtual bool
   AnswerHasProperty(PPluginIdentifierChild* aId,
                     bool* aHasProperty);
 
   virtual bool
-  AnswerGetProperty(PPluginIdentifierChild* aId,
-                    Variant* aResult,
-                    bool* aSuccess);
+  AnswerGetChildProperty(PPluginIdentifierChild* aId,
+                         bool* aHasProperty,
+                         bool* aHasMethod,
+                         Variant* aResult,
+                         bool* aSuccess);
 
   virtual bool
   AnswerSetProperty(PPluginIdentifierChild* aId,
                     const Variant& aValue,
                     bool* aSuccess);
 
   virtual bool
   AnswerRemoveProperty(PPluginIdentifierChild* aId,
--- a/dom/plugins/PluginScriptableObjectParent.cpp
+++ b/dom/plugins/PluginScriptableObjectParent.cpp
@@ -316,56 +316,19 @@ PluginScriptableObjectParent::Scriptable
 }
 
 // static
 bool
 PluginScriptableObjectParent::ScriptableGetProperty(NPObject* aObject,
                                                     NPIdentifier aName,
                                                     NPVariant* aResult)
 {
-  if (aObject->_class != GetClass()) {
-    NS_ERROR("Don't know what kind of object this is!");
-    return false;
-  }
-
-  ParentNPObject* object = reinterpret_cast<ParentNPObject*>(aObject);
-  if (object->invalidated) {
-    NS_WARNING("Calling method on an invalidated object!");
-    return false;
-  }
-
-  PPluginIdentifierParent* identifier = GetIdentifier(aObject, aName);
-  if (!identifier) {
-    return false;
-  }
-
-  ProtectedActor<PluginScriptableObjectParent> actor(object->parent);
-  if (!actor) {
-    return false;
-  }
-
-  NS_ASSERTION(actor->Type() == Proxy, "Bad type!");
-
-  Variant result;
-  bool success;
-  if (!actor->CallGetProperty(identifier, &result, &success)) {
-    NS_WARNING("Failed to send message!");
-    return false;
-  }
-
-  if (!success) {
-    return false;
-  }
-
-  if (!ConvertToVariant(result, *aResult, actor->GetInstance())) {
-    NS_WARNING("Failed to convert result!");
-    return false;
-  }
-
-  return true;
+  // See GetPropertyHelper below.
+  NS_NOTREACHED("Shouldn't ever call this directly!");
+  return false;
 }
 
 // static
 bool
 PluginScriptableObjectParent::ScriptableSetProperty(NPObject* aObject,
                                                     NPIdentifier aName,
                                                     const NPVariant* aValue)
 {
@@ -959,19 +922,20 @@ PluginScriptableObjectParent::AnswerHasP
 
   PluginIdentifierParent* id = static_cast<PluginIdentifierParent*>(aId);
   *aHasProperty = npn->hasproperty(instance->GetNPP(), mObject,
                                    id->ToNPIdentifier());
   return true;
 }
 
 bool
-PluginScriptableObjectParent::AnswerGetProperty(PPluginIdentifierParent* aId,
-                                                Variant* aResult,
-                                                bool* aSuccess)
+PluginScriptableObjectParent::AnswerGetParentProperty(
+                                                   PPluginIdentifierParent* aId,
+                                                   Variant* aResult,
+                                                   bool* aSuccess)
 {
   if (!mObject) {
     NS_WARNING("Calling AnswerGetProperty with an invalidated object!");
     *aResult = void_t();
     *aSuccess = false;
     return true;
   }
 
@@ -1282,8 +1246,48 @@ PluginScriptableObjectParent::AnswerNPN_
     *aSuccess = false;
     return true;
   }
 
   *aSuccess = true;
   *aResult = convertedResult;
   return true;
 }
+
+JSBool
+PluginScriptableObjectParent::GetPropertyHelper(NPIdentifier aName,
+                                                PRBool* aHasProperty,
+                                                PRBool* aHasMethod,
+                                                NPVariant* aResult)
+{
+  NS_ASSERTION(Type() == Proxy, "Bad type!");
+
+  ParentNPObject* object = static_cast<ParentNPObject*>(mObject);
+  if (object->invalidated) {
+    NS_WARNING("Calling method on an invalidated object!");
+    return JS_FALSE;
+  }
+
+  PPluginIdentifierParent* identifier = GetIdentifier(GetInstance(), aName);
+  if (!identifier) {
+    return JS_FALSE;
+  }
+
+  bool hasProperty, hasMethod, success;
+  Variant result;
+  if (!CallGetChildProperty(identifier, &hasProperty, &hasMethod, &result,
+                            &success)) {
+    return JS_FALSE;
+  }
+
+  if (!success) {
+    return JS_FALSE;
+  }
+
+  if (!ConvertToVariant(result, *aResult, GetInstance())) {
+    NS_WARNING("Failed to convert result!");
+    return JS_FALSE;
+  }
+
+  *aHasProperty = hasProperty;
+  *aHasMethod = hasMethod;
+  return JS_TRUE;
+}
--- a/dom/plugins/PluginScriptableObjectParent.h
+++ b/dom/plugins/PluginScriptableObjectParent.h
@@ -36,16 +36,17 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef dom_plugins_PluginScriptableObjectParent_h
 #define dom_plugins_PluginScriptableObjectParent_h 1
 
 #include "mozilla/plugins/PPluginScriptableObjectParent.h"
 
+#include "jsapi.h"
 #include "npfunctions.h"
 #include "npruntime.h"
 
 namespace mozilla {
 namespace plugins {
 
 class PluginInstanceParent;
 class PluginScriptableObjectParent;
@@ -91,19 +92,19 @@ public:
                       Variant* aResult,
                       bool* aSuccess);
 
   virtual bool
   AnswerHasProperty(PPluginIdentifierParent* aId,
                     bool* aHasProperty);
 
   virtual bool
-  AnswerGetProperty(PPluginIdentifierParent* aId,
-                    Variant* aResult,
-                    bool* aSuccess);
+  AnswerGetParentProperty(PPluginIdentifierParent* aId,
+                          Variant* aResult,
+                          bool* aSuccess);
 
   virtual bool
   AnswerSetProperty(PPluginIdentifierParent* aId,
                     const Variant& aValue,
                     bool* aSuccess);
 
   virtual bool
   AnswerRemoveProperty(PPluginIdentifierParent* aId,
@@ -163,16 +164,21 @@ public:
   // ResurrectProxyObject).
   void DropNPObject();
 
   ScriptableObjectType
   Type() const {
     return mType;
   }
 
+  JSBool GetPropertyHelper(NPIdentifier aName,
+                           PRBool* aHasProperty,
+                           PRBool* aHasMethod,
+                           NPVariant* aResult);
+
 private:
   static NPObject*
   ScriptableAllocate(NPP aInstance,
                      NPClass* aClass);
 
   static void
   ScriptableInvalidate(NPObject* aObject);
 
--- a/modules/plugin/base/src/nsJSNPRuntime.cpp
+++ b/modules/plugin/base/src/nsJSNPRuntime.cpp
@@ -31,16 +31,20 @@
  * 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 ***** */
 
+#ifdef MOZ_IPC
+#include "base/basictypes.h"
+#endif
+
 // FIXME(bug 332648): Give me a real API please!
 #include "jscntxt.h"
 
 #include "nsJSNPRuntime.h"
 #include "nsNPAPIPlugin.h"
 #include "nsNPAPIPluginInstance.h"
 #include "nsIScriptGlobalObject.h"
 #include "nsIScriptContext.h"
@@ -50,16 +54,22 @@
 #include "nsIJSContextStack.h"
 #include "nsIXPConnect.h"
 #include "nsIDOMElement.h"
 #include "prmem.h"
 #include "nsIContent.h"
 
 using namespace mozilla::plugins::parent;
 
+#ifdef MOZ_IPC
+#include "mozilla/plugins/PluginScriptableObjectParent.h"
+using mozilla::plugins::PluginScriptableObjectParent;
+using mozilla::plugins::ParentNPObject;
+#endif
+
 // Hash of JSObject wrappers that wraps JSObjects as NPObjects. There
 // will be one wrapper per JSObject per plugin instance, i.e. if two
 // plugins access the JSObject x, two wrappers for x will be
 // created. This is needed to be able to properly drop the wrappers
 // when a plugin is torn down in case there's a leak in the plugin (we
 // don't want to leak the world just because a plugin leaks an
 // NPObject).
 static PLDHashTable sJSObjWrappers;
@@ -77,16 +87,30 @@ static PRInt32 sWrapperCount;
 static JSRuntime *sJSRuntime;
 
 // The JS context stack, we use this to push a plugin's JSContext onto
 // while executing JS on the context.
 static nsIJSContextStack *sContextStack;
 
 static nsTArray<NPObject*>* sDelayedReleases;
 
+namespace {
+
+inline bool
+NPObjectIsOutOfProcessProxy(NPObject *obj)
+{
+#ifdef MOZ_IPC
+  return obj->_class == PluginScriptableObjectParent::GetClass();
+#else
+  return false;
+#endif
+}
+
+} // anonymous namespace
+
 // Helper class that reports any JS exceptions that were thrown while
 // the plugin executed JS.
 
 class AutoJSExceptionReporter
 {
 public:
   AutoJSExceptionReporter(JSContext *cx)
     : mCx(cx)
@@ -150,18 +174,18 @@ static JSBool
 NPObjWrapper_Call(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
                   jsval *rval);
 
 static JSBool
 NPObjWrapper_Construct(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
                        jsval *rval);
 
 static JSBool
-CreateNPObjectMember(NPP npp, JSContext *cx, JSObject *obj,
-                     NPObject *npobj, jsval id, jsval *vp);
+CreateNPObjectMember(NPP npp, JSContext *cx, JSObject *obj, NPObject *npobj,
+                     jsval id, NPVariant* getPropertyResult, jsval *vp);
 
 static JSClass sNPObjectJSWrapperClass =
   {
     NPRUNTIME_JSCLASS_NAME,
     JSCLASS_HAS_PRIVATE | JSCLASS_NEW_RESOLVE | JSCLASS_NEW_ENUMERATE,
     NPObjWrapper_AddProperty, NPObjWrapper_DelProperty,
     NPObjWrapper_GetProperty, NPObjWrapper_SetProperty,
     (JSEnumerateOp)NPObjWrapper_newEnumerate,
@@ -1177,16 +1201,20 @@ NPObjWrapper_AddProperty(JSContext *cx, 
 
   if (!npobj || !npobj->_class || !npobj->_class->hasProperty ||
       !npobj->_class->hasMethod) {
     ThrowJSException(cx, "Bad NPObject as private data!");
 
     return JS_FALSE;
   }
 
+  if (NPObjectIsOutOfProcessProxy(npobj)) {
+    return JS_TRUE;
+  }
+
   PluginDestructionGuard pdg(LookupNPP(npobj));
 
   JSBool hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
   if (!ReportExceptionIfPending(cx))
     return JS_FALSE;
 
   if (hasProperty)
     return JS_TRUE;
@@ -1215,22 +1243,24 @@ NPObjWrapper_DelProperty(JSContext *cx, 
       !npobj->_class->removeProperty) {
     ThrowJSException(cx, "Bad NPObject as private data!");
 
     return JS_FALSE;
   }
 
   PluginDestructionGuard pdg(LookupNPP(npobj));
 
-  JSBool hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
-  if (!ReportExceptionIfPending(cx))
-    return JS_FALSE;
-
-  if (!hasProperty)
-    return JS_TRUE;
+  if (!NPObjectIsOutOfProcessProxy(npobj)) {
+    JSBool hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
+    if (!ReportExceptionIfPending(cx))
+      return JS_FALSE;
+
+    if (!hasProperty)
+      return JS_TRUE;
+  }
 
   if (!npobj->_class->removeProperty(npobj, (NPIdentifier)id))
     *vp = JSVAL_FALSE;
 
   return ReportExceptionIfPending(cx);
 }
 
 static JSBool
@@ -1252,24 +1282,26 @@ NPObjWrapper_SetProperty(JSContext *cx, 
   if (!npp) {
     ThrowJSException(cx, "No NPP found for NPObject!");
 
     return JS_FALSE;
   }
 
   PluginDestructionGuard pdg(npp);
 
-  JSBool hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
-  if (!ReportExceptionIfPending(cx))
-    return JS_FALSE;
-
-  if (!hasProperty) {
-    ThrowJSException(cx, "Trying to set unsupported property on NPObject!");
-
-    return JS_FALSE;
+  if (!NPObjectIsOutOfProcessProxy(npobj)) {
+    JSBool hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
+    if (!ReportExceptionIfPending(cx))
+      return JS_FALSE;
+
+    if (!hasProperty) {
+      ThrowJSException(cx, "Trying to set unsupported property on NPObject!");
+
+      return JS_FALSE;
+    }
   }
 
   NPVariant npv;
   if (!JSValToNPVariant(npp, cx, *vp, &npv)) {
     ThrowJSException(cx, "Error converting jsval to NPVariant!");
 
     return JS_FALSE;
   }
@@ -1306,32 +1338,63 @@ NPObjWrapper_GetProperty(JSContext *cx, 
   if (!npp) {
     ThrowJSException(cx, "No NPP found for NPObject!");
 
     return JS_FALSE;
   }
 
   PluginDestructionGuard pdg(npp);
 
-  PRBool hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
+  PRBool hasProperty, hasMethod;
+
+  NPVariant npv;
+  VOID_TO_NPVARIANT(npv);
+
+#ifdef MOZ_IPC
+  if (NPObjectIsOutOfProcessProxy(npobj)) {
+    PluginScriptableObjectParent* actor =
+      static_cast<ParentNPObject*>(npobj)->parent;
+    JSBool success = actor->GetPropertyHelper((NPIdentifier)id, &hasProperty,
+                                              &hasMethod, &npv);
+    if (!ReportExceptionIfPending(cx)) {
+      if (success)
+        _releasevariantvalue(&npv);
+      return JS_FALSE;
+    }
+
+    if (success) {
+      // We return NPObject Member class here to support ambiguous members.
+      if (hasProperty && hasMethod)
+        return CreateNPObjectMember(npp, cx, obj, npobj, id, &npv, vp);
+
+      if (hasProperty) {
+        *vp = NPVariantToJSVal(npp, cx, &npv);
+        _releasevariantvalue(&npv);
+
+        if (!ReportExceptionIfPending(cx))
+          return JS_FALSE;
+      }
+    }
+    return JS_TRUE;
+  }
+#endif
+
+  hasProperty = npobj->_class->hasProperty(npobj, (NPIdentifier)id);
   if (!ReportExceptionIfPending(cx))
     return JS_FALSE;
 
-  PRBool hasMethod = npobj->_class->hasMethod(npobj, (NPIdentifier)id);
+  hasMethod = npobj->_class->hasMethod(npobj, (NPIdentifier)id);
   if (!ReportExceptionIfPending(cx))
     return JS_FALSE;
 
   // We return NPObject Member class here to support ambiguous members.
   if (hasProperty && hasMethod)
-    return CreateNPObjectMember(npp, cx, obj, npobj, id, vp);
+    return CreateNPObjectMember(npp, cx, obj, npobj, id, nsnull, vp);
 
   if (hasProperty) {
-    NPVariant npv;
-    VOID_TO_NPVARIANT(npv);
-
     if (npobj->_class->getProperty(npobj, (NPIdentifier)id, &npv))
       *vp = NPVariantToJSVal(npp, cx, &npv);
 
     _releasevariantvalue(&npv);
 
     if (!ReportExceptionIfPending(cx))
       return JS_FALSE;
   }
@@ -2043,18 +2106,18 @@ LookupNPP(NPObject *npobj)
   }
 
   NS_ASSERTION(entry->mNpp, "Live NPObject entry w/o an NPP!");
 
   return entry->mNpp;
 }
 
 JSBool
-CreateNPObjectMember(NPP npp, JSContext *cx, JSObject *obj,
-                     NPObject* npobj, jsval id, jsval *vp)
+CreateNPObjectMember(NPP npp, JSContext *cx, JSObject *obj, NPObject* npobj,
+                     jsval id,  NPVariant* getPropertyResult, jsval *vp)
 {
   NS_ENSURE_TRUE(vp, JS_FALSE);
 
   if (!npobj || !npobj->_class || !npobj->_class->getProperty ||
       !npobj->_class->invoke) {
     ThrowJSException(cx, "Bad NPObject");
 
     return JS_FALSE;
@@ -2077,28 +2140,37 @@ CreateNPObjectMember(NPP npp, JSContext 
 
   *vp = OBJECT_TO_JSVAL(memobj);
   ::JS_AddRoot(cx, vp);
 
   ::JS_SetPrivate(cx, memobj, (void *)memberPrivate);
 
   jsval fieldValue;
   NPVariant npv;
-  VOID_TO_NPVARIANT(npv);
-
-  NPBool hasProperty = npobj->_class->getProperty(npobj, (NPIdentifier)id,
-                                                  &npv);
-  if (ReportExceptionIfPending(cx)) {
-    ::JS_RemoveRoot(cx, vp);
-    return JS_FALSE;
+  NPBool hasProperty;
+
+  if (getPropertyResult) {
+    // Plugin has already handed us the value we want here.
+    npv = *getPropertyResult;
+    hasProperty = true;
   }
-
-  if (!hasProperty) {
-    ::JS_RemoveRoot(cx, vp);
-    return JS_FALSE;
+  else {
+    VOID_TO_NPVARIANT(npv);
+
+    NPBool hasProperty = npobj->_class->getProperty(npobj, (NPIdentifier)id,
+                                                    &npv);
+    if (ReportExceptionIfPending(cx)) {
+      ::JS_RemoveRoot(cx, vp);
+      return JS_FALSE;
+    }
+
+    if (!hasProperty) {
+      ::JS_RemoveRoot(cx, vp);
+      return JS_FALSE;
+    }
   }
 
   fieldValue = NPVariantToJSVal(npp, cx, &npv);
 
   // npobjWrapper is the JSObject through which we make sure we don't
   // outlive the underlying NPObject, so make sure it points to the
   // real JSObject wrapper for the NPObject.
   while (JS_GET_CLASS(cx, obj) != &sNPObjectJSWrapperClass) {
--- a/modules/plugin/test/mochitest/Makefile.in
+++ b/modules/plugin/test/mochitest/Makefile.in
@@ -80,16 +80,17 @@ include $(topsrcdir)/config/rules.mk
   test_streamatclose.html \
   neverending.sjs \
   test_newstreamondestroy.html \
   test_crashing.html \
   test_crashing2.html \
   test_hanging.html \
   crashing_subpage.html \
   test_GCrace.html \
+  test_propertyAndMethod.html \
   $(NULL)
 
 #  test_npruntime_npnsetexception.html \ Disabled for e10s
 
 ifeq ($(OS_ARCH),WINNT)
 _MOCHITEST_FILES += \
   test_windowed_invalidate.html \
   $(NULL)
new file mode 100644
--- /dev/null
+++ b/modules/plugin/test/mochitest/test_propertyAndMethod.html
@@ -0,0 +1,48 @@
+<head>
+  <title>NPObject with property and method with the same name</title>
+
+  <script type="text/javascript" src="/MochiKit/packed.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+
+<body onload="run()">
+
+  <embed id="plugin" type="application/x-test" wmode="window"></embed>
+
+  <script class="testbody" type="application/javascript">
+
+    if (typeof Object.getPrototypeOf !== "function") {
+      if (typeof "test".__proto__ === "object") {
+        Object.getPrototypeOf = function(object) {
+          return object.__proto__;
+        };
+      } else {
+        Object.getPrototypeOf = function(object) {
+          // May break if the constructor has been tampered with
+          return object.constructor.prototype;
+        };
+      }
+    }
+
+    SimpleTest.waitForExplicitFinish();
+
+    function run() {
+      var plugin = document.getElementById("plugin");
+      var pluginProto = Object.getPrototypeOf(plugin);
+      delete pluginProto.propertyAndMethod;
+      ok(isNaN(plugin.propertyAndMethod + 0), "Shouldn't be set yet!");
+
+      plugin.propertyAndMethod = 5;
+      is(plugin.propertyAndMethod, 5, "Should be set to 5!");
+
+      delete pluginProto.propertyAndMethod;
+      ok(isNaN(plugin.propertyAndMethod + 0), "Shouldn't be set any more!");
+
+      var res = plugin.propertyAndMethod();
+      is(res, 5, "Method invocation should return 5!");
+
+      SimpleTest.finish();
+    }
+  </script>
+</body>
--- a/modules/plugin/test/testplugin/nptest.cpp
+++ b/modules/plugin/test/testplugin/nptest.cpp
@@ -152,16 +152,17 @@ static bool setCookie(NPObject* npobj, c
 static bool getCookie(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool getAuthInfo(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool asyncCallbackTest(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool checkGCRace(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool hangPlugin(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool getClipboardText(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool callOnDestroy(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 static bool reinitWidget(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
+static bool propertyAndMethod(NPObject* npobj, const NPVariant* args, uint32_t argCount, NPVariant* result);
 
 static const NPUTF8* sPluginMethodIdentifierNames[] = {
   "npnEvaluateTest",
   "npnInvokeTest",
   "npnInvokeDefaultTest",
   "setUndefinedValueTest",
   "identifierToStringTest",
   "timerTest",
@@ -194,16 +195,17 @@ static const NPUTF8* sPluginMethodIdenti
   "getCookie",
   "getAuthInfo",
   "asyncCallbackTest",
   "checkGCRace",
   "hang",
   "getClipboardText",
   "callOnDestroy",
   "reinitWidget",
+  "propertyAndMethod"
 };
 static NPIdentifier sPluginMethodIdentifiers[ARRAY_LENGTH(sPluginMethodIdentifierNames)];
 static const ScriptableFunction sPluginMethodFunctions[ARRAY_LENGTH(sPluginMethodIdentifierNames)] = {
   npnEvaluateTest,
   npnInvokeTest,
   npnInvokeDefaultTest,
   setUndefinedValueTest,
   identifierToStringTest,
@@ -237,17 +239,23 @@ static const ScriptableFunction sPluginM
   getCookie,
   getAuthInfo,
   asyncCallbackTest,
   checkGCRace,
   hangPlugin,
   getClipboardText,
   callOnDestroy,
   reinitWidget,
+  propertyAndMethod
 };
+static const NPUTF8* sPluginPropertyIdentifierNames[] = {
+  "propertyAndMethod"
+};
+static NPIdentifier sPluginPropertyIdentifiers[ARRAY_LENGTH(sPluginPropertyIdentifierNames)];
+static NPVariant sPluginPropertyValues[ARRAY_LENGTH(sPluginPropertyIdentifierNames)];
 
 struct URLNotifyData
 {
   const char* cookie;
   NPObject* callback;
   uint32_t size;
   char* data;
 };
@@ -302,29 +310,35 @@ static int32_t sInstanceCount = 0;
  */
 static bool sWatchingInstanceCount = false;
 
 static void initializeIdentifiers()
 {
   if (!sIdentifiersInitialized) {
     NPN_GetStringIdentifiers(sPluginMethodIdentifierNames,
         ARRAY_LENGTH(sPluginMethodIdentifierNames), sPluginMethodIdentifiers);
+    NPN_GetStringIdentifiers(sPluginPropertyIdentifierNames,
+        ARRAY_LENGTH(sPluginPropertyIdentifierNames), sPluginPropertyIdentifiers);
+
     sIdentifiersInitialized = true;    
 
     // Check whether NULL is handled in NPN_GetStringIdentifiers
     NPIdentifier IDList[2];
     static char const *const kIDNames[2] = { NULL, "setCookie" };
     NPN_GetStringIdentifiers(const_cast<const NPUTF8**>(kIDNames), 2, IDList);
   }
 }
 
 static void clearIdentifiers()
 {
   memset(sPluginMethodIdentifiers, 0,
       ARRAY_LENGTH(sPluginMethodIdentifiers) * sizeof(NPIdentifier));
+  memset(sPluginPropertyIdentifiers, 0,
+      ARRAY_LENGTH(sPluginPropertyIdentifiers) * sizeof(NPIdentifier));
+
   sIdentifiersInitialized = false;
 }
 
 static void addRange(InstanceData* instanceData, const char* range)
 {
   char rangestr[16];
   strncpy(rangestr, range, sizeof(rangestr));
   const char* str1 = strtok(rangestr, ",");
@@ -441,16 +455,35 @@ getFuncFromString(const char* funcname)
   int32_t i = 0;
   while(funcTable[i].funcName) {
     if (!strcmp(funcname, funcTable[i].funcName)) return funcTable[i].funcId;
     i++;
   }
   return FUNCTION_NONE;
 }
 
+static void
+DuplicateNPVariant(NPVariant& aDest, const NPVariant& aSrc)
+{
+  if (NPVARIANT_IS_STRING(aSrc)) {
+    NPString src = NPVARIANT_TO_STRING(aSrc);
+    char* buf = new char[src.UTF8Length];
+    strncpy(buf, src.UTF8Characters, src.UTF8Length);
+    STRINGN_TO_NPVARIANT(buf, src.UTF8Length, aDest);
+  }
+  else if (NPVARIANT_IS_OBJECT(aSrc)) {
+    NPObject* obj =
+      NPN_RetainObject(NPVARIANT_TO_OBJECT(aSrc));
+    OBJECT_TO_NPVARIANT(obj, aDest);
+  }
+  else {
+    aDest = aSrc;
+  }
+}
+
 //
 // function signatures
 //
 
 NPObject* scriptableAllocate(NPP npp, NPClass* aClass);
 void scriptableDeallocate(NPObject* npobj);
 void scriptableInvalidate(NPObject* npobj);
 bool scriptableHasMethod(NPObject* npobj, NPIdentifier name);
@@ -528,16 +561,20 @@ NPError OSCALL NP_Initialize(NPNetscapeF
 #elif defined(XP_UNIX)
 NP_EXPORT(NPError) NP_Initialize(NPNetscapeFuncs* bFuncs, NPPluginFuncs* pFuncs)
 #endif
 {
   sBrowserFuncs = bFuncs;
 
   initializeIdentifiers();
 
+  for (int i = 0; i < ARRAY_LENGTH(sPluginPropertyValues); i++) {
+    VOID_TO_NPVARIANT(sPluginPropertyValues[i]);
+  }
+
   memset(&sNPClass, 0, sizeof(NPClass));
   sNPClass.structVersion =  NP_CLASS_STRUCT_VERSION;
   sNPClass.allocate =       (NPAllocateFunctionPtr)scriptableAllocate;
   sNPClass.deallocate =     (NPDeallocateFunctionPtr)scriptableDeallocate;
   sNPClass.invalidate =     (NPInvalidateFunctionPtr)scriptableInvalidate;
   sNPClass.hasMethod =      (NPHasMethodFunctionPtr)scriptableHasMethod;
   sNPClass.invoke =         (NPInvokeFunctionPtr)scriptableInvoke;
   sNPClass.invokeDefault =  (NPInvokeDefaultFunctionPtr)scriptableInvokeDefault;
@@ -570,16 +607,20 @@ NPError OSCALL NP_GetEntryPoints(NPPlugi
 #if defined(XP_UNIX)
 NP_EXPORT(NPError) NP_Shutdown()
 #elif defined(XP_WIN) || defined(XP_OS2)
 NPError OSCALL NP_Shutdown()
 #endif
 {
   clearIdentifiers();
 
+  for (int i = 0; i < ARRAY_LENGTH(sPluginPropertyValues); i++) {
+    NPN_ReleaseVariantValue(&sPluginPropertyValues[i]);
+  }
+
   return NPERR_NO_ERROR;
 }
 
 NPError
 NPP_New(NPMIMEType pluginType, NPP instance, uint16_t mode, int16_t argc, char* argn[], char* argv[], NPSavedData* saved)
 {
   // Make sure our pdata field is NULL at this point. If it isn't, that
   // probably means the browser gave us uninitialized memory.
@@ -1539,34 +1580,57 @@ scriptableInvokeDefault(NPObject* npobj,
   }
   STRINGZ_TO_NPVARIANT(strdup(value.str().c_str()), *result);
   return true;
 }
 
 bool
 scriptableHasProperty(NPObject* npobj, NPIdentifier name)
 {
+  for (int i = 0; i < int(ARRAY_LENGTH(sPluginPropertyIdentifiers)); i++) {
+    if (name == sPluginPropertyIdentifiers[i])
+      return true;
+  }
   return false;
 }
 
 bool
 scriptableGetProperty(NPObject* npobj, NPIdentifier name, NPVariant* result)
 {
+  for (int i = 0; i < int(ARRAY_LENGTH(sPluginPropertyIdentifiers)); i++) {
+    if (name == sPluginPropertyIdentifiers[i]) {
+      DuplicateNPVariant(*result, sPluginPropertyValues[i]);
+      return true;
+    }
+  }
   return false;
 }
 
 bool
 scriptableSetProperty(NPObject* npobj, NPIdentifier name, const NPVariant* value)
 {
+  for (int i = 0; i < int(ARRAY_LENGTH(sPluginPropertyIdentifiers)); i++) {
+    if (name == sPluginPropertyIdentifiers[i]) {
+      NPN_ReleaseVariantValue(&sPluginPropertyValues[i]);
+      DuplicateNPVariant(sPluginPropertyValues[i], *value);
+      return true;
+    }
+  }
   return false;
 }
 
 bool
 scriptableRemoveProperty(NPObject* npobj, NPIdentifier name)
 {
+  for (int i = 0; i < int(ARRAY_LENGTH(sPluginPropertyIdentifiers)); i++) {
+    if (name == sPluginPropertyIdentifiers[i]) {
+      NPN_ReleaseVariantValue(&sPluginPropertyValues[i]);
+      return true;
+    }
+  }
   return false;
 }
 
 bool
 scriptableEnumerate(NPObject* npobj, NPIdentifier** identifier, uint32_t* count)
 {
   return false;
 }
@@ -2682,8 +2746,16 @@ reinitWidget(NPObject* npobj, const NPVa
   InstanceData* id = static_cast<InstanceData*>(npp->pdata);
 
   if (!id->hasWidget)
     return false;
 
   pluginWidgetInit(id, id->window.window);
   return true;
 }
+
+bool
+propertyAndMethod(NPObject* npobj, const NPVariant* args, uint32_t argCount,
+                  NPVariant* result)
+{
+  INT32_TO_NPVARIANT(5, *result);
+  return true;
+}