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 441801 a42a9fc0c5f4bc801873a3803ffefbe0761c9b59
parent 441800 344b7e258ea5d73fdd919a6ed6459189874c3533
child 441802 ee8274a9b6c09241f0da9b6ed3b83a95b8091574
push id109051
push userccoroiu@mozilla.com
push dateThu, 18 Oct 2018 03:23:57 +0000
treeherdermozilla-inbound@a42a9fc0c5f4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1398354
milestone64.0a1
backs out344b7e258ea5d73fdd919a6ed6459189874c3533
8e41c385edb99568fed370ef17d646dceefa00f9
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
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
+