Bug 1364816 part 3. Switch NeedResolve bindings to using a newResolve hook instead of a resolve hook. r=qdot,jandem
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 06 Jun 2017 21:21:44 -0400
changeset 410742 ab1e672183ad4d6c44d8d1c8d73b9e6a1e8e3d54
parent 410741 4b72b120485d9bf593f95da95e6b1d701b74e72d
child 410743 79157ef8e455f91349e4bf5c73c13f5ecb9b63cf
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersqdot, jandem
bugs1364816
milestone55.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 1364816 part 3. Switch NeedResolve bindings to using a newResolve hook instead of a resolve hook. r=qdot,jandem
dom/base/nsDOMClassInfo.cpp
dom/base/nsGlobalWindow.cpp
dom/bindings/Codegen.py
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -1606,16 +1606,21 @@ LookupComponentsShim(JSContext *cx, JS::
                      JS::MutableHandle<JS::PropertyDescriptor> desc);
 
 // static
 bool
 nsWindowSH::NameStructEnabled(JSContext* aCx, nsGlobalWindow *aWin,
                               const nsAString& aName,
                               const nsGlobalNameStruct& aNameStruct)
 {
+  // DOMConstructor is special: creating its proto does not actually define it
+  // as a property on the global.  So we don't want to expose its name either.
+  if (aName.EqualsLiteral("DOMConstructor")) {
+    return false;
+  }
   const nsGlobalNameStruct* nameStruct = &aNameStruct;
   return (nameStruct->mType != nsGlobalNameStruct::eTypeProperty &&
           nameStruct->mType != nsGlobalNameStruct::eTypeClassConstructor) ||
          OldBindingConstructorEnabled(nameStruct, aWin, aCx);
 }
 
 #ifdef RELEASE_OR_BETA
 #define USE_CONTROLLERS_SHIM
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -5144,23 +5144,38 @@ nsGlobalWindow::GetOwnPropertyNames(JSCo
   MOZ_ASSERT(IsInnerWindow());
   // "Components" is marked as enumerable but only resolved on demand :-/.
   //aNames.AppendElement(NS_LITERAL_STRING("Components"));
 
   nsScriptNameSpaceManager* nameSpaceManager = GetNameSpaceManager();
   if (nameSpaceManager) {
     JS::Rooted<JSObject*> wrapper(aCx, GetWrapper());
 
-    WebIDLGlobalNameHash::GetNames(aCx, wrapper, WebIDLGlobalNameHash::AllNames,
-                                   aNames);
+    // There are actually two ways we can get called here: For normal
+    // enumeration or for Xray enumeration.  In the latter case, we want to
+    // return all possible WebIDL names, because we don't really support
+    // deleting these names off our Xray; trying to resolve them will just make
+    // them come back.  In the former case, we want to avoid returning deleted
+    // names.  But the JS engine already knows about the non-deleted
+    // already-resolved names, so we can just return the so-far-unresolved ones.
+    //
+    // We can tell which case we're in by whether aCx is in our wrapper's
+    // compartment.  If not, we're in the Xray case.
+    WebIDLGlobalNameHash::NameType nameType =
+      js::IsObjectInContextCompartment(wrapper, aCx) ?
+        WebIDLGlobalNameHash::UnresolvedNamesOnly :
+        WebIDLGlobalNameHash::AllNames;
+    WebIDLGlobalNameHash::GetNames(aCx, wrapper, nameType, aNames);
 
     for (auto i = nameSpaceManager->GlobalNameIter(); !i.Done(); i.Next()) {
       const GlobalNameMapEntry* entry = i.Get();
       if (nsWindowSH::NameStructEnabled(aCx, this, entry->mKey,
                                         entry->mGlobalName)) {
+        // Just append all of these; even if they get deleted our resolve hook
+        // just goes ahead and recreates them.
         aNames.AppendElement(entry->mKey);
       }
     }
   }
 }
 
 /* static */ bool
 nsGlobalWindow::IsPrivilegedChromeWindow(JSContext* aCx, JSObject* aObj)
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -449,31 +449,55 @@ class CGDOMJSClass(CGThing):
             traceHook = "JS_GlobalObjectTraceHook"
             reservedSlots = "JSCLASS_GLOBAL_APPLICATION_SLOTS"
         else:
             classFlags += "JSCLASS_HAS_RESERVED_SLOTS(%d)" % slotCount
             traceHook = 'nullptr'
             reservedSlots = slotCount
         if self.descriptor.interface.hasProbablyShortLivingWrapper():
             classFlags += " | JSCLASS_SKIP_NURSERY_FINALIZE"
+
+        objectOps = "JS_NULL_OBJECT_OPS"
+        objectOpsString = ""
         if self.descriptor.interface.getExtendedAttribute("NeedResolve"):
             resolveHook = RESOLVE_HOOK_NAME
             mayResolveHook = MAY_RESOLVE_HOOK_NAME
-            enumerateHook = ENUMERATE_HOOK_NAME
+            enumerateHook = "nullptr"
+            objectOpsString = fill(
+                """
+                const js::ObjectOps sInstanceObjectOps = {
+                  nullptr, /* lookupProperty */
+                  nullptr, /* defineProperty */
+                  nullptr, /* hasProperty */
+                  nullptr, /* getProperty */
+                  nullptr, /* setProperty */
+                  nullptr, /* getOwnPropertyDescriptor */
+                  nullptr, /* deleteProperty */
+                  nullptr, /* watch */
+                  nullptr, /* unwatch */
+                  nullptr, /* getElements */
+                  ${enumerate}, /* enumerate */
+                  nullptr, /* funToString */
+                };
+
+                """,
+                enumerate=ENUMERATE_HOOK_NAME)
+            objectOps = "&sInstanceObjectOps"
         elif self.descriptor.isGlobal():
             resolveHook = "mozilla::dom::ResolveGlobal"
             mayResolveHook = "mozilla::dom::MayResolveGlobal"
             enumerateHook = "mozilla::dom::EnumerateGlobal"
         else:
             resolveHook = "nullptr"
             mayResolveHook = "nullptr"
             enumerateHook = "nullptr"
 
         return fill(
             """
+            $*{objectOpsString}
             static const js::ClassOps sClassOps = {
               ${addProperty}, /* addProperty */
               nullptr,               /* delProperty */
               nullptr,               /* getProperty */
               nullptr,               /* setProperty */
               ${enumerate}, /* enumerate */
               ${resolve}, /* resolve */
               ${mayResolve}, /* mayResolve */
@@ -490,35 +514,37 @@ class CGDOMJSClass(CGThing):
             };
 
             static const DOMJSClass sClass = {
               { "${name}",
                 ${flags},
                 &sClassOps,
                 JS_NULL_CLASS_SPEC,
                 &sClassExtension,
-                JS_NULL_OBJECT_OPS
+                ${objectOps}
               },
               $*{descriptor}
             };
             static_assert(${instanceReservedSlots} == DOM_INSTANCE_RESERVED_SLOTS,
                           "Must have the right minimal number of reserved slots.");
             static_assert(${reservedSlots} >= ${slotCount},
                           "Must have enough reserved slots.");
             """,
+            objectOpsString=objectOpsString,
             name=self.descriptor.interface.identifier.name,
             flags=classFlags,
             addProperty=ADDPROPERTY_HOOK_NAME if wantsAddProperty(self.descriptor) else 'nullptr',
             enumerate=enumerateHook,
             resolve=resolveHook,
             mayResolve=mayResolveHook,
             finalize=FINALIZE_HOOK_NAME,
             call=callHook,
             trace=traceHook,
             objectMoved=objectMovedHook,
+            objectOps=objectOps,
             descriptor=DOMClass(self.descriptor),
             instanceReservedSlots=INSTANCE_RESERVED_SLOTS,
             reservedSlots=reservedSlots,
             slotCount=slotCount)
 
 
 class CGDOMProxyJSClass(CGThing):
     """
@@ -8888,43 +8914,52 @@ class CGMayResolveHook(CGAbstractStaticM
 class CGEnumerateHook(CGAbstractBindingMethod):
     """
     Enumerate hook for objects with custom hooks.
     """
     def __init__(self, descriptor):
         assert descriptor.interface.getExtendedAttribute("NeedResolve")
 
         args = [Argument('JSContext*', 'cx'),
-                Argument('JS::Handle<JSObject*>', 'obj')]
+                Argument('JS::Handle<JSObject*>', 'obj'),
+                Argument('JS::AutoIdVector&', 'properties'),
+                Argument('bool', 'enumerableOnly')]
         # Our "self" is actually the "obj" argument in this case, not the thisval.
         CGAbstractBindingMethod.__init__(
             self, descriptor, ENUMERATE_HOOK_NAME,
             args, getThisObj="", callArgs="")
 
     def generate_code(self):
         return CGGeneric(dedent("""
             AutoTArray<nsString, 8> names;
             binding_detail::FastErrorResult rv;
             self->GetOwnPropertyNames(cx, names, rv);
             if (rv.MaybeSetPendingException(cx)) {
               return false;
             }
-            bool dummy;
-            for (uint32_t i = 0; i < names.Length(); ++i) {
-              if (!JS_HasUCProperty(cx, obj, names[i].get(), names[i].Length(), &dummy)) {
+            JS::Rooted<JS::Value> v(cx);
+            JS::Rooted<jsid> id(cx);
+            for (auto& name : names) {
+              if (!xpc::NonVoidStringToJsval(cx, name, &v) ||
+                  !JS_ValueToId(cx, v, &id) ||
+                  !properties.append(id)) {
                 return false;
               }
             }
             return true;
             """))
 
     def definition_body(self):
         if self.descriptor.isGlobal():
             # Enumerate standard classes
             prefix = dedent("""
+                // This is OK even though we're a newEnumerate hook: this will
+                // define the relevant properties on the global, and the JS
+                // engine will pick those up, because it looks at the object's
+                // properties after this hook has returned.
                 if (!EnumerateGlobal(cx, obj)) {
                   return false;
                 }
 
                 """)
         else:
             prefix = ""
         return prefix + CGAbstractBindingMethod.definition_body(self)