Backed out 2 changesets (bug 1398354) for spidermonkey and hazard bustages at js/src/jsapi-tests/testStringIsArrayIndex.cpp on a CLOSED TREE
authorCoroiu Cristina <ccoroiu@mozilla.com>
Thu, 18 Oct 2018 06:23:14 +0300
changeset 490133 a42a9fc0c5f4bc801873a3803ffefbe0761c9b59
parent 490132 344b7e258ea5d73fdd919a6ed6459189874c3533
child 490134 ee8274a9b6c09241f0da9b6ed3b83a95b8091574
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
bugs1398354
milestone64.0a1
backs out344b7e258ea5d73fdd919a6ed6459189874c3533
8e41c385edb99568fed370ef17d646dceefa00f9
Backed out 2 changesets (bug 1398354) for spidermonkey and hazard bustages at js/src/jsapi-tests/testStringIsArrayIndex.cpp on a CLOSED TREE Backed out changeset 344b7e258ea5 (bug 1398354) Backed out changeset 8e41c385edb9 (bug 1398354)
dom/html/HTMLAllCollection.cpp
dom/html/HTMLAllCollection.h
dom/webidl/HTMLAllCollection.webidl
js/src/builtin/Array.cpp
js/src/jsapi-tests/moz.build
js/src/jsapi-tests/testStringIsArrayIndex.cpp
js/src/jsfriendapi.h
testing/web-platform/meta/html/dom/interfaces.https.html.ini
testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini
--- a/dom/html/HTMLAllCollection.cpp
+++ b/dom/html/HTMLAllCollection.cpp
@@ -44,47 +44,20 @@ HTMLAllCollection::GetParentObject() con
 }
 
 uint32_t
 HTMLAllCollection::Length()
 {
   return Collection()->Length(true);
 }
 
-Element*
+nsIContent*
 HTMLAllCollection::Item(uint32_t aIndex)
 {
-  nsIContent* item = Collection()->Item(aIndex);
-  return item ? item->AsElement() : nullptr;
-}
-
-void
-HTMLAllCollection::Item(const Optional<nsAString>& aNameOrIndex,
-                        Nullable<OwningHTMLCollectionOrElement>& aResult)
-{
-  if (!aNameOrIndex.WasPassed()) {
-    aResult.SetNull();
-    return;
-  }
-
-  const nsAString& nameOrIndex = aNameOrIndex.Value();
-  uint32_t indexVal;
-  if (js::StringIsArrayIndex(nameOrIndex.BeginReading(),
-                             nameOrIndex.Length(),
-                             &indexVal)) {
-    Element* element = Item(indexVal);
-    if (element) {
-      aResult.SetValue().SetAsElement() = element;
-    } else {
-      aResult.SetNull();
-    }
-    return;
-  }
-
-  NamedItem(nameOrIndex, aResult);
+  return Collection()->Item(aIndex);
 }
 
 nsContentList*
 HTMLAllCollection::Collection()
 {
   if (!mCollection) {
     nsIDocument* document = mDocument;
     mCollection = document->GetElementsByTagName(NS_LITERAL_STRING("*"));
@@ -143,17 +116,17 @@ HTMLAllCollection::GetDocumentAllList(co
       return new nsContentList(mDocument, DocAllResultMatch, nullptr,
                                nullptr, true, id);
     });
 }
 
 void
 HTMLAllCollection::NamedGetter(const nsAString& aID,
                                bool& aFound,
-                               Nullable<OwningHTMLCollectionOrElement>& aResult)
+                               Nullable<OwningNodeOrHTMLCollection>& aResult)
 {
   if (aID.IsEmpty()) {
     aFound = false;
     aResult.SetNull();
     return;
   }
 
   nsContentList* docAllList = GetDocumentAllList(aID);
@@ -170,17 +143,17 @@ HTMLAllCollection::NamedGetter(const nsA
     aFound = true;
     aResult.SetValue().SetAsHTMLCollection() = docAllList;
     return;
   }
 
   // There's only 0 or 1 items. Return the first one or null.
   if (nsIContent* node = docAllList->Item(0, true)) {
     aFound = true;
-    aResult.SetValue().SetAsElement() = node->AsElement();
+    aResult.SetValue().SetAsNode() = node;
     return;
   }
 
   aFound = false;
   aResult.SetNull();
 }
 
 void
