Bug 838146 part 9. Make Xrays play nice with WebIDL resolve hooks. r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 16 Jul 2013 01:31:03 -0400
changeset 138996 0249e847f1c453bc8527786a24c7bdbdc2a94660
parent 138995 325c7efdbcf5b62100428e9bfa35e1d0de9d183d
child 138997 90c9e64a03dcdfd79b4aca29e1d63bc8f1b7eb1a
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersbholley
bugs838146
milestone25.0a1
Bug 838146 part 9. Make Xrays play nice with WebIDL resolve hooks. r=bholley
content/base/src/nsObjectLoadingContent.cpp
content/base/src/nsObjectLoadingContent.h
dom/base/Navigator.cpp
dom/base/Navigator.h
dom/base/nsDOMClassInfo.cpp
dom/bindings/Codegen.py
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -3345,18 +3345,18 @@ nsObjectLoadingContent::TeardownProtoCha
     }
 
     obj = proto;
   }
 }
 
 bool
 nsObjectLoadingContent::DoNewResolve(JSContext* aCx, JS::Handle<JSObject*> aObject,
-                                     JS::Handle<jsid> aId, unsigned aFlags,
-                                     JS::MutableHandle<JSObject*> aObjp)
+                                     JS::Handle<jsid> aId,
+                                     JS::MutableHandle<JS::Value> aValue)
 {
   // We don't resolve anything; we just try to make sure we're instantiated
 
   nsRefPtr<nsNPAPIPluginInstance> pi;
   nsresult rv = ScriptRequestPluginInstance(aCx, getter_AddRefs(pi));
   if (NS_FAILED(rv)) {
     return mozilla::dom::Throw<true>(aCx, rv);
   }
--- a/content/base/src/nsObjectLoadingContent.h
+++ b/content/base/src/nsObjectLoadingContent.h
@@ -143,18 +143,19 @@ class nsObjectLoadingContent : public ns
      */
     // Helper for WebIDL node wrapping
     void SetupProtoChain(JSContext* aCx, JS::Handle<JSObject*> aObject);
 
     // Remove plugin from protochain
     void TeardownProtoChain();
 
     // Helper for WebIDL newResolve
-    bool DoNewResolve(JSContext* aCx, JS::Handle<JSObject*> aObject, JS::Handle<jsid> aId,
-                      unsigned aFlags, JS::MutableHandle<JSObject*> aObjp);
+    bool DoNewResolve(JSContext* aCx, JS::Handle<JSObject*> aObject,
+                      JS::Handle<jsid> aId,
+                      JS::MutableHandle<JS::Value> aValue);
 
     // WebIDL API
     nsIDocument* GetContentDocument();
     void GetActualType(nsAString& aType) const
     {
       CopyUTF8toUTF16(mContentType, aType);
     }
     uint32_t DisplayedType() const
--- a/dom/base/Navigator.cpp
+++ b/dom/base/Navigator.cpp
@@ -2063,18 +2063,18 @@ Navigator::GetMozAudioChannelManager(Err
   }
 
   return mAudioChannelManager;
 }
 #endif
 
 bool
 Navigator::DoNewResolve(JSContext* aCx, JS::Handle<JSObject*> aObject,
-                        JS::Handle<jsid> aId, unsigned aFlags,
-                        JS::MutableHandle<JSObject*> aObjp)
+                        JS::Handle<jsid> aId,
+                        JS::MutableHandle<JS::Value> aValue)
 {
   if (!JSID_IS_STRING(aId)) {
     return true;
   }
 
   nsScriptNameSpaceManager *nameSpaceManager =
     nsJSRuntime::GetNameSpaceManager();
   if (!nameSpaceManager) {
@@ -2113,25 +2113,21 @@ Navigator::DoNewResolve(JSContext* aCx, 
       }
 
       domObject = construct(aCx, naviObj);
       if (!domObject) {
         return Throw<true>(aCx, NS_ERROR_FAILURE);
       }
     }
 
-    if (!JS_WrapObject(aCx, domObject.address()) ||
-        !JS_DefinePropertyById(aCx, aObject, aId,
-                               JS::ObjectValue(*domObject),
-                               nullptr, nullptr, JSPROP_ENUMERATE)) {
+    if (!JS_WrapObject(aCx, domObject.address())) {
       return false;
     }
 
-    aObjp.set(aObject);
-
+    aValue.setObject(*domObject);
     return true;
   }
 
   NS_ASSERTION(name_struct->mType == nsGlobalNameStruct::eTypeNavigatorProperty,
                "unexpected type");
 
   nsresult rv = NS_OK;
 
@@ -2164,23 +2160,18 @@ Navigator::DoNewResolve(JSContext* aCx, 
       return Throw<true>(aCx, rv);
     }
   }
 
   if (!JS_WrapValue(aCx, prop_val.address())) {
     return Throw<true>(aCx, NS_ERROR_UNEXPECTED);
   }
 
