Bug 1274159 - Part 1: Support looking up definitions by using constructor as a key; r=wchen,jonco
☠☠ backed out by dd390860c6c9 ☠ ☠
authorEdgar Chen <echen@mozilla.com>
Wed, 07 Sep 2016 17:55:21 +0800
changeset 325784 33c69deecb7a16639664b91e7e107e1d7dd74e33
parent 325783 896c2e2190869600e65d5d8c5c4365e4a0bcc9b3
child 325785 5c3a6908b84eb22a6ff121e52a948f1f6b5913e1
push id84797
push userechen@mozilla.com
push dateWed, 14 Dec 2016 03:58:37 +0000
treeherdermozilla-inbound@86d70a2e93e1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswchen, jonco
bugs1274159
milestone53.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 1274159 - Part 1: Support looking up definitions by using constructor as a key; r=wchen,jonco MozReview-Commit-ID: Bj5fNNETT3w
dom/base/CustomElementRegistry.cpp
dom/base/CustomElementRegistry.h
testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini
testing/web-platform/meta/custom-elements/custom-element-registry/define.html.ini
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -98,16 +98,17 @@ CustomElementData::RunCallbackQueue()
   mCurrentCallback = -1;
 }
 
 // Only needed for refcounted objects.
 NS_IMPL_CYCLE_COLLECTION_CLASS(CustomElementRegistry)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(CustomElementRegistry)
   tmp->mCustomDefinitions.Clear();
+  tmp->mConstructors.clear();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWhenDefinedPromiseMap)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWindow)
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(CustomElementRegistry)
   for (auto iter = tmp->mCustomDefinitions.Iter(); !iter.Done(); iter.Next()) {
     nsAutoPtr<LifecycleCallbacks>& callbacks = iter.UserData()->mCallbacks;
@@ -145,16 +146,22 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(Cus
   for (auto iter = tmp->mCustomDefinitions.Iter(); !iter.Done(); iter.Next()) {
     aCallbacks.Trace(&iter.UserData()->mConstructor,
                      "mCustomDefinitions constructor",
                      aClosure);
     aCallbacks.Trace(&iter.UserData()->mPrototype,
                      "mCustomDefinitions prototype",
                      aClosure);
   }
+
+  for (ConstructorMap::Enum iter(tmp->mConstructors); !iter.empty(); iter.popFront()) {
+    aCallbacks.Trace(&iter.front().mutableKey(),
+                     "mConstructors key",
+                     aClosure);
+  }
   NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(CustomElementRegistry)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(CustomElementRegistry)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(CustomElementRegistry)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
@@ -185,16 +192,21 @@ CustomElementRegistry::Create(nsPIDOMWin
 
   if (!Preferences::GetBool("dom.webcomponents.customelements.enabled") &&
       !Preferences::GetBool("dom.webcomponents.enabled")) {
     return nullptr;
   }
 
   RefPtr<CustomElementRegistry> customElementRegistry =
     new CustomElementRegistry(aWindow);
+
+  if (!customElementRegistry->Init()) {
+    return nullptr;
+  }
+
   return customElementRegistry.forget();
 }
 
 /* static */ void
 CustomElementRegistry::ProcessTopElementQueue()
 {
   MOZ_ASSERT(nsContentUtils::IsSafeToRunScript());
 
@@ -243,16 +255,22 @@ CustomElementRegistry::CustomElementRegi
   }
 }
 
 CustomElementRegistry::~CustomElementRegistry()
 {
   mozilla::DropJSObjects(this);
 }
 
+bool
+CustomElementRegistry::Init()
+{
+  return mConstructors.init();
+}
+
 CustomElementDefinition*
 CustomElementRegistry::LookupCustomElementDefinition(const nsAString& aLocalName,
                                                      const nsAString* aIs) const
 {
   nsCOMPtr<nsIAtom> localNameAtom = NS_Atomize(aLocalName);
   nsCOMPtr<nsIAtom> typeAtom = aIs ? NS_Atomize(*aIs) : localNameAtom;
 
   CustomElementDefinition* data = mCustomDefinitions.Get(typeAtom);
@@ -611,19 +629,23 @@ CustomElementRegistry::Define(const nsAS
     aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
     return;
   }
 
   /**
    * 4. If this CustomElementRegistry contains an entry with constructor constructor,
    *    then throw a "NotSupportedError" DOMException and abort these steps.
    */
-  // TODO: Step 3 of HTMLConstructor also needs a way to look up definition by
-  // using constructor. So I plans to figure out a solution to support both of
-  // them in bug 1274159.
+  const auto& ptr = mConstructors.lookup(constructorUnwrapped);
+  if (ptr) {
+    MOZ_ASSERT(mCustomDefinitions.Get(ptr->value()),
+               "Definition must be found in mCustomDefinitions");
+    aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
+    return;
+  }
 
   /**
    * 5. Let localName be name.
    * 6. Let extends be the value of the extends member of options, or null if
    *    no such member exists.
    * 7. If extends is not null, then:
    *    1. If extends is a valid custom element name, then throw a
    *       "NotSupportedError" DOMException.
@@ -769,18 +791,26 @@ CustomElementRegistry::Define(const nsAS
                                 constructor,
                                 constructorPrototype,
                                 callbacks,
                                 0 /* TODO dependent on HTML imports. Bug 877072 */);
 
   /**
    * 12. Add definition to this CustomElementRegistry.
    */
+  if (!mConstructors.put(constructorUnwrapped, nameAtom)) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
   mCustomDefinitions.Put(nameAtom, definition);
 
+  MOZ_ASSERT(mCustomDefinitions.Count() == mConstructors.count(),
+             "Number of entries should be the same");
+
   /**
    * 13. 14. 15. Upgrade candidates
    */
   // TODO: Bug 1299363 - Implement custom element v1 upgrade algorithm
   UpgradeCandidates(cx, nameAtom, definition);
 
   /**
    * 16. If this CustomElementRegistry's when-defined promise map contains an
@@ -855,9 +885,9 @@ CustomElementDefinition::CustomElementDe
     mConstructor(aConstructor),
     mPrototype(aPrototype),
     mCallbacks(aCallbacks),
     mDocOrder(aDocOrder)
 {
 }
 
 } // namespace dom
-} // namespace mozilla
\ No newline at end of file
+} // namespace mozilla
--- a/dom/base/CustomElementRegistry.h
+++ b/dom/base/CustomElementRegistry.h
@@ -2,23 +2,24 @@
 /* vim:set ts=2 sw=2 sts=2 et cindent: */
 /* 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/. */
 
 #ifndef mozilla_dom_CustomElementRegistry_h
 #define mozilla_dom_CustomElementRegistry_h
 
+#include "js/GCHashTable.h"
 #include "js/TypeDecls.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/BindingDeclarations.h"
+#include "mozilla/dom/FunctionBinding.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsWrapperCache.h"
-#include "mozilla/dom/FunctionBinding.h"
 
 class nsDocument;
 
 namespace mozilla {
 namespace dom {
 
 struct CustomElementData;
 struct ElementDefinitionOptions;
@@ -167,16 +168,18 @@ public:
 
   void GetCustomPrototype(nsIAtom* aAtom,
                           JS::MutableHandle<JSObject*> aPrototype);
 
 private:
   explicit CustomElementRegistry(nsPIDOMWindowInner* aWindow);
   ~CustomElementRegistry();
 
+  bool Init();
+
   /**
    * Registers an unresolved custom element that is a candidate for
    * upgrade when the definition is registered via registerElement.
    * |aTypeName| is the name of the custom element type, if it is not
    * provided, then element name is used. |aTypeName| should be provided
    * when registering a custom element that extends an existing
    * element. e.g. <button is="x-button">.
    */
@@ -186,25 +189,35 @@ private:
   void UpgradeCandidates(JSContext* aCx,
                          nsIAtom* aKey,
                          CustomElementDefinition* aDefinition);
 
   typedef nsClassHashtable<nsISupportsHashKey, CustomElementDefinition>
     DefinitionMap;
   typedef nsClassHashtable<nsISupportsHashKey, nsTArray<nsWeakPtr>>
     CandidateMap;
+  typedef JS::GCHashMap<JS::Heap<JSObject*>,
+                        nsCOMPtr<nsIAtom>,
+                        js::MovableCellHasher<JS::Heap<JSObject*>>,
+                        js::SystemAllocPolicy> ConstructorMap;
 
   // Hashtable for custom element definitions in web components.
   // Custom prototypes are stored in the compartment where
   // registerElement was called.
   DefinitionMap mCustomDefinitions;
 
+  // Hashtable for looking up definitions by using constructor as key.
+  // Custom elements' name are stored here and we need to lookup
+  // mCustomDefinitions again to get definitions.
+  ConstructorMap mConstructors;
+
   typedef nsRefPtrHashtable<nsISupportsHashKey, Promise>
     WhenDefinedPromiseMap;
   WhenDefinedPromiseMap mWhenDefinedPromiseMap;
+
   // The "upgrade candidates map" from the web components spec. Maps from a
   // namespace id and local name to a list of elements to upgrade if that
   // element is registered as a custom element.
   CandidateMap mCandidatesMap;
 
   nsCOMPtr<nsPIDOMWindowInner> mWindow;
 
   // Array representing the processing stack in the custom elements
--- a/testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini
+++ b/testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini
@@ -1,13 +1,10 @@
 [CustomElementRegistry.html]
   type: testharness
-  [customElements.define must throw a NotSupportedError when there is already a custom element with the same class]
-    expected: FAIL
-
   [customElements.define must get callbacks of the constructor prototype]
     expected: FAIL
 
   [customElements.define must get "observedAttributes" property on the constructor prototype when "attributeChangedCallback" is present]
     expected: FAIL
 
   [customElements.define must rethrow an exception thrown while getting observedAttributes on the constructor prototype]
     expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/custom-elements/custom-element-registry/define.html.ini
+++ /dev/null
@@ -1,26 +0,0 @@
-[define.html]
-  type: testharness
-  [If constructor is HTMLElement, should throw a TypeError]
-    expected: FAIL
-
-  [If constructor is HTMLButtonElement, should throw a TypeError]
-    expected: FAIL
-
-  [If constructor is HTMLImageElement, should throw a TypeError]
-    expected: FAIL
-
-  [If constructor is HTMLMediaElement, should throw a TypeError]
-    expected: FAIL
-
-  [If constructor is Image, should throw a TypeError]
-    expected: FAIL
-
-  [If constructor is Audio, should throw a TypeError]
-    expected: FAIL
-
-  [If constructor is Option, should throw a TypeError]
-    expected: FAIL
-
-  [If the constructor is already defined, should throw a NotSupportedError]
-    expected: FAIL
-