--- a/dom/html/HTMLAllCollection.h
+++ b/dom/html/HTMLAllCollection.h
@@ -11,82 +11,76 @@
 #include "nsISupportsImpl.h"
 #include "nsRefPtrHashtable.h"
 #include "nsWrapperCache.h"
 
 #include <stdint.h>
 
 class nsContentList;
 class nsHTMLDocument;
+class nsIContent;
 class nsINode;
 
 namespace mozilla {
 namespace dom {
 
-class Element;
-class OwningHTMLCollectionOrElement;
+class OwningNodeOrHTMLCollection;
 template<typename> struct Nullable;
-template<typename> class Optional;
 
 class HTMLAllCollection final : public nsISupports
                               , public nsWrapperCache
 {
   ~HTMLAllCollection();
 
 public:
   explicit HTMLAllCollection(nsHTMLDocument* aDocument);
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(HTMLAllCollection)
 
   virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
   nsINode* GetParentObject() const;
 
   uint32_t Length();
-  Element* IndexedGetter(uint32_t aIndex, bool& aFound)
+  nsIContent* Item(uint32_t aIndex);
+  void Item(const nsAString& aName, Nullable<OwningNodeOrHTMLCollection>& aResult)
   {
-    Element* result = Item(aIndex);
+    NamedItem(aName, aResult);
+  }
+  nsIContent* IndexedGetter(uint32_t aIndex, bool& aFound)
+  {
+    nsIContent* result = Item(aIndex);
     aFound = !!result;
     return result;
   }
 
   void NamedItem(const nsAString& aName,
-                 Nullable<OwningHTMLCollectionOrElement>& aResult)
+                 Nullable<OwningNodeOrHTMLCollection>& aResult)
   {
     bool found = false;
     NamedGetter(aName, found, aResult);
   }
   void NamedGetter(const nsAString& aName,
                    bool& aFound,
-                   Nullable<OwningHTMLCollectionOrElement>& aResult);
+                   Nullable<OwningNodeOrHTMLCollection>& aResult);
   void GetSupportedNames(nsTArray<nsString>& aNames);
-
-  void Item(const Optional<nsAString>& aNameOrIndex,
-            Nullable<OwningHTMLCollectionOrElement>& aResult);
-
-  void LegacyCall(JS::Handle<JS::Value>,
-                  const Optional<nsAString>& aNameOrIndex,
-                  Nullable<OwningHTMLCollectionOrElement>& aResult)
+  void LegacyCall(JS::Handle<JS::Value>, const nsAString& aName,
+                  Nullable<OwningNodeOrHTMLCollection>& aResult)
   {
-    Item(aNameOrIndex, aResult);
+    NamedItem(aName, aResult);
   }
 
 private:
   nsContentList* Collection();
 
   /**
    * Returns the HTMLCollection for document.all[aID], or null if there isn't one.
    */
   nsContentList* GetDocumentAllList(const nsAString& aID);
 
-  /**
-   * Helper for indexed getter and spec Item() method.
-   */
-  Element* Item(uint32_t aIndex);
-
   RefPtr<nsHTMLDocument> mDocument;
   RefPtr<nsContentList> mCollection;
   nsRefPtrHashtable<nsStringHashKey, nsContentList> mNamedMap;
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/webidl/HTMLAllCollection.webidl
+++ b/dom/webidl/HTMLAllCollection.webidl
@@ -1,13 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* Emulates undefined through Codegen.py. */
 [LegacyUnenumerableNamedProperties]
 interface HTMLAllCollection {
   readonly attribute unsigned long length;
-  getter Element (unsigned long index);
-  getter (HTMLCollection or Element)? namedItem(DOMString name);
-  (HTMLCollection or Element)? item(optional DOMString nameOrIndex);
-  legacycaller (HTMLCollection or Element)? (optional DOMString nameOrIndex);
+  getter Node? (unsigned long index);
+  Node? item(unsigned long index);
+  (Node or HTMLCollection)? item(DOMString name);
+  legacycaller (Node or HTMLCollection)? (DOMString name);
+  getter (Node or HTMLCollection)? namedItem(DOMString name);
 };
--- a/js/src/builtin/Array.cpp
+++ b/js/src/builtin/Array.cpp
@@ -218,17 +218,17 @@ GetLengthProperty(JSContext* cx, HandleO
  * except that by using signed 31-bit integers we miss the top half of the
  * valid range. This function checks the string representation itself; note
  * that calling a standard conversion routine might allow strings such as
  * "08" or "4.0" as array indices, which they are not.
  *
  */
 template <typename CharT>
 static bool
-StringIsArrayIndexHelper(const CharT* s, uint32_t length, uint32_t* indexp)
+StringIsArrayIndex(const CharT* s, uint32_t length, uint32_t* indexp)
 {
     const CharT* end = s + length;
 
     if (length == 0 || length > (sizeof("4294967294") - 1) || !IsAsciiDigit(*s)) {
         return false;
     }
 
     uint32_t c = 0, previous = 0;
@@ -260,30 +260,18 @@ StringIsArrayIndexHelper(const CharT* s,
     return false;
 }
 
 JS_FRIEND_API(bool)
 js::StringIsArrayIndex(JSLinearString* str, uint32_t* indexp)
 {
     AutoCheckCannotGC nogc;
     return str->hasLatin1Chars()
-           ? StringIsArrayIndexHelper(str->latin1Chars(nogc), str->length(), indexp)
-           : StringIsArrayIndexHelper(str->twoByteChars(nogc), str->length(), indexp);
-}
-
-JS_FRIEND_API(bool)
-js::StringIsArrayIndex(const char16_t* str, uint32_t length, uint32_t* indexp)
-{
-    return StringIsArrayIndexHelper(str, length, indexp);
-}
-
-JS_FRIEND_API(bool)
-js::StringIsArrayIndex(const char* str, uint32_t length, uint32_t* indexp)
-{
-    return StringIsArrayIndexHelper(str, length, indexp);
+           ? ::StringIsArrayIndex(str->latin1Chars(nogc), str->length(), indexp)
+           : ::StringIsArrayIndex(str->twoByteChars(nogc), str->length(), indexp);
 }
 
 template <typename T>
 static bool
 ToId(JSContext* cx, T index, MutableHandleId id);
 
 template <>
 bool
--- a/js/src/jsapi-tests/moz.build
+++ b/js/src/jsapi-tests/moz.build
@@ -92,17 +92,16 @@ UNIFIED_SOURCES += [
     'testSavedStacks.cpp',
     'testScriptInfo.cpp',
     'testScriptObject.cpp',
     'testSetProperty.cpp',
     'testSetPropertyIgnoringNamedGetter.cpp',
     'testSharedImmutableStringsCache.cpp',
     'testSourcePolicy.cpp',
     'testStringBuffer.cpp',
-    'testStringIsArrayIndex.cpp',
     'testStructuredClone.cpp',
     'testSymbol.cpp',
     'testThreadingConditionVariable.cpp',
     'testThreadingExclusiveData.cpp',
     'testThreadingMutex.cpp',
     'testThreadingThread.cpp',
     'testToSignedOrUnsignedInteger.cpp',
     'testTypedArrays.cpp',
deleted file mode 100644
--- a/js/src/jsapi-tests/testStringIsArrayIndex.cpp
+++ /dev/null
@@ -1,79 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * vim: set ts=8 sts=4 et sw=4 tw=99:
- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "jsfriendapi.h"
-
-#include "jsapi-tests/tests.h"
-#include <stdio.h>
-
-using mozilla::ArrayLength;
-
-// Need to account for string literals including the \0 at the end.
-#define STR(x) x, (ArrayLength(x)-1)
-
-static const struct TestTuple {
-    /* The string being tested. */
-    const char* string;
-    /* The number of characters from the string to use. */
-    size_t length;
-    /* Whether this string is an index. */
-    bool isindex;
-    /* If it's an index, what index it is.  Ignored if not an index. */
-    uint32_t index;
-} tests[] = {
-    { STR("0"), true, 0 },
-    { STR("1"), true, 1 },
-    { STR("2"), true, 2 },
-    { STR("9"), true, 9 },
-    { STR("10"), true, 10 },
-    { STR("15"), true, 15 },
-    { STR("16"), true, 16 },
-    { STR("17"), true, 17 },
-    { STR("99"), true, 99 },
-    { STR("100"), true, 100 },
-    { STR("255"), true, 255 },
-    { STR("256"), true, 256 },
-    { STR("257"), true, 257 },
-    { STR("999"), true, 999 },
-    { STR("1000"), true, 1000 },
-    { STR("4095"), true, 4095 },
-    { STR("4096"), true, 4096 },
-    { STR("9999"), true, 9999 },
-    { STR("1073741823"), true, 1073741823 },
-    { STR("1073741824"), true, 1073741824 },
-    { STR("1073741825"), true, 1073741825 },
-    { STR("2147483647"), true, 2147483647 },
-    { STR("2147483648"), true, 2147483648u },
-    { STR("2147483649"), true, 2147483649u },
-    { STR("4294967294"), true, 4294967294u },
-    { STR("4294967295"), false, 0 },  // Not an array index because need to be able to represent length
-    { STR("-1"), false, 0 },
-    { STR("abc"), false, 0 },
-    { STR(" 0"), false, 0 },
-    { STR("0 "), false, 0 },
-    // Tests to make sure the passed-in length is taken into account
-    { "0 ", 1, true, 0 },
-    { "123abc", 3, true, 123 },
-    { "123abc", 2, true, 12 },
-};
-
-BEGIN_TEST(testStringIsArrayIndex)
-{
-    for (size_t i = 0, sz = ArrayLength(tests); i < sz; i++) {
-        uint32_t index;
-        bool isindex = js::StringIsArrayIndex(tests[i].string,
-                                              tests[i].length,
-                                              &index);
-        CHECK_EQUAL(isindex, tests[i].isindex);
-        if (isindex) {
-            CHECK_EQUAL(index, tests[i].index);
-        }
-    }
-
-    return true;
-}
-END_TEST(testStringIsArrayIndex)
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -1002,36 +1002,19 @@ CopyFlatStringChars(char16_t* dest, JSFl
  * results that match the output of Reflect.ownKeys.
  */
 JS_FRIEND_API(bool)
 GetPropertyKeys(JSContext* cx, JS::HandleObject obj, unsigned flags, JS::AutoIdVector* props);
 
 JS_FRIEND_API(bool)
 AppendUnique(JSContext* cx, JS::AutoIdVector& base, JS::AutoIdVector& others);
 
-/**
- * Determine whether the given string is an array index in the sense of <https://tc39.github.io/ecma262/#array-index>.
- *
- * If it isn't, returns false.
- *
- * If it is, returns true and outputs the index in *indexp.
- */
 JS_FRIEND_API(bool)
 StringIsArrayIndex(JSLinearString* str, uint32_t* indexp);
 
-/**
- * Overloads of StringIsArrayIndex taking (char*,length) pairs.  These
- * behave the same as the JSLinearString version.
- */
-JS_FRIEND_API(bool)
-StringIsArrayIndex(const char* str, uint32_t length, uint32_t* indexp);
-
-JS_FRIEND_API(bool)
-StringIsArrayIndex(const char16_t* str, uint32_t length, uint32_t* indexp);
-
 JS_FRIEND_API(void)
 SetPreserveWrapperCallback(JSContext* cx, PreserveWrapperCallback callback);
 
 JS_FRIEND_API(bool)
 IsObjectInContextCompartment(JSObject* obj, const JSContext* cx);
 
 /*
  * NB: keep these in sync with the copy in builtin/SelfHostingDefines.h.
--- a/testing/web-platform/meta/html/dom/interfaces.https.html.ini
+++ b/testing/web-platform/meta/html/dom/interfaces.https.html.ini
@@ -353,16 +353,19 @@
   [Window interface: operation postMessage(any, [object Object\], WindowPostMessageOptions)]
     expected: FAIL
 
   [html interfaces]
     expected: FAIL
 
 
 [interfaces.https.html?include=HTML.*]
+  [HTMLAllCollection interface: operation item(DOMString)]
+    expected: FAIL
+
   [HTMLAllCollection must be primary interface of document.all]
     expected: FAIL
 
   [Stringification of document.all]
     expected: FAIL
 
   [HTMLAllCollection interface: document.all must inherit property "length" with the proper type]
     expected: FAIL
--- a/testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini
+++ b/testing/web-platform/meta/html/infrastructure/common-dom-interfaces/collections/htmlallcollection.html.ini
@@ -1,3 +1,31 @@
 [htmlallcollection.html]
+  [legacy caller with "array index property name"]
+    expected: FAIL
+
+  [legacy caller with "array index property name" as number]
+    expected: FAIL
+
+  [legacy caller with invalid "array index property name"]
+    expected: FAIL
+
+  [legacy caller with no argument]
+    expected: FAIL
+
+  [item method with "array index property name"]
+    expected: FAIL
+
+  [item method with invalid "array index property name"]
+    expected: FAIL
+
+  [item method with no argument]
+    expected: FAIL
+
   [collections are new live HTMLCollection instances]
     expected: FAIL
+
+  [legacy caller with undefined]
+    expected: FAIL
+
+  [item method with undefined]
+    expected: FAIL
+