-  JSBool ok = ::JS_DefinePropertyById(aCx, aObject, aId, prop_val,
-                                      JS_PropertyStub, JS_StrictPropertyStub,
-                                      JSPROP_ENUMERATE);
-
-  aObjp.set(aObject);
-
-  return ok;
+  aValue.set(prop_val);
+  return true;
 }
 
 /* static */
 bool
 Navigator::HasBatterySupport(JSContext* /* unused*/, JSObject* /*unused */)
 {
   return battery::BatteryManager::HasSupport();
 }
--- a/dom/base/Navigator.h
+++ b/dom/base/Navigator.h
@@ -345,18 +345,17 @@ public:
   void MozGetUserMediaDevices(MozGetUserMediaDevicesSuccessCallback* aOnSuccess,
                               MozDOMGetUserMediaErrorCallback* aOnError,
                               ErrorResult& aRv);
   void MozGetUserMediaDevices(nsIGetUserMediaDevicesSuccessCallback* aOnSuccess,
                               nsIDOMGetUserMediaErrorCallback* aOnError,
                               ErrorResult& aRv);
 #endif // MOZ_MEDIA_NAVIGATOR
   bool DoNewResolve(JSContext* aCx, JS::Handle<JSObject*> aObject,
-                    JS::Handle<jsid> aId, unsigned aFlags,
-                    JS::MutableHandle<JSObject*> aObjp);
+                    JS::Handle<jsid> aId, JS::MutableHandle<JS::Value> aValue);
 
   // WebIDL helper methods
   static bool HasBatterySupport(JSContext* /* unused*/, JSObject* /*unused */);
   static bool HasPowerSupport(JSContext* /* unused */, JSObject* aGlobal);
   static bool HasIdleSupport(JSContext* /* unused */, JSObject* aGlobal);
   static bool HasWakeLockSupport(JSContext* /* unused*/, JSObject* /*unused */);
   static bool HasDesktopNotificationSupport(JSContext* /* unused*/,
                                             JSObject* /*unused */)
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -4655,22 +4655,31 @@ nsLocationSH::AddProperty(nsIXPConnectWr
 NS_IMETHODIMP
 nsNavigatorSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                           JSObject *aObj, jsid aId, uint32_t flags,
                           JSObject **objp, bool *_retval)
 {
   JS::Rooted<JSObject*> obj(cx, aObj);
   JS::Rooted<jsid> id(cx, aId);
   nsCOMPtr<nsIDOMNavigator> navigator = do_QueryWrappedNative(wrapper);
-  if (!static_cast<Navigator*>(navigator.get())->
-        DoNewResolve(cx, obj, id, flags,
-                     JS::MutableHandle<JSObject*>::fromMarkedLocation(objp))) {
+  JS::Rooted<JS::Value> value(cx);
+  if (!static_cast<Navigator*>(navigator.get())->DoNewResolve(cx, obj, id,
+                                                              &value)) {
     return NS_ERROR_FAILURE;
   }
 
+  if (!value.isUndefined()) {
+    if (!JS_DefinePropertyById(cx, obj, id, value, JS_PropertyStub,
+                               JS_StrictPropertyStub, JSPROP_ENUMERATE)) {
+      return NS_ERROR_FAILURE;
+    }
+
+    *objp = obj;
+  }
+
   *_retval = true;
   return NS_OK;
 }
 
 // static
 nsresult
 nsNavigatorSH::PreCreate(nsISupports *nativeObj, JSContext *cx,
                          JSObject *globalObj, JSObject **parentObj)
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -80,16 +80,19 @@ class CGNativePropertyHooks(CGThing):
             return ""
         return "extern const NativePropertyHooks sNativePropertyHooks;\n"
     def define(self):
         if self.descriptor.workers:
             return ""
         if self.descriptor.concrete and self.descriptor.proxy:
             resolveOwnProperty = "ResolveOwnProperty"
             enumerateOwnProperties = "EnumerateOwnProperties"
+        elif self.descriptor.interface.getExtendedAttribute("NeedNewResolve"):
+            resolveOwnProperty = "ResolveOwnPropertyViaNewresolve"
+            enumerateOwnProperties = "nullptr"
         else:
             resolveOwnProperty = "nullptr"
             enumerateOwnProperties = "nullptr"
         if self.properties.hasNonChromeOnly():
             regular = "&sNativeProperties"
         else:
             regular = "nullptr"
         if self.properties.hasChromeOnly():
@@ -5125,38 +5128,40 @@ class CGAbstractBindingMethod(CGAbstract
     Common class to generate the JSNatives for all our methods, getters, and
     setters.  This will generate the function declaration and unwrap the
     |this| object.  Subclasses are expected to override the generate_code
     function to do the rest of the work.  This function should return a
     CGThing which is already properly indented.
     """
     def __init__(self, descriptor, name, args, unwrapFailureCode=None,
                  getThisObj="args.computeThis(cx).toObjectOrNull()",
-                 callArgs="JS::CallArgs args = JS::CallArgsFromVp(argc, vp);"):
-        CGAbstractStaticMethod.__init__(self, descriptor, name, "JSBool", args)
+                 callArgs="JS::CallArgs args = JS::CallArgsFromVp(argc, vp);",
+                 returnType="JSBool"):
+        CGAbstractStaticMethod.__init__(self, descriptor, name, returnType, args)
 
         if unwrapFailureCode is None:
             self.unwrapFailureCode = 'return ThrowErrorMessage(cx, MSG_THIS_DOES_NOT_IMPLEMENT_INTERFACE, "Value", "%s");' % descriptor.interface.identifier.name
         else:
             self.unwrapFailureCode = unwrapFailureCode
         self.getThisObj = getThisObj
         self.callArgs = callArgs
 
     def definition_body(self):
         # Our descriptor might claim that we're not castable, simply because
         # we're someone's consequential interface.  But for this-unwrapping, we
         # know that we're the real deal.  So fake a descriptor here for
         # consumption by CastableObjectUnwrapper.
-        getThis = CGGeneric("""%s
-JS::RootedObject obj(cx, %s);
-if (!obj) {
-  return false;
-}
-
-%s* self;""" % (self.callArgs, self.getThisObj, self.descriptor.nativeType))
+        getThis = CGList([
+                CGGeneric(self.callArgs) if self.callArgs != "" else None,
+                CGGeneric("JS::RootedObject obj(cx, %s);\n"
+                          "if (!obj) {\n"
+                          "  return false;\n"
+                          "}" % self.getThisObj) if self.getThisObj else None,
+                CGGeneric("%s* self;" % self.descriptor.nativeType)
+                ], "\n")
         unwrapThis = CGGeneric(
             str(CastableObjectUnwrapper(
                         self.descriptor,
                         "obj", "self", self.unwrapFailureCode)))
         return CGList([ CGIndenter(getThis), CGIndenter(unwrapThis),
                         self.generate_code() ], "\n").define()
 
     def generate_code(self):
@@ -5260,31 +5265,43 @@ class CGLegacyCallHook(CGAbstractBinding
                             self._legacycaller)
 
 class CGNewResolveHook(CGAbstractBindingMethod):
     """
     NewResolve hook for our object
     """
     def __init__(self, descriptor):
         self._needNewResolve = descriptor.interface.getExtendedAttribute("NeedNewResolve")
-        args = [Argument('JSContext*', 'cx'), Argument('JS::Handle<JSObject*>', 'obj_'),
+        args = [Argument('JSContext*', 'cx'), Argument('JS::Handle<JSObject*>', 'obj'),
                 Argument('JS::Handle<jsid>', 'id'), Argument('unsigned', 'flags'),
                 Argument('JS::MutableHandle<JSObject*>', 'objp')]
         # Our "self" is actually the callee in this case, not the thisval.
         CGAbstractBindingMethod.__init__(
             self, descriptor, NEWRESOLVE_HOOK_NAME,
-            args, getThisObj="obj_", callArgs="")
+            args, getThisObj="", callArgs="")
 
     def define(self):
         if not self._needNewResolve:
             return ""
         return CGAbstractBindingMethod.define(self)
 
     def generate_code(self):
-        return CGIndenter(CGGeneric("return self->DoNewResolve(cx, obj, id, flags, objp);"))
+        return CGIndenter(CGGeneric(
+                "JS::Rooted<JS::Value> value(cx);\n"
+                "if (!self->DoNewResolve(cx, obj, id, &value)) {\n"
+                "  return false;\n"
+                "}\n"
+                "if (value.isUndefined()) {\n"
+                "  return true;\n"
+                "}\n"
+                "if (!JS_DefinePropertyById(cx, obj, id, value, nullptr, nullptr, JSPROP_ENUMERATE)) {\n"
+                "  return false;\n"
+                "}\n"
+                "objp.set(obj);\n"
+                "return true;"))
 
 class CppKeywords():
     """
     A class for checking if method names declared in webidl
     are not in conflict with C++ keywords.
     """
     keywords = frozenset(['alignas', 'alignof', 'and', 'and_eq', 'asm', 'auto', 'bitand', 'bitor', 'bool',
     'break', 'case', 'catch', 'char', 'char16_t', 'char32_t', 'class', 'compl', 'const', 'constexpr',
@@ -6624,36 +6641,65 @@ class CGClass(CGThing):
         result = self.extradefinitions
         itemCount = 0
         for (memberList, separator) in order:
             (memberString, itemCount) = defineMembers(self, memberList,
                                                       itemCount, separator)
             result = result + memberString
         return result
 
-class CGResolveOwnProperty(CGAbstractMethod):
+class CGResolveOwnProperty(CGAbstractStaticMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'wrapper'),
                 Argument('JS::Handle<JSObject*>', 'obj'),
                 Argument('JS::Handle<jsid>', 'id'),
                 Argument('JSPropertyDescriptor*', 'desc'), Argument('unsigned', 'flags'),
                 ]
-        CGAbstractMethod.__init__(self, descriptor, "ResolveOwnProperty", "bool", args)
+        CGAbstractStaticMethod.__init__(self, descriptor, "ResolveOwnProperty",
+                                        "bool", args)
     def definition_body(self):
         return """  return js::GetProxyHandler(obj)->getOwnPropertyDescriptor(cx, wrapper, id, desc, flags);
 """
 
-class CGEnumerateOwnProperties(CGAbstractMethod):
+class CGResolveOwnPropertyViaNewresolve(CGAbstractBindingMethod):
+    """
+    An implementation of Xray ResolveOwnProperty stuff for things that have a
+    newresolve hook.
+    """
+    def __init__(self, descriptor):
+        args = [Argument('JSContext*', 'cx'),
+                Argument('JS::Handle<JSObject*>', 'wrapper'),
+                Argument('JS::Handle<JSObject*>', 'obj'),
+                Argument('JS::Handle<jsid>', 'id'),
+                Argument('JSPropertyDescriptor*', 'desc'), Argument('unsigned', 'flags'),
+                ]
+        CGAbstractBindingMethod.__init__(self, descriptor,
+                                         "ResolveOwnPropertyViaNewresolve",
+                                         args, getThisObj="",
+                                         callArgs="", returnType="bool")
+    def generate_code(self):
+        return CGIndenter(CGGeneric(
+                "JS::Rooted<JS::Value> value(cx);\n"
+                "if (!self->DoNewResolve(cx, obj, id, &value)) {\n"
+                "  return false;\n"
+                "}\n"
+                "if (!value.isUndefined()) {\n"
+                "  FillPropertyDescriptor(desc, wrapper, value, false);\n"
+                "}\n"
+                "return true;"))
+
+class CGEnumerateOwnProperties(CGAbstractStaticMethod):
     def __init__(self, descriptor):
         args = [Argument('JSContext*', 'cx'),
                 Argument('JS::Handle<JSObject*>', 'wrapper'),
                 Argument('JS::Handle<JSObject*>', 'obj'),
                 Argument('JS::AutoIdVector&', 'props')]
-        CGAbstractMethod.__init__(self, descriptor, "EnumerateOwnProperties", "bool", args)
+        CGAbstractStaticMethod.__init__(self, descriptor,
+                                        "EnumerateOwnProperties", "bool", args)
     def definition_body(self):
         return """  return js::GetProxyHandler(obj)->getOwnPropertyNames(cx, wrapper, props);
 """
 
 class CGPrototypeTraitsClass(CGClass):
     def __init__(self, descriptor, indent=''):
         templateArgs = [Argument('prototypes::ID', 'PrototypeID')]
         templateSpecialization = ['prototypes::id::' + descriptor.name]
@@ -7601,16 +7647,26 @@ class CGDescriptor(CGThing):
                 # Only generate a trace hook if the class wants a custom hook.
                 if (descriptor.customTrace):
                     cgThings.append(CGClassTraceHook(descriptor))
 
         properties = PropertyArrays(descriptor)
         cgThings.append(CGGeneric(define=str(properties)))
         cgThings.append(CGNativeProperties(descriptor, properties))
 
+        # Set up our Xray callbacks as needed.  Note that we don't need to do
+        # it in workers.
+        if not descriptor.workers and descriptor.concrete and descriptor.proxy:
+            cgThings.append(CGResolveOwnProperty(descriptor))
+            cgThings.append(CGEnumerateOwnProperties(descriptor))
+        elif descriptor.interface.getExtendedAttribute("NeedNewResolve"):
+            cgThings.append(CGResolveOwnPropertyViaNewresolve(descriptor))
+
+        # Now that we have our ResolveOwnProperty/EnumerateOwnProperties stuff
+        # done, set up our NativePropertyHooks.
         cgThings.append(CGNativePropertyHooks(descriptor, properties))
 
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGClassConstructor(descriptor,
                                                descriptor.interface.ctor()))
             cgThings.append(CGClassHasInstanceHook(descriptor))
             cgThings.append(CGInterfaceObjectJSClass(descriptor, properties))
             if descriptor.needsConstructHookHolder():
@@ -7624,22 +7680,16 @@ class CGDescriptor(CGThing):
             cgThings.append(CGPrototypeJSClass(descriptor, properties))
 
         cgThings.append(CGCreateInterfaceObjectsMethod(descriptor, properties))
         if descriptor.interface.hasInterfacePrototypeObject():
             cgThings.append(CGGetProtoObjectMethod(descriptor))
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGGetConstructorObjectMethod(descriptor))
 
-        # Set up our Xray callbacks as needed.  Note that we don't need to do
-        # it in workers.
-        if not descriptor.workers and descriptor.concrete and descriptor.proxy:
-            cgThings.append(CGResolveOwnProperty(descriptor))
-            cgThings.append(CGEnumerateOwnProperties(descriptor))
-
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGDefineDOMInterfaceMethod(descriptor))
 
         if ((descriptor.interface.hasInterfaceObject() or descriptor.interface.getNavigatorProperty()) and
             not descriptor.interface.isExternal() and
             # Workers stuff is never pref-controlled
             not descriptor.workers):
             prefControlled = descriptor.interface.getExtendedAttribute("PrefControlled")