Bug 484180 - Don't call the class getter (especially not the scriptable helper!) for functions. r+sr=jst
authorBlake Kaplan <mrbkap@gmail.com>
Fri, 17 Apr 2009 16:52:10 -0700
changeset 27444 63376e31726ba8c9981c9d07a16dea9172ceaa8a
parent 27443 4e27daf94e4f7efea846742772da429df4b12643
child 27445 a09f2e4f26cf8d8bdf8fd65bb6067b42613f4672
push id6565
push usermrbkap@mozilla.com
push dateFri, 17 Apr 2009 23:53:54 +0000
treeherdermozilla-central@a09f2e4f26cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs484180
milestone1.9.2a1pre
Bug 484180 - Don't call the class getter (especially not the scriptable helper!) for functions. r+sr=jst
dom/tests/mochitest/localstorage/test_localStorageFromChrome.xhtml
js/src/xpconnect/src/XPCWrapper.cpp
--- a/dom/tests/mochitest/localstorage/test_localStorageFromChrome.xhtml
+++ b/dom/tests/mochitest/localstorage/test_localStorageFromChrome.xhtml
@@ -17,22 +17,22 @@ function startTest()
     .getService(Components.interfaces.nsIScriptSecurityManager);
   var dsm = Components.classes["@mozilla.org/dom/storagemanager;1"]
     .getService(Components.interfaces.nsIDOMStorageManager);
 
   var uri = ios.newURI(url, "", null);
   var principal = ssm.getCodebasePrincipal(uri);
   var storage = dsm.getLocalStorageForPrincipal(principal);
   
-  storage.wrappedJSObject.setItem("chromekey", "chromevalue");
+  storage.setItem("chromekey", "chromevalue");
   
   var aframe = document.getElementById("aframe");
   aframe.onload = function()
   {
-    is(storage.wrappedJSObject.getItem("chromekey"), "chromevalue");
+    is(storage.getItem("chromekey"), "chromevalue");
     is(aframe.contentDocument.getElementById("data").innerHTML, "chromevalue");
     SimpleTest.finish();
   }
   aframe.src = "http://example.com/tests/dom/tests/mochitest/localstorage/frameChromeSlave.html";  
 }
 
 SimpleTest.waitForExplicitFinish();
 
--- a/js/src/xpconnect/src/XPCWrapper.cpp
+++ b/js/src/xpconnect/src/XPCWrapper.cpp
@@ -444,16 +444,18 @@ XPCWrapper::ResolveNativeProperty(JSCont
   if (!str) {
     return ThrowException(NS_ERROR_UNEXPECTED, cx);
   }
 
   // Get (and perhaps lazily create) the member's value (commonly a
   // cloneable function).
   jsval v;
   uintN attrs = JSPROP_ENUMERATE;
+  JSPropertyOp getter = nsnull;
+  JSPropertyOp setter = nsnull;
 
   if (member->IsConstant()) {
     if (!member->GetConstantValue(ccx, iface, &v)) {
       return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
     }
   } else if (member->IsAttribute()) {
     // An attribute is being resolved. Define the property, the value
     // will be dealt with in the get/set hooks.  Use JSPROP_SHARED to
@@ -480,16 +482,23 @@ XPCWrapper::ResolveNativeProperty(JSCont
            ::JS_GetStringBytes(JSVAL_TO_STRING(id)));
 #endif
 
     if (!WrapFunction(cx, wrapperObj, JSVAL_TO_OBJECT(funval), &v,
                       isNativeWrapper)) {
       return JS_FALSE;
     }
 
+    // Functions shouldn't have a getter or a setter. Without the wrappers,
+    // they would live on the prototype (and call its getter), since we don't
+    // have a prototype, and we need to avoid calling the scriptable helper's
+    // GetProperty method for this property, stub out the getters and setters
+    // explicitly.
+    getter = setter = JS_PropertyStub;
+
     // Since the XPC_*_NewResolve functions ensure that the method's property
     // name is accessible, we set the eAllAccessSlot bit, which indicates to
     // XPC_NW_FunctionWrapper that the method is safe to unwrap and call, even
     // if XPCNativeWrapper::GetWrappedNative disagrees.
     JS_SetReservedSlot(cx, JSVAL_TO_OBJECT(v), eAllAccessSlot, JSVAL_TRUE);
   }
 
   // Make sure v doesn't go away while we mess with it.
@@ -499,17 +508,17 @@ XPCWrapper::ResolveNativeProperty(JSCont
   jsval oldFlags;
   if (!isNativeWrapper &&
       (!::JS_GetReservedSlot(cx, wrapperObj, sResolvingSlot, &oldFlags) ||
        !::JS_SetReservedSlot(cx, wrapperObj, sResolvingSlot, JSVAL_TRUE))) {
     return JS_FALSE;
   }
 
   if (!::JS_DefineUCProperty(cx, wrapperObj, ::JS_GetStringChars(str),
-                            ::JS_GetStringLength(str), v, nsnull, nsnull,
+                            ::JS_GetStringLength(str), v, getter, setter,
                             attrs)) {
     return JS_FALSE;
   }
 
   if (!isNativeWrapper &&
       !::JS_SetReservedSlot(cx, wrapperObj, sResolvingSlot, oldFlags)) {
     return JS_FALSE;
   }