Bug 880367 part 1. Change the "enabled" callback for WebIDL constructors to take a JSContext* and the object the constructor will be defined on. r=smaug,bholley

content/html/content/src/HTMLTrackElement.cpp

@@ -114,7 +114,8 @@ HTMLTrackElement::WrapNode(JSContext* aCx, JS::Handle<JSObject*> aScope)
 bool
 HTMLTrackElement::IsWebVTTEnabled()
 {
-  return HTMLTrackElementBinding::PrefEnabled();
+  // Our callee does not use its arguments.
+  return HTMLTrackElementBinding::ConstructorEnabled(nullptr, JS::NullPtr());
 }
 
 TextTrack*

dom/base/nsDOMClassInfo.cpp

@@ -3934,7 +3934,8 @@ ResolvePrototype(nsIXPConnect *aXPConnect, nsGlobalWindow *aWin, JSContext *cx,
 }
 
 static bool
-ConstructorEnabled(const nsGlobalNameStruct *aStruct, nsGlobalWindow *aWin)
+OldBindingConstructorEnabled(const nsGlobalNameStruct *aStruct,
+                             nsGlobalWindow *aWin)
 {
   MOZ_ASSERT(aStruct->mType == nsGlobalNameStruct::eTypeClassConstructor ||
              aStruct->mType == nsGlobalNameStruct::eTypeExternalClassInfo);
@@ -3998,11 +3999,7 @@ nsWindowSH::GlobalResolve(nsGlobalWindow *aWin, JSContext *cx,
       name_struct->mDefineDOMInterface;
     if (define) {
       if (name_struct->mType == nsGlobalNameStruct::eTypeClassConstructor &&
-          !ConstructorEnabled(name_struct, aWin)) {
-        return NS_OK;
-      }
-
-      if (name_struct->mPrefEnabled && !(*name_struct->mPrefEnabled)()) {
+          !OldBindingConstructorEnabled(name_struct, aWin)) {
         return NS_OK;
       }
 
@@ -4019,6 +4016,14 @@ nsWindowSH::GlobalResolve(nsGlobalWindow *aWin, JSContext *cx,
         global = obj;
       }
 
+      // Check whether our constructor is enabled after we unwrap Xrays, since
+      // we don't want to define an interface on the Xray if it's disabled in
+      // the target global, even if it's enabled in the Xray's global.
+      if (name_struct->mConstructorEnabled &&
+          !(*name_struct->mConstructorEnabled)(cx, global)) {
+        return NS_OK;
+      }
+
       bool enabled;
       JS::Rooted<JSObject*> interfaceObject(cx, define(cx, global, id, &enabled));
       if (enabled) {
@@ -4081,7 +4086,7 @@ nsWindowSH::GlobalResolve(nsGlobalWindow *aWin, JSContext *cx,
 
   if (name_struct->mType == nsGlobalNameStruct::eTypeClassConstructor ||
       name_struct->mType == nsGlobalNameStruct::eTypeExternalClassInfo) {
-    if (!ConstructorEnabled(name_struct, aWin)) {
+    if (!OldBindingConstructorEnabled(name_struct, aWin)) {
       return NS_OK;
     }
 
@@ -4888,16 +4893,21 @@ nsNavigatorSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
     mozilla::dom::ConstructNavigatorProperty construct = name_struct->mConstructNavigatorProperty;
     MOZ_ASSERT(construct);
 
-    if (name_struct->mPrefEnabled && !(*name_struct->mPrefEnabled)()) {
-      return NS_OK;
-    }
-
     JS::Rooted<JSObject*> naviObj(cx, js::CheckedUnwrap(obj, /* stopAtOuter = */ false));
     NS_ENSURE_TRUE(naviObj, NS_ERROR_DOM_SECURITY_ERR);
 
     JS::Rooted<JSObject*> domObject(cx);
     {
       JSAutoCompartment ac(cx, naviObj);
+
+      // Check whether our constructor is enabled after we unwrap Xrays, since
+      // we don't want to define an interface on the Xray if it's disabled in
+      // the target global, even if it's enabled in the Xray's global.
+      if (name_struct->mConstructorEnabled &&
+          !(*name_struct->mConstructorEnabled)(cx, naviObj)) {
+        return NS_OK;
+      }
+
       domObject = construct(cx, naviObj);
       if (!domObject) {
         return NS_ERROR_FAILURE;

dom/base/nsScriptNameSpaceManager.cpp

@@ -825,7 +825,7 @@ nsScriptNameSpaceManager::Observe(nsISupports* aSubject, const char* aTopic,
 void
 nsScriptNameSpaceManager::RegisterDefineDOMInterface(const nsAFlatString& aName,
     mozilla::dom::DefineInterface aDefineDOMInterface,
-    mozilla::dom::PrefEnabled aPrefEnabled)
+    mozilla::dom::ConstructorEnabled* aConstructorEnabled)
 {
   nsGlobalNameStruct *s = AddToHash(&mGlobalNames, &aName);
   if (s) {
@@ -833,7 +833,7 @@ nsScriptNameSpaceManager::RegisterDefineDOMInterface(const nsAFlatString& aName,
       s->mType = nsGlobalNameStruct::eTypeNewDOMBinding;
     }
     s->mDefineDOMInterface = aDefineDOMInterface;
-    s->mPrefEnabled = aPrefEnabled;
+    s->mConstructorEnabled = aConstructorEnabled;
   }
 }
 
@@ -841,7 +841,7 @@ void
 nsScriptNameSpaceManager::RegisterNavigatorDOMConstructor(
     const nsAFlatString& aName,
     mozilla::dom::ConstructNavigatorProperty aNavConstructor,
-    mozilla::dom::PrefEnabled aPrefEnabled)
+    mozilla::dom::ConstructorEnabled* aConstructorEnabled)
 {
   nsGlobalNameStruct *s = AddToHash(&mNavigatorNames, &aName);
   if (s) {
@@ -849,7 +849,7 @@ nsScriptNameSpaceManager::RegisterNavigatorDOMConstructor(
       s->mType = nsGlobalNameStruct::eTypeNewDOMBinding;
     }
     s->mConstructNavigatorProperty = aNavConstructor;
-    s->mPrefEnabled = aPrefEnabled;
+    s->mConstructorEnabled = aConstructorEnabled;
   }
 }
 

dom/base/nsScriptNameSpaceManager.h

@@ -55,6 +55,9 @@ struct nsGlobalNameStruct
     eTypeExternalConstructorAlias
   } mType;
 
+  // mChromeOnly is only used for structs that define non-WebIDL things
+  // (possibly in addition to WebIDL ones).  In particular, it's not even
+  // initialized for eTypeNewDOMBinding structs.
   bool mChromeOnly;
   bool mDisabled;
 
@@ -71,7 +74,8 @@ struct nsGlobalNameStruct
     mozilla::dom::DefineInterface mDefineDOMInterface; // for window
     mozilla::dom::ConstructNavigatorProperty mConstructNavigatorProperty; // for navigator
   };
-  mozilla::dom::PrefEnabled mPrefEnabled; // May be null if not pref controlled
+  // May be null if enabled unconditionally
+  mozilla::dom::ConstructorEnabled* mConstructorEnabled;
 };
 
 
@@ -140,11 +144,11 @@ public:
 
   void RegisterDefineDOMInterface(const nsAFlatString& aName,
     mozilla::dom::DefineInterface aDefineDOMInterface,
-    mozilla::dom::PrefEnabled aPrefEnabled);
+    mozilla::dom::ConstructorEnabled* aConstructorEnabled);
 
   void RegisterNavigatorDOMConstructor(const nsAFlatString& aName,
     mozilla::dom::ConstructNavigatorProperty aNavConstructor,
-    mozilla::dom::PrefEnabled aPrefEnabled);
+    mozilla::dom::ConstructorEnabled* aConstructorEnabled);
 
   typedef PLDHashOperator
   (* GlobalNameEnumerator)(const nsAString& aGlobalName, void* aClosure);

dom/bindings/Codegen.py

@@ -1978,7 +1978,11 @@ class CGPrefEnabledNative(CGAbstractMethod):
     if the method returns true.
     """
     def __init__(self, descriptor):
-        CGAbstractMethod.__init__(self, descriptor, 'PrefEnabled', 'bool', [])
+        CGAbstractMethod.__init__(self, descriptor,
+                                  'ConstructorEnabled', 'bool',
+                                  [Argument("JSContext*", "/* unused */"),
+                                   Argument("JS::Handle<JSObject*>",
+                                            "/* unused */")])
 
     def definition_body(self):
         return "  return %s::PrefEnabled();" % self.descriptor.nativeType
@@ -1991,7 +1995,11 @@ class CGPrefEnabled(CGAbstractMethod):
     on the global if the pref is true.
     """
     def __init__(self, descriptor):
-        CGAbstractMethod.__init__(self, descriptor, 'PrefEnabled', 'bool', [])
+        CGAbstractMethod.__init__(self, descriptor,
+                                  'ConstructorEnabled', 'bool',
+                                  [Argument("JSContext*", "/* unused */"),
+                                   Argument("JS::Handle<JSObject*>",
+                                            "/* unused */")])
 
     def definition_body(self):
         pref = self.descriptor.interface.getExtendedAttribute("Pref")
@@ -8027,12 +8035,12 @@ class CGRegisterProtos(CGAbstractMethod):
 
     def _defineMacro(self):
        return """
-#define REGISTER_PROTO(_dom_class, _pref_check) \\
-  aNameSpaceManager->RegisterDefineDOMInterface(NS_LITERAL_STRING(#_dom_class), _dom_class##Binding::DefineDOMInterface, _pref_check);
-#define REGISTER_CONSTRUCTOR(_dom_constructor, _dom_class, _pref_check) \\
-  aNameSpaceManager->RegisterDefineDOMInterface(NS_LITERAL_STRING(#_dom_constructor), _dom_class##Binding::DefineDOMInterface, _pref_check);
-#define REGISTER_NAVIGATOR_CONSTRUCTOR(_prop, _dom_class, _pref_check) \\
-  aNameSpaceManager->RegisterNavigatorDOMConstructor(NS_LITERAL_STRING(_prop), _dom_class##Binding::ConstructNavigatorObject, _pref_check);
+#define REGISTER_PROTO(_dom_class, _ctor_check) \\
+  aNameSpaceManager->RegisterDefineDOMInterface(NS_LITERAL_STRING(#_dom_class), _dom_class##Binding::DefineDOMInterface, _ctor_check);
+#define REGISTER_CONSTRUCTOR(_dom_constructor, _dom_class, _ctor_check) \\
+  aNameSpaceManager->RegisterDefineDOMInterface(NS_LITERAL_STRING(#_dom_constructor), _dom_class##Binding::DefineDOMInterface, _ctor_check);
+#define REGISTER_NAVIGATOR_CONSTRUCTOR(_prop, _dom_class, _ctor_check) \\
+  aNameSpaceManager->RegisterNavigatorDOMConstructor(NS_LITERAL_STRING(_prop), _dom_class##Binding::ConstructNavigatorObject, _ctor_check);
 
 """
     def _undefineMacro(self):
@@ -8041,23 +8049,23 @@ class CGRegisterProtos(CGAbstractMethod):
 #undef REGISTER_PROTO
 #undef REGISTER_NAVIGATOR_CONSTRUCTOR"""
     def _registerProtos(self):
-        def getPrefCheck(desc):
+        def getCheck(desc):
             if (desc.interface.getExtendedAttribute("PrefControlled") is None and
                 desc.interface.getExtendedAttribute("Pref") is None):
                 return "nullptr"
-            return "%sBinding::PrefEnabled" % desc.name
+            return "%sBinding::ConstructorEnabled" % desc.name
         lines = []
         for desc in self.config.getDescriptors(hasInterfaceObject=True,
                                                isExternal=False,
                                                workers=False,
                                                register=True):
-            lines.append("REGISTER_PROTO(%s, %s);" % (desc.name, getPrefCheck(desc)))
-            lines.extend("REGISTER_CONSTRUCTOR(%s, %s, %s);" % (n.identifier.name, desc.name, getPrefCheck(desc))
+            lines.append("REGISTER_PROTO(%s, %s);" % (desc.name, getCheck(desc)))
+            lines.extend("REGISTER_CONSTRUCTOR(%s, %s, %s);" % (n.identifier.name, desc.name, getCheck(desc))
                          for n in desc.interface.namedConstructors)
         for desc in self.config.getDescriptors(isNavigatorProperty=True, register=True):
             propName = desc.interface.getNavigatorProperty()
             assert propName
-            lines.append('REGISTER_NAVIGATOR_CONSTRUCTOR("%s", %s, %s);' % (propName, desc.name, getPrefCheck(desc)))
+            lines.append('REGISTER_NAVIGATOR_CONSTRUCTOR("%s", %s, %s);' % (propName, desc.name, getCheck(desc)))
         return '\n'.join(lines) + '\n'
     def definition_body(self):
         return self._defineMacro() + self._registerProtos() + self._undefineMacro()

js/xpconnect/src/xpcpublic.h

@@ -479,8 +479,14 @@ typedef JSObject*
 typedef JSObject*
 (*ConstructNavigatorProperty)(JSContext *cx, JS::Handle<JSObject*> naviObj);
 
+// Check whether a constructor should be enabled for the given object.
+// Note that the object should NOT be an Xray, since Xrays will end up
+// defining constructors on the underlying object.
+// This is a typedef for the function type itself, not the function
+// pointer, so it's more obvious that pointers to a ConstructorEnabled
+// can be null.
 typedef bool
-(*PrefEnabled)();
+(ConstructorEnabled)(JSContext* cx, JS::Handle<JSObject*> obj);
 
 extern bool
 DefineStaticJSVals(JSContext *cx);