Bug 1765992 - Fix WebIDL Xrays to properly deal with a static and non-static property with the same name. r=edgar

Differential Revision: https://phabricator.services.mozilla.com/D171705

dom/bindings/BindingUtils.cpp

@@ -1268,17 +1268,14 @@ bool WrapObject(JSContext* cx, const WindowProxyHolder& p,
   return ToJSValue(cx, p, rval);
 }
 
-static int CompareIdsAtIndices(const void* aElement1, const void* aElement2,
-                               void* aClosure) {
+static int ComparePropertyInfosAtIndices(const void* aElement1,
+                                         const void* aElement2,
+                                         void* aClosure) {
   const uint16_t index1 = *static_cast<const uint16_t*>(aElement1);
   const uint16_t index2 = *static_cast<const uint16_t*>(aElement2);
   const PropertyInfo* infos = static_cast<PropertyInfo*>(aClosure);
 
-  uintptr_t rawBits1 = infos[index1].Id().asRawBits();
-  uintptr_t rawBits2 = infos[index2].Id().asRawBits();
-  MOZ_ASSERT(rawBits1 != rawBits2);
-
-  return rawBits1 < rawBits2 ? -1 : 1;
+  return PropertyInfo::Compare(infos[index1], infos[index2]);
 }
 
 // {JSPropertySpec,JSFunctionSpec} use {JSPropertySpec,JSFunctionSpec}::Name
@@ -1351,10 +1348,11 @@ static bool InitPropertyInfos(JSContext* cx,
   for (unsigned int i = 0; i < nativeProperties->propertyInfoCount; ++i) {
     indices[i] = i;
   }
-  // CompareIdsAtIndices() doesn't actually modify the PropertyInfo array, so
-  // the const_cast here is OK in spite of the signature of NS_QuickSort().
+  // ComparePropertyInfosAtIndices() doesn't actually modify the PropertyInfo
+  // array, so the const_cast here is OK in spite of the signature of
+  // NS_QuickSort().
   NS_QuickSort(indices, nativeProperties->propertyInfoCount, sizeof(uint16_t),
-               CompareIdsAtIndices,
+               ComparePropertyInfosAtIndices,
                const_cast<PropertyInfo*>(nativeProperties->PropertyInfos()));
 
   return true;
@@ -1476,23 +1474,40 @@ static JSObject* XrayCreateFunction(JSContext* cx,
 struct IdToIndexComparator {
   // The id we're searching for.
   const jsid& mId;
+  // Whether we're searching for static operations.
+  const bool mStatic;
   // The list of ids we're searching in.
   const PropertyInfo* mInfos;
 
-  explicit IdToIndexComparator(const jsid& aId, const PropertyInfo* aInfos)
-      : mId(aId), mInfos(aInfos) {}
+  IdToIndexComparator(const jsid& aId, DOMObjectType aType,
+                      const PropertyInfo* aInfos)
+      : mId(aId), mStatic(aType == eInterface), mInfos(aInfos) {}
   int operator()(const uint16_t aIndex) const {
-    if (mId.asRawBits() == mInfos[aIndex].Id().asRawBits()) {
-      return 0;
+    const PropertyInfo& info = mInfos[aIndex];
+    if (mId.asRawBits() == info.Id().asRawBits()) {
+      if (info.type != eMethod && info.type != eStaticMethod) {
+        return 0;
+      }
+
+      if (mStatic == info.IsStaticMethod()) {
+        // We're looking for static properties and we've found a static one for
+        // the right name.
+        return 0;
+      }
+
+      // Static operations are sorted before others by PropertyInfo::Compare.
+      return mStatic ? -1 : 1;
     }
-    return mId.asRawBits() < mInfos[aIndex].Id().asRawBits() ? -1 : 1;
+
+    return mId.asRawBits() < info.Id().asRawBits() ? -1 : 1;
   }
 };
 
 static const PropertyInfo* XrayFindOwnPropertyInfo(
-    JSContext* cx, JS::Handle<jsid> id,
+    JSContext* cx, DOMObjectType type, JS::Handle<jsid> id,
     const NativeProperties* nativeProperties) {
-  if (MOZ_UNLIKELY(nativeProperties->iteratorAliasMethodIndex >= 0) &&
+  if ((type == eInterfacePrototype || type == eGlobalInstance) &&
+      MOZ_UNLIKELY(nativeProperties->iteratorAliasMethodIndex >= 0) &&
       id.isWellKnownSymbol(JS::SymbolCode::iterator)) {
     return nativeProperties->MethodPropertyInfos() +
            nativeProperties->iteratorAliasMethodIndex;
@@ -1505,7 +1520,7 @@ static const PropertyInfo* XrayFindOwnPropertyInfo(
 
   if (BinarySearchIf(sortedPropertyIndices, 0,
                      nativeProperties->propertyInfoCount,
-                     IdToIndexComparator(id, propertyInfos), &idx)) {
+                     IdToIndexComparator(id, type, propertyInfos), &idx)) {
     return propertyInfos + sortedPropertyIndices[idx];
   }
 
@@ -1737,11 +1752,11 @@ static bool ResolvePrototypeOrConstructor(
   const PropertyInfo* found = nullptr;
 
   if ((nativeProperties = nativePropertiesHolder.regular)) {
-    found = XrayFindOwnPropertyInfo(cx, id, nativeProperties);
+    found = XrayFindOwnPropertyInfo(cx, type, id, nativeProperties);
   }
   if (!found && (nativeProperties = nativePropertiesHolder.chromeOnly) &&
       xpc::AccessCheck::isChrome(JS::GetCompartment(wrapper))) {
-    found = XrayFindOwnPropertyInfo(cx, id, nativeProperties);
+    found = XrayFindOwnPropertyInfo(cx, type, id, nativeProperties);
   }
 
   if (IsInstance(type)) {

dom/bindings/DOMJSClass.h

@@ -221,6 +221,25 @@ struct PropertyInfo {
     mIdBits = aId.asRawBits();
   }
   MOZ_ALWAYS_INLINE jsid Id() const { return jsid::fromRawBits(mIdBits); }
+
+  bool IsStaticMethod() const { return type == eStaticMethod; }
+
+  static int Compare(const PropertyInfo& aInfo1, const PropertyInfo& aInfo2) {
+    // IdToIndexComparator needs to be updated if the order here is changed!
+    if (MOZ_UNLIKELY(aInfo1.mIdBits == aInfo2.mIdBits)) {
+      MOZ_ASSERT((aInfo1.type == eMethod || aInfo1.type == eStaticMethod) &&
+                 (aInfo2.type == eMethod || aInfo2.type == eStaticMethod));
+
+      bool isStatic1 = aInfo1.IsStaticMethod();
+
+      MOZ_ASSERT(isStatic1 != aInfo2.IsStaticMethod(),
+                 "We shouldn't have 2 static methods with the same name!");
+
+      return isStatic1 ? -1 : 1;
+    }
+
+    return aInfo1.mIdBits < aInfo2.mIdBits ? -1 : 1;
+  }
 };
 
 static_assert(

dom/bindings/test/TestFunctions.h

@@ -110,6 +110,12 @@ class TestFunctions : public NonRefcountedDOMObject {
   void TestUnionOfAllowSharedBuffferSource(
       const MaybeSharedArrayBufferOrMaybeSharedArrayBufferView& aUnion);
 
+  bool StaticAndNonStaticOverload() { return false; }
+  static bool StaticAndNonStaticOverload(GlobalObject& aGlobal,
+                                         const Optional<uint32_t>& aLength) {
+    return true;
+  }
+
   static bool ObjectFromAboutBlank(JSContext* aCx, JSObject* aObj);
 
   WrapperCachedNonISupportsTestInterface* WrapperCachedNonISupportsObject();

dom/bindings/test/test_dom_xrays.html

@@ -13,7 +13,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=787070
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=787070">Mozilla Bug 787070</a>
 <p id="display"></p>
 <div id="content" style="display: none">
-<iframe id="t" src="http://example.org/tests/dom/bindings/test/file_dom_xrays.html"></iframe>
+<iframe id="t"></iframe>
 </div>
 <pre id="test">
 <script type="application/javascript">
@@ -293,6 +293,34 @@ function test() {
   isnot(typeof Object.getOwnPropertyDescriptor(win.Node.prototype, "ELEMENT_NODE"), "undefined",
         "Should see constant property on prototype objects");
 
+  // Interfaces can have both static and non-static properties with the same name.
+  isnot(typeof win.TestFunctions.staticAndNonStaticOverload, "undefined",
+        "Should see static property on interface objects (even with non-static property with the same name)");
+  isnot(typeof Object.getOwnPropertyDescriptor(win.TestFunctions, "staticAndNonStaticOverload"), "undefined",
+        "Should see static property on interface objects (even with non-static property with the same name)");
+  isnot(Object.getOwnPropertyNames(win.TestFunctions).indexOf("staticAndNonStaticOverload"), -1,
+        "Should see static property on interface objects (even with non-static property with the same name)");
+  isnot(typeof (new win.TestFunctions("")).staticAndNonStaticOverload, "undefined",
+        "Should see non-static property on prototype objects (even with static property with the same name)");
+  let testFunctions = new win.TestFunctions();
+  is(Object.getOwnPropertyDescriptor(testFunctions, "staticAndNonStaticOverload"), undefined,
+     "Shouldn't see non-static property on instances (even with static property with the same name)");
+  ok(!testFunctions.staticAndNonStaticOverload(),
+     "Should call the non-static overload on the instance");
+  ok(win.TestFunctions.staticAndNonStaticOverload(),
+     "Should call the static overload on the interface object");
+  isnot(typeof Object.getOwnPropertyDescriptor(win.TestFunctions.prototype, "staticAndNonStaticOverload"), "undefined",
+        "Should see non-static property on prototype objects (even with static property with the same name)");
+  is(Object.getOwnPropertyDescriptor(win.TestFunctions, "staticAndNonStaticOverload").value,
+     Object.getOwnPropertyDescriptor(win.TestFunctions, "staticAndNonStaticOverload").value,
+     "Should get the same value when getting the static property twice");
+  is(Object.getOwnPropertyDescriptor(win.TestFunctions.prototype, "staticAndNonStaticOverload").value,
+     Object.getOwnPropertyDescriptor(win.TestFunctions.prototype, "staticAndNonStaticOverload").value,
+     "Should get the same value when getting the non-static property twice");
+  isnot(Object.getOwnPropertyDescriptor(win.TestFunctions, "staticAndNonStaticOverload").value,
+        Object.getOwnPropertyDescriptor(win.TestFunctions.prototype, "staticAndNonStaticOverload").value,
+        "Should get different values for static and non-static properties with the same name");
+
   // Adopting nodes should not lose expandos.
   elem = document.createElement("span");
   elem.expando = 5;
@@ -331,7 +359,18 @@ function test() {
 }
 
 SimpleTest.waitForExplicitFinish();
-addLoadEvent(test);
+SimpleTest.requestLongerTimeout(2);
+
+function loadIframeAndTest() {
+  let iframe = document.getElementById("t");
+  iframe.addEventListener("load", test, { once: true });
+  iframe.src = "http://example.org/tests/dom/bindings/test/file_dom_xrays.html";
+}
+
+addLoadEvent(() => {
+  SpecialPowers.pushPrefEnv({set: [["dom.expose_test_interfaces", true]]},
+                            loadIframeAndTest);
+});
 
 </script>
 </pre>

dom/webidl/TestFunctions.webidl

@@ -141,6 +141,9 @@ interface TestFunctions {
   undefined testDictWithAllowShared(optional DictWithAllowSharedBufferSource buffer = {});
   undefined testUnionOfBuffferSource((ArrayBuffer or ArrayBufferView or DOMString) foo);
   undefined testUnionOfAllowSharedBuffferSource(([AllowShared] ArrayBuffer or [AllowShared] ArrayBufferView) foo);
+
+  boolean staticAndNonStaticOverload();
+  static boolean staticAndNonStaticOverload(optional unsigned long foo);
 };
 
 dictionary DictWithAllowSharedBufferSource {