Bug 1330699 part 7. Change JS to MozMap conversion to more closely follow the record<> spec. r=qdot
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 15 Feb 2017 00:00:06 -0500
changeset 484461 1c8ff160682ed634ece98ea863856abacd5b0c9a
parent 484460 97d307213bf663a9144ced667e3820fdf2399320
child 484462 52a24f98f12a23e67463ab60bd7a737b355df255
push id45482
push userbmo:kmckinley@mozilla.com
push dateWed, 15 Feb 2017 09:52:37 +0000
reviewersqdot
bugs1330699
milestone54.0a1
Bug 1330699 part 7. Change JS to MozMap conversion to more closely follow the record<> spec. r=qdot The spec says to get all the property keys, then check each one for enumerability before doing the Get(). Our current code, before this change, asks for all the _enumerable_ keys instead, which is observably different when proxies are involved.
dom/bindings/Codegen.py
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4874,37 +4874,55 @@ def getJSToNativeConversionInfo(type, de
             "passedToJSImpl": "${passedToJSImpl}"
         })
 
         templateBody = fill(
             """
             auto& mozMapEntries = ${mozMapRef}.Entries();
 
             JS::Rooted<JSObject*> mozMapObj(cx, &$${val}.toObject());
-            JS::Rooted<JS::IdVector> ids(cx, JS::IdVector(cx));
-            if (!JS_Enumerate(cx, mozMapObj, &ids)) {
+            JS::AutoIdVector ids(cx);
+            // Keep skipping symbols until
+            // https://github.com/heycam/webidl/issues/294 is sorted out.
+            if (!js::GetPropertyKeys(cx, mozMapObj,
+                                     JSITER_OWNONLY | JSITER_HIDDEN, &ids)) {
               $*{exceptionCode}
             }
             if (!mozMapEntries.SetCapacity(ids.length(), mozilla::fallible)) {
               JS_ReportOutOfMemory(cx);
               $*{exceptionCode}
             }
             JS::Rooted<JS::Value> propNameValue(cx);
             JS::Rooted<JS::Value> temp(cx);
             JS::Rooted<jsid> curId(cx);
             for (size_t i = 0; i < ids.length(); ++i) {
               curId = ids[i];
+
+              MOZ_ASSERT(!JSID_IS_SYMBOL(curId), "No symbols, we said!");
+
+              JS::Rooted<JS::PropertyDescriptor> desc(cx);
+              if (!JS_GetOwnPropertyDescriptorById(cx, mozMapObj, curId,
+                                                   &desc)) {
+                $*{exceptionCode}
+              }
+
+              if (!desc.object() /* == undefined in spec terms */ ||
+                  !desc.enumerable()) {
+                continue;
+              }
+
               binding_detail::FakeString propName;
               bool isSymbol;
-              if (!ConvertIdToString(cx, curId, propName, isSymbol) ||
-                  (!isSymbol && !JS_GetPropertyById(cx, mozMapObj, curId, &temp))) {
+              if (!ConvertIdToString(cx, curId, propName, isSymbol)) {
                 $*{exceptionCode}
               }
-              if (isSymbol) {
-                continue;
+              MOZ_ASSERT(!isSymbol, "We said, no symbols!");
+
+              if (!JS_GetPropertyById(cx, mozMapObj, curId, &temp)) {
+                $*{exceptionCode}
               }
 
               // Safe to do an infallible append here, because we did a
               // SetCapacity above to the right capacity.
               ${mozMapType}::EntryType* entry = mozMapEntries.AppendElement();
               entry->mKey = propName;
               ${valueType}& slot = entry->mValue;
               $*{valueConversion}