Make Custom Elements V1 work in HTML imports documents

third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp

Index: third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp
diff --git a/third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp b/third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp
index 1bd3aad2ffc571283db758477872f8e23179e72b..6f0f80d864b9358e3dbfed4eb7d2b7bc1fdab6f4 100644
--- a/third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp
+++ b/third_party/WebKit/Source/core/dom/custom/CustomElementsRegistry.cpp
@@ -75,6 +75,9 @@ CustomElementsRegistry::CustomElementsRegistry(Document* document)
     : m_document(document)
     , m_upgradeCandidates(new UpgradeCandidateMap())
 {
+    // !contextDocument() just for unit testing.
+    DCHECK(document == document->contextDocument()
+        || !document->contextDocument());
 }
 
 DEFINE_TRACE(CustomElementsRegistry)
@@ -214,6 +217,10 @@ CustomElementDefinition* CustomElementsRegistry::definitionForName(
 
 void CustomElementsRegistry::addCandidate(Element* candidate)
 {
+    // !contextDocument() just for unit testing.
+    DCHECK(candidate->document().contextDocument() == m_document
+        || !candidate->document().contextDocument());
+
     const AtomicString& name = candidate->localName();
     if (nameIsDefined(name) || v0NameIsDefined(name))
         return;
@@ -249,6 +256,33 @@ ScriptPromise CustomElementsRegistry::whenDefined(
     return newResolver->promise();
 }
 
+using ElementsInDocument = HeapVector<Member<Element>>;
+using ElementsDocumentMap = HeapHashMap<Document*, ElementsInDocument>;

[dominicc (has gone to gerrit), 2016/07/19 01:57:39]

Maybe DocumentElementsMap, since it's a map *from* document *to* element?

+
+static void addToImports(ElementsDocumentMap& imports, Element* element)
+{
+    Document* document = &element->document();
+    const auto& it = imports.find(document);
+    ElementsInDocument* list;
+    if (it != imports.end()) {
+        list = &it->value;
+    } else {
+        list = &imports.add(document, ElementsInDocument())
+            .storedValue->value;
+    }
+    list->append(element);
+}
+
+static void collectFromImports(ElementsDocumentMap& imports, HeapVector<Member<Element>>* elements)
+{
+    for (const auto& it : imports) {

[dominicc (has gone to gerrit), 2016/07/19 01:57:39]

I don't think hashtable iteration order is a good idea; generally web platform
APIs do things in some stable order.

+        CustomElementUpgradeSorter sorter;
+        for (Element* element : it.value)
+            sorter.add(element);
+        sorter.sorted(elements, it.key);
+    }
+}
+
 void CustomElementsRegistry::collectCandidates(
     const CustomElementDescriptor& desc,
     HeapVector<Member<Element>>* elements)
@@ -256,14 +290,25 @@ void CustomElementsRegistry::collectCandidates(
     UpgradeCandidateMap::iterator it = m_upgradeCandidates->find(desc.name());
     if (it == m_upgradeCandidates->end())
         return;
+
+    ElementsDocumentMap imports;
     CustomElementUpgradeSorter sorter;
     for (Element* element : *it.get()->value) {
         if (!element || !desc.matches(*element))
             continue;
-        sorter.add(element);
+
+        // Group elements by document if they are in import documents.
+        Document& document = element->document();
+        if (document == m_document)
+            sorter.add(element);
+        else if (document.contextDocument() == m_document)
+            addToImports(imports, element);
     }
 
     m_upgradeCandidates->remove(it);
+    // If there were elements from import documents, put them first.
+    if (!imports.isEmpty())
+        collectFromImports(imports, elements);

[dominicc (has gone to gerrit), 2016/07/19 01:57:39]

TL;DR I think you should handle sorting in the custom elements upgrade sorter
and not split that logic into two places unless there's a good reason to do so.

     sorter.sorted(elements, m_document.get());
